Greg - 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:
https://scan5.coverity.com:8443/reports.htm#v40850/p10064 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. donald On Sat, Jul 25, 2015 at 9:43 AM, Greg Troxel <[email protected]> wrote: > > nolan <[email protected]> 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 > [email protected] > https://lists.quagga.net/mailman/listinfo/quagga-dev >
_______________________________________________ Quagga-dev mailing list [email protected] https://lists.quagga.net/mailman/listinfo/quagga-dev
