Re: [PATCH 1/2] inet_ntop: silence gcc warning

[Eric Blake]

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   address@hidden    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature