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