[libvirt] [PATCH] Fix building with -Og
When building using -Og, gcc sees that some variables can be used uninitialized It can be debatable whether it is possible with our codeflow, but functions should be self-contained and initializations are always good. The return instead of goto is due to actualType being used in the cleanup. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/lxc/lxc_driver.c | 2 +- src/nwfilter/nwfilter_ebiptables_driver.c | 2 +- src/util/virbitmap.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 67f14fe766a5..f0948eae774e 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -4275,7 +4275,7 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn, if (!priv->initpid) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Cannot attach disk until init PID is known")); - goto cleanup; + return -1; } if (virLXCProcessValidateInterface(net) < 0) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 423d069e1b26..b7be2917e29e 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -1570,7 +1570,7 @@ iptablesCreateRuleInstanceStateCtrl(virFirewallPtr fw, const char *ifname, virNWFilterVarCombIterPtr vars) { - int rc; + int rc = 0; bool directionIn = false; char chainPrefix[2]; bool maySkipICMP, inout = false; diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 9283aef1735b..4ca59f9d6227 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -817,7 +817,7 @@ virBitmapLastSetBit(virBitmapPtr bitmap) ssize_t i; int unusedBits; ssize_t sz; - unsigned long bits; + unsigned long bits = 0; unusedBits = bitmap->map_len * VIR_BITMAP_BITS_PER_UNIT - bitmap->max_bit; -- 2.8.3
On Fri, Jun 03, 2016 at 13:32:02 +0200, Martin Kletzander wrote:
When building using -Og, gcc sees that some variables can be used uninitialized It can be debatable whether it is possible with our codeflow, but functions should be self-contained and initializations are always good. The return instead of goto is due to actualType being used in the cleanup.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/lxc/lxc_driver.c | 2 +- src/nwfilter/nwfilter_ebiptables_driver.c | 2 +- src/util/virbitmap.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 67f14fe766a5..f0948eae774e 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -4275,7 +4275,7 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn, if (!priv->initpid) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Cannot attach disk until init PID is known")); - goto cleanup; + return -1; }
False positive. actualType is never evaluated uninitialized since ret is set to -1 and veth is NULL. It makes sense to change it since a lot of stuff below is returning -1 directly.
if (virLXCProcessValidateInterface(net) < 0) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 423d069e1b26..b7be2917e29e 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -1570,7 +1570,7 @@ iptablesCreateRuleInstanceStateCtrl(virFirewallPtr fw, const char *ifname, virNWFilterVarCombIterPtr vars) { - int rc; + int rc = 0; bool directionIn = false; char chainPrefix[2]; bool maySkipICMP, inout = false;
Bah. This function makes my brain hurt. I didn't bother checking.
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 9283aef1735b..4ca59f9d6227 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -817,7 +817,7 @@ virBitmapLastSetBit(virBitmapPtr bitmap) ssize_t i; int unusedBits; ssize_t sz; - unsigned long bits; + unsigned long bits = 0;
okay, so there are possible input values ableit being invalid and impossible that would actually allow to evaluate bits when uninitialized. ACK, I hope that people get tired of experimenting with bleeding edge compilers soon. Peter
On 03.06.2016 13:52, Peter Krempa wrote:
On Fri, Jun 03, 2016 at 13:32:02 +0200, Martin Kletzander wrote:
<snip/>
ACK, I hope that people get tired of experimenting with bleeding edge compilers soon.
Well, this is not a result of a bleeding edge compiler. This occurs on my (now ancient) compiler too. I got to the same result. Martin and me were trying to get a sensible backtraces instead of the following ones: Condvar #0 (0x0x7f97580010c8) first referenced by: /home/zippy/tmp/mutrace.git/.libs/libmutrace.so(pthread_cond_init+0x122) [0x7f97b92946f0] /home/zippy/work/libvirt/libvirt.git/src/.libs/libvirt.so.0(virCondInit+0x1d) [0x7f97b8757a27] /home/zippy/work/libvirt/libvirt.git/src/.libs/libvirt_driver_qemu.so(+0xb87d0) [0x7f978679c7d0] /home/zippy/work/libvirt/libvirt.git/src/.libs/libvirt_driver_qemu.so(qemuMonitorOpen+0x12e) [0x7f978679cbc8] /home/zippy/work/libvirt/libvirt.git/src/.libs/libvirt_driver_qemu.so(+0x99a84) [0x7f978677da84] /home/zippy/work/libvirt/libvirt.git/src/.libs/libvirt_driver_qemu.so(+0x9d4e1) [0x7f97867814e1] /home/zippy/work/libvirt/libvirt.git/src/.libs/libvirt.so.0(+0xe6c4c) [0x7f97b8757c4c] /lib64/libpthread.so.0(+0x7434) [0x7f97b54c6434] (notice those anonymous static functions which makes this output unreadable) This is an output from a tool that analyses mutex/condvar congestions: http://git.0pointer.net/mutrace.git Cheers. Michal
On Fri, Jun 03, 2016 at 14:56:53 +0200, Michal Privoznik wrote:
On 03.06.2016 13:52, Peter Krempa wrote:
On Fri, Jun 03, 2016 at 13:32:02 +0200, Martin Kletzander wrote:
<snip/>
ACK, I hope that people get tired of experimenting with bleeding edge compilers soon.
Well, this is not a result of a bleeding edge compiler. This occurs on my (now ancient) compiler too. I got to the same result. Martin and me were trying to get a sensible backtraces instead of the following ones:
I guess I'm too iritated by the recent influx of patches regarding false-positive build failures with new gcc.
On Fri, Jun 03, 2016 at 01:52:40PM +0200, Peter Krempa wrote:
On Fri, Jun 03, 2016 at 13:32:02 +0200, Martin Kletzander wrote:
When building using -Og, gcc sees that some variables can be used uninitialized It can be debatable whether it is possible with our codeflow, but functions should be self-contained and initializations are always good. The return instead of goto is due to actualType being used in the cleanup.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/lxc/lxc_driver.c | 2 +- src/nwfilter/nwfilter_ebiptables_driver.c | 2 +- src/util/virbitmap.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 67f14fe766a5..f0948eae774e 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -4275,7 +4275,7 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn, if (!priv->initpid) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Cannot attach disk until init PID is known")); - goto cleanup; + return -1; }
False positive. actualType is never evaluated uninitialized since ret is set to -1 and veth is NULL.
I didn't check the initialization for veth, but it looks better this way.
It makes sense to change it since a lot of stuff below is returning -1 directly.
if (virLXCProcessValidateInterface(net) < 0) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 423d069e1b26..b7be2917e29e 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -1570,7 +1570,7 @@ iptablesCreateRuleInstanceStateCtrl(virFirewallPtr fw, const char *ifname, virNWFilterVarCombIterPtr vars) { - int rc; + int rc = 0; bool directionIn = false; char chainPrefix[2]; bool maySkipICMP, inout = false;
Bah. This function makes my brain hurt. I didn't bother checking.
Mine too, had to check it three times.
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 9283aef1735b..4ca59f9d6227 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -817,7 +817,7 @@ virBitmapLastSetBit(virBitmapPtr bitmap) ssize_t i; int unusedBits; ssize_t sz; - unsigned long bits; + unsigned long bits = 0;
okay, so there are possible input values ableit being invalid and impossible that would actually allow to evaluate bits when uninitialized.
ACK, I hope that people get tired of experimenting with bleeding edge compilers soon.
I have no idea what that has in common with this patch. I used gcc 5 (hardly bleeding edge, more like a flesh wound border) and just wanted to get some more debugging info kept in the binary. Plus lok at this excerpt from gcc(1) about -Og: "It should be the optimization level of choice for the standard edit-compile-debug cycle, ..." Anyway, sorry for compiling our source with non-default config ;) Martin P.S.: Pushed now.
participants (3)
-
Martin Kletzander -
Michal Privoznik -
Peter Krempa