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 <j...@rokeby.ie> 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.char...@wanadoo.fr> 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@lists.launchpad.net > Unsubscribe : https://launchpad.net/~kicad-developers > More help : https://help.launchpad.net/ListHelp >
_______________________________________________ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
