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.
>
[...]
>
> 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.
I must admit I don't see a NULL initialization of a pointer for safety
when entering a routine as a bad thing, actually this could be
considered a standard feature in any highlevel language. Assignment of
a computed value is rather different. So while I agree with your example
I still think it's fine to initialize the pointer to NULL in the patch.
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list