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.
Just to show you that they're not all so ambiguous, node_device_conf.c:116
has a "dead initialization" that is obviously worth fixing:
virNodeDevCapsDefPtr caps = def->caps;
...
for (caps = def->caps; caps; caps = caps->next) {
Posting that separately.
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list