kicad-developers team mailing list archive

[Jon Evans <jon@xxxxxxxxxxxxx>]

Any reason not to just make a policy moving forward to define anything
related to file formats using stdint types so that there are no
architecture variations possible?

On Mon, Mar 25, 2019 at 2:59 PM Jeff Young <jeff@xxxxxxxxx> wrote:

> Hi JP,
>
> I’m afraid I just move the decl (and its comment) from another file.  It
> appears the author
> was Henner Zeller in 3f57fa5d2443a085c9fa243c615d71dc470a0107.
>
> Commit comment was:
>
>     Change time_t in the functions that deal with timestamps to a new
>     typedef timestamp_t (defined as a long).
>     that makes sure the c++ side and swigged Python side agree on the
>     type, because time_t create problems in Python scripts.
>
> Cheers,
> Jeff.
>
>
> On 25 Mar 2019, at 15:51, jp charras <jp.charras@xxxxxxxxxx> wrote:
>
> Le 25/03/2019 à 16:22, Wayne Stambaugh a écrit :
>
> On 3/24/2019 2:34 PM, Seth Hillbrand wrote:
>
> Am 2019-03-24 13:23, schrieb Jon Evans:
>
> Hi all,
>
> Another question from this forum thread:
>
> https://forum.kicad.info/t/before-giving-up-on-kicad-one-last-attempt-erc-misery/15933/67
>
>
> timestamp_t is defined as "long", with a note that swig can't handle
> int32_t.
>
>
> I don't understand this comment.  SWIG includes stdint.i which
> explicitly defines the exact integral types.  Maybe Jeff can shed some
> light here?
>
>
> This means that timestamp_t will be 32-bit on many platforms, but
> 64-bit on Linux and MacOS running on 64-bit hardware.  Apparently
> there is at least one bug here involving the Eagle importer writing
> out library files with 64-bit timestamps, which 32-bit KiCad cannot
> open.
>
>
> This is a problem in ParseHex().  If it gets a hex value that overflows,
> it throws an error, stopping the processing.  So this isn't specific to
> the Eagle plugin but rather a 32-bit/64-bit issue.  We allow 64-bit to
> be written to file but only generate a time_t (32-bit) value for new,
> internal stamps.
>
> The Eagle processor creates its own "timestamp" by hash which is 64 bits
> on a 64 bit machine.
>
>
> Do you mean our Eagle plugins are not truncating 64 bit time stamps in
> Eagle files?  If so, the problem needs to be fixed here.  AFAIK, KiCad
> has always used 32 bit time stamps.
>
>
> The issue is the fact the legacy plugin currently uses a long as time
> stamp, but do not truncate if to 32 bits when writing a .sch file.
> So because Eagle importer generate a long value, the legacy plugin
> writes 8 hexa chars on 32 bits platform, but can write more than 8 chars
> on 64 bits platforms
>
>
>
> Q1: Is there actually a bug here? maybe someone more familiar with the
> Eagle importer can take a look.
>
>
> Yes.  Definitely bug.  We should be trimming the hash-based timestamp to
> 32-bits.
>
>
> You would also have to add checking for duplicate time stamps due to the
> truncation of the upper 32 bits.
>
>
> Q2: Shouldn't we change the type of timestamp_t to be either "int" (if
> we want it to be 32-bit) or "long long" (if we want it to be 64-bit)?
>
>
> I think 32-bits is the correct way to go here and using uint32_t should
> work but I may be missing an important detail.
>
> My first thought is that we should change timestamp_t to be 32-bit on
> all platforms, but I'm not sure the best way to handle existing files
> that have been written out with 64-bit timestamps.
>
>
> This is problematic.  I think we could patch the reader to handle 64-bit
> values but store 32-bit.
>
> -Seth
>
>
> I have a fix for that (it forces 32 bits in timestamp_t and allows 64
> bits when reading a file).
>
> Annotation should already take care of duplicate timestamps (and replace
> duplicates if any) unless bug.
>
> The right fix is to use uint32_t for timestamps, at least for the
> current file format.
>
> @Jeff, you added a comment in common.h about an issue with Swig.
> What is this issue you encountered when using uint32_t as typedef for
> timestamp_t with swig (I did not see an issue during my tests)?
>
> --
> Jean-Pierre CHARRAS
>
>
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>