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

Re: [libvirt] Miscellaneous fixes to build with -Werror



On Thu, Sep 24, 2009 at 01:02:24PM +0200, Chris Lalancette wrote:
> Charles Duffy wrote:
> > HACKING suggests compiling with --enable-compile-warnings=error before 
> > submitting any patches; however, current master fails for me on this 
> > account (CentOS 5.3; gcc 4.1.2).
> > 
> > Please see attached. I suspect most of these should be uncontroversial 
> > -- but wonder if perhaps virStrcpy uses would be better converted to 
> > virStrcpyStatic rather than adding virStrcpy to the symbol list as done 
> > here, and am curious about whether the need for explicit initialization 
> > to silence a warning regarding qemudSetLogging's log_level indicates a 
> > bug in the macro later used to assign that value.
> > 
> > diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> > index 2bae782..ec2eab1 100644
> > --- a/daemon/libvirtd.c
> > +++ b/daemon/libvirtd.c
> > @@ -2492,7 +2492,7 @@ remoteReadSaslAllowedUsernameList (virConfPtr conf ATTRIBUTE_UNUSED,
> >   */
> >  static int
> >  qemudSetLogging(virConfPtr conf, const char *filename) {
> > -    int log_level;
> > +    int log_level = 0;
> >      char *log_filters = NULL;
> >      char *log_outputs = NULL;
> >      int ret = -1;
> 
> Looking at this more, I'm not sure.  The comment above GET_CONF_INT(log_level)
> looks to be bogus; GET_CONF_INT does *not* return 0 if the value is not in the
> config file, it doesn't change anything at all.  Still, I don't quite know the
> reasoning behind the original change (back in early August), so I'm
> uncomfortable changing it.

Its pretty clear though that the 'log_level' var will never be initialized
if the 'log_level' setting isn't in the config file. ie, 

  GET_CONF_INTconf, filename, log_level);

expands to

        virConfValuePtr p = virConfGetValue (conf, "log_level");
        if (p) {
            if (checkType (p, filename, "log_level", VIR_CONF_LONG) < 0)
                goto free_and_fail;
            (log_level) = p->l;
        }

So it is never set if 'p' is NULL


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]