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 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
