[libvirt] [PATCH] Fix building with -Og

[Martin Kletzander]

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 at 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160603/50245b55/attachment-0001.sig>