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

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

 



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.

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]