kicad-developers team mailing list archive

[Chris Pavlina <pavlina.chris@xxxxxxxxx>]

Thank you. I added a comment to that effect locally, which will find its way
into the upcoming format version handling patch. (I refactored that section a
bit as the relevant function was a bit of a monster and hard to integrate
version handling into without some rearranging. No intentional changes in
actual behavior, and I will be testing this section very thoroughly tomorrow.)

On Sun, May 01, 2016 at 08:06:41AM +0200, jp charras wrote:
> Le 01/05/2016 à 05:39, Chris Pavlina a écrit :
> > I have a pretty specific question about some relatively old code that
> > intersects with what I'm working on now. Dick was the original author, not sure
> > if he'd remember... -
> > 
> > pcbnew/legacy_plugin.h:
> > 33  #define FOOTPRINT_LIBRARY_HEADER       "PCBNEW-LibModule-V1"
> > 34  #define FOOTPRINT_LIBRARY_HEADER_CNT   18
> > 
> > 
> > FOOTPRINT_LIBRARY_HEADER_CNT is being used as a length in comparisons using
> > strncmp(). The length does not match, however, as "PCBNEW-LibModule-V1" is 19
> > characters long, not 18. At first I thought this was a simple mistake, and
> > replaced FOOTPRINT_LIBRARY_HEADER_CNT with strlen(FOOTPRINT_LIBRARY_HEADER),
> > which eliminates the possibility of a miscount (and for the micro-optimizers
> > out there, all the compilers I've used optimize strlen(literal) ;).
> > 
> > But then it occurs to me that it *might* have been intentional, as comparing
> > with a length of 18 will compare everything but the version number, which may
> > have been desired.
> > 
> > Anyone willing to have a look at the code that uses this, and see what they
> > think the intended use was? Has there ever been a V2?
> > 
> 
> It *is* intentional.
> PCBNEW-LibModule-V1 is the current identifier of legacy libraries.
> PCBNEW-LibModule-Vnn was planned for future version.
> Obviously, it did not happen because footprint libraries are no more files.
> 
> The first 18 letters are used to know if a file is a kicad footprint file or not.
> Now, they are used to select the plugin, when trying to automatically select a plugin to import a
> footprint file, and using strlen(FOOTPRINT_LIBRARY_HEADER) is not good in this case.
> 
> Be careful when changing code which read old files.
> You easily can break compatibility of very old files (for instance 10 year old: you cannot be sure
> the identifier is PCBNEW-LibModule-V1. It can be PCBNEW-LibModule-Version 1 of some other).
> To change this kind of (good) code, you must have very good reasons.
> 
> -- 
> Jean-Pierre CHARRAS
> 
> _______________________________________________
> 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