[quagga-dev 12881] Re: [PATCH 2/7] Fixed logically dead code in lib/stream.c
sharpd at cumulusnetworks.com
Sat Jul 25 16:10:55 BST 2015
These scan id's are from coverity. They provide a open source scanning
tool. I assumed since one of the maintainers was running the scan that it
was a well known resource for Quagga:
Hence my request for Denil to put the scan id in, it seemed more than
reasonable at the time.
As for the assert, since I knew that asserts were always compiled into the
code, I figured the removal of the if statement was the correct thing to
do, hence my instructions to Denil to do so.
On Sat, Jul 25, 2015 at 9:43 AM, Greg Troxel <gdt at ir.bbn.com> wrote:
> nolan <nolan at cumulusnetworks.com> writes:
> (with top-posting repaired!)
> > On 07/23/2015 06:52 AM, Denil Vira wrote:
> >> Coverity Scan ID 1302488. Test for size==0 makes no sense, since assert
> immediately before it
> >> would not let this code happen.
> What namespace are these scan ids in? Are they someplace public that's
> committed to existing for a lont time? If they aren't, I would prefer
> that they not be in the commit messages.
> > Asserts are sometimes compiled out, so the redundant check is
> > necessary. The optimizer will cull it if asserts are enabled, so it
> > doesn't hurt the generated code.
> Really we need rules in our style/design guide that addresses this, and
> then we can evaluate the code against the rules. It's an entirely
> reasonable style (known as design by contract) to have asserts at the
> beginning of a function to check the documented calling conventions.
> Then, one would expect the asserts to remain in production. Having
> asserts and then conditionals to return errors seems overly complex to me.
> Quagga-dev mailing list
> Quagga-dev at lists.quagga.net
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the Quagga-dev