← Back to team overview

kicad-developers team mailing list archive

Re: timestamp_t bit width


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.

>> 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
> _______________________________________________
> 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

Follow ups