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(a)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.