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

[Daniel Veillard]

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 at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/