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

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



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 :|


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