← Back to team overview

kicad-developers team mailing list archive

Re: timestamp_t bit width

 

MSVC 2010 includes it.

https://stackoverflow.com/questions/126279/c99-stdint-h-header-and-ms-visual-studio


*Drew Van Zandt*


On Mon, Mar 25, 2019 at 5:25 PM Wayne Stambaugh <stambaughw@xxxxxxxxx>
wrote:

> I don't know if it's still true but msvc didn't include stdint.h so
> making this policy would have left msvc users without a solution.  This
> may no longer be true and I seem to remember that someone was providing
> an implementation of stdint.h for msvc.  If this is no longer the case,
> then it could be added to the coding policy.
>
> On 3/25/19 3:02 PM, Jon Evans wrote:
> > 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
> > <mailto: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
> >>     <mailto: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
> >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
> >     Unsubscribe : https://launchpad.net/~kicad-developers
> >     More help   : https://help.launchpad.net/ListHelp
> >
> >
> > _______________________________________________
> > 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
> >
>
> _______________________________________________
> 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

References