kicad-developers team mailing list archive
Mailing list archive
Re: timestamp_t bit width
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:
>>> timestamp_t is defined as "long", with a note that swig can't handle
>> 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
>> 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
> 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.
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)?