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