[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] uml_conf.c: don't return an uninitialized pointer

Jim Meyering wrote:

> Jim Meyering wrote:
>> Daniel P. Berrange wrote:
>> ...
>>>> Actually I did that first, but then un-did it in favor
>>>> of the change above.  Why?  because that initialization could
>>>> mask a failure to initialize in a new case.
>>>> With per-case initialization, we'd detect the bug at
>>>> compile/static-analysis time.  With the up-front unconditional
>>>> initialization, we cannot, and would have to rely on testing to find it.
>>> It is a tradeoff, but I still prefer the initialization at time of
>>> declaration as a safety net, and we do use this pattern pretty much
>>> everywhere
>> Ok.  adjusted
> Actually, I will now try to convince you that we should
> do it the other way.  Not only is it better to have the compiler
> tell us about this problem, but if ever someone were to add the
> definition that seems to be missing in the default case, clang
> would report the preceding initialization (the one you prefer)
> as a "dead store" error.
> In fact, now that I'm looking at what clang calls
> dead initializations, I see just such a case in mdns.c:
> static void libvirtd_mdns_client_callback(AvahiClient *c, AvahiClientState state, void *userdata) {
>     ...
>     struct libvirtd_mdns_group *group = mdns->group;
>     ...
>     switch (state) {
>     ...
>     }
> And in every "case" of that switch, there is an assignment to "group".
> That renders the preceding assignment useless, and hence clang calls it
> a dead initialization.

Oops.  That was inaccurate.
Each case does not set group.
However, each following *use* of group
is preceded by another assignment to that variable.

The point stands though.

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]