[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/2] inet_ntop: silence gcc warning
From: |
Eric Blake |
Subject: |
Re: [PATCH 1/2] inet_ntop: silence gcc warning |
Date: |
Wed, 11 Jan 2012 17:01:43 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:9.0) Gecko/20111222 Thunderbird/9.0 |
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.
--
Eric Blake address@hidden +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- FYI misc compile warnings on Mingw32, Daniel P. Berrange, 2012/01/11
- [PATCH 0/2] silence some mingw warnings, Eric Blake, 2012/01/11
- [PATCH 1/2] inet_ntop: silence gcc warning, Eric Blake, 2012/01/11
- Re: [PATCH 1/2] inet_ntop: silence gcc warning, Paul Eggert, 2012/01/11
- Re: [PATCH 1/2] inet_ntop: silence gcc warning,
Eric Blake <=
- Re: [PATCH 1/2] inet_ntop: silence gcc warning, Eric Blake, 2012/01/11
- Re: [PATCH 1/2] inet_ntop: silence gcc warning, Paul Eggert, 2012/01/11
- Re: [PATCH 1/2] inet_ntop: silence gcc warning, Jim Meyering, 2012/01/12
- Re: [PATCH 1/2] inet_ntop: silence gcc warning, Eric Blake, 2012/01/12