Re: [Kicad-developers] timestamp_t bit width

2019-03-25 Thread Mark Roszko 
Yes it was added in 2010. There were some other headers VS had issues with
for longer. But as of VS2017 15.9, it is compliant fully with all of
C++11/14/17 and most of C99.

On Mon, Mar 25, 2019 at 6:16 PM Drew Van Zandt 
wrote:

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

Re: [Kicad-developers] timestamp_t bit width

2019-03-25 Thread Drew Van Zandt 
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 
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  > > 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  >> > 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

Re: [Kicad-developers] timestamp_t bit width

2019-03-25 Thread Wayne Stambaugh 
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  > 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 > > 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

Re: [Kicad-developers] timestamp_t bit width

2019-03-25 Thread jp charras 
Le 25/03/2019 à 19:58, Jeff Young a écrit :
> 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.

Hi Jeff,
So i am guessing Henner had an issue with time_t, not with uint32_t.

Thanks.


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

Re: [Kicad-developers] timestamp_t bit width

2019-03-25 Thread Jon Evans 
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  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  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

Re: [Kicad-developers] timestamp_t bit width

2019-03-25 Thread Jeff Young 
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  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

Re: [Kicad-developers] timestamp_t bit width

2019-03-25 Thread jp charras 
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

Re: [Kicad-developers] timestamp_t bit width

2019-03-25 Thread Wayne Stambaugh 
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@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

Re: [Kicad-developers] timestamp_t bit width

2019-03-24 Thread Nick Østergaard 
A but report about this was also reported for tracking at
https://bugs.launchpad.net/bugs/1821476

søn. 24. mar. 2019 19.34 skrev Seth Hillbrand :

> 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.
>
> > 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.
>
> > 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@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

Re: [Kicad-developers] timestamp_t bit width

2019-03-24 Thread Seth Hillbrand 

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.



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.



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@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp