On Thu, Sep 03, 2009 at 01:39:25PM +0200, 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. This isn't a dead store though - we are setting the default return value for cases where there's no explicit return value set. It would only become a dead store if we also added the redundant 'ret = NULL' initialization that your initial patch did. > In fact, now that I'm looking at what clang calls > dead initializations, I see just such a case in mdns.c: These are different scenarios dealing with local use variables only, rather tha default return values as the above case. Removing these redundant inits makes sense for mdns/nodedev. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list