On 01/11/2012 05:01 PM, Eric Blake wrote: > On 01/11/2012 04:44 PM, Paul Eggert wrote: >> On 01/11/12 15:24, Eric Blake wrote: >>> + best.len = 0; >> >> I have some qualms about adding unnecessary initializations >> merely to silence GCC. It's not just that it bloats the >> runtime -- it's that it makes the code more confusing, because >> later readers might mistakenly assume that the initializations >> are necessary, which might cause them to waste time trying to figure >> out why the initializations are there. >> >> How about if we put that assignment inside an "#ifdef lint", >> or wrap it in IF_LINT, or something like that? That should >> make it clearer and avoid the runtime bloat. > > But putting it inside #ifdef lint means you won't solve the compilation > warning in the default case. And in this case, it took me several > minutes of staring at the code to see _why_ the variable was never used > uninitialized - no wonder gcc -O2 had problems coming to the same > conclusion. All uses of best.len are gated behind an earlier > short-circuiting check of best.base != -1, while the only assignment to > best.base is via the assignment of best = cur; which in turn means you > have to prove that cur.base is assigned on all paths that reach the > 'best = cur' statement, which involves reasoning through multiple > iterations of the loop body. And in this particular case, setting > best.len to 0 _is_ appropriate for the algorithm at hand (finding the > longest sequence of consecutive 0 bytes, where we start with a length of > 0). At least to me, seeing the explicit initialization made it easier > to reason about the algorithm, rather than trying to figure out whether > the initialization was necessary.
If nothing else, the glibc code for inet_ntop (which derives from the same Paul Vixie algorithm) initialized not only best.len, but also cur.len. -- Eric Blake ebl...@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
