kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxx>]

On 3/31/2015 9:18 AM, Nick Østergaard wrote:
> 
> Den 31/03/2015 13.29 skrev "Brian Sidebotham"
> <brian.sidebotham@xxxxxxxxx <mailto:brian.sidebotham@xxxxxxxxx>>:
>>
>> I've just been going through the Coverity thing during a break at work
>> and tagged a few things ready to fix at home. The second I noticed is
>> because we use delete on a member variable pointer without
>> re-assignment.
>>
>> I think it would be good policy to say unless the scope of the pointer
>> variable is the local code block, delete must be immediately followed
>> by a NULL assignment. i.e. from what I'm looking at now in the
>> vrml_v2_modelparser.cpp:241 or :263
>>
>>     delete m_model;
>>
>> should really be:
>>
>>     delete m_model;
>>     m_model = NULL;
>>
>> Other functions (namely read_DEF) make assignment from m_model. It's
>> too complicated to try and rely on knowing the logical order of
>> functions in a file format parser to guarantee that it won't break. At
>> least if the assignment from m_model assigns NULL, debugging is
>> reasonably sane.
>>
>> As an aside, I came across code ( pcbnew/eagle_plugin.cpp:1406 :1504,
>> etc ) in which the coding style policy 4.2.3 has been applied to else
>> if clauses. Is that intended? There's both types in that file. I was
>> going to do some tidying as I went along fixing Coverity items and
>> don't know which to go for. The coding style policy 4.7 has an example
>> where there's no blank line above them for reference...
> 
> I dont have the file at hand now, not as I see it, the brace rule shouls
> override the emtpy line before if rule.  Wayne, please confirm if this
> is how it is supposed to be read. FYI I have an upcomming patch that
> will fix som import issues in the eagle importer plugin.

There should not be any blank lines in between the "else if()"
statements according to the policy.  I'm also not a big fan of comments
between control statements such as.

if()
    doSomething()

// This is a comment for the else condtion.
else
    doSomethingElse()

should look like this

if()
    doSomething()
else
    // This is a comment for the else condtion.
    doSomethingElse()

but this is not strictly policy so you can leave it that way if you
prefer.  I always use a blank line at the beginning and the end of every
control block.  It makes it easier to see where the control blocks begin
and end.  Once again, this is not policy so this is up to the programmer.

>>
>> Best Regards,
>>
>> Brian.
>>
>>
>>
>> On 30 March 2015 at 19:15, Wayne Stambaugh <stambaughw@xxxxxxxxx
> <mailto:stambaughw@xxxxxxxxx>> wrote:
>> > Now that we are in feature freeze, the next big task is to fix all of
>> > our critical and high importance bugs in the bug tracker.  To me,
>> > critical bugs are anything that causes a crash and/or data
>> > loss/curruption and high importance bugs are memory leaks.  These must
>> > be fixed before release.  Everything else is medium, low, or wishlist
>> > and do not need to be fixed for the stable release.  It doesn't look
>> > like there are many critical/high importance bugs that are not tagged as
>> > fix released.  However, I'm not sure all of the bugs that are
>> > critical/high importance have been tagged correctly so we need to make
>> > sure nothing has fallen through the cracks.  We also need to tagged any
>> > of the these bugs as fix committed if they have been fixed.
>> >
>> > All Coverity scan errors with a high impact need to be address.  If they
>> > are false positives, please mark them as such.  I still see some
>> > resource leaks in there so those should be given priority.  All other
>> > errors can be fixed when convenient.
>> >
>> > I would like to have at a minimum of one month after the last of the
>> > these issues is resolved so users can test to make sure there are no
>> > other issues lurking just beneath the surface.
>> >
>> > Thank you everyone for your hard work.  Hopefully we can make the stable
>> > release happen sometime around the middle of the year.
>> >
>> > Cheers,
>> >
>> > Wayne
>> >
>> > _______________________________________________
>> > 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
> <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help   : https://help.launchpad.net/ListHelp
>