[libvirt] [PATCH 2/2] tests: Avoid "jump skips variable initialization" with GCC 9.

[Daniel P. Berrangé]

On Mon, Jan 21, 2019 at 03:13:21PM +0000, Richard W.M. Jones wrote:
> GCC 9 gives pages of errors like:
> 
> qemumonitorjsontest.c: In function 'mymain':
> qemumonitorjsontest.c:2904:9: error: jump skips variable initialization [-Werror=jump-misses-init]
>  2904 |         goto cleanup;
>       |         ^~~~
> qemumonitorjsontest.c:3111:2: note: label 'cleanup' defined here
>  3111 |  cleanup:
>       |  ^~~~~~~
> qemumonitorjsontest.c:2920:54: note: '({anonymous})' declared here
>  2920 |     simpleFunc = (testQemuMonitorJSONSimpleFuncData) {.xmlopt = driver.xmlopt, \
>       |                                                      ^
> qemumonitorjsontest.c:3008:5: note: in expansion of macro 'DO_TEST_GEN'
>  3008 |     DO_TEST_GEN(qemuMonitorJSONBlockdevMediumInsert);
>       |     ^~~~~~~~~~~

Ok, it is complaining about "simpleFunc", because for the 5 locally
declared variables, we've done explicit initialization before any
goto's are encountered.  The fun thing is that the cleanup: block
does not reference "simpleFunc" in any way - it is only ever used
immediately after it is initialized.

So GCC is warning about something that can't happen here.

The definition of -Wjump-misses-init says

  [quote]
     Warn if a 'goto' statement or a 'switch' statement jumps forward
     across the initialization of a variable, or jumps backward to a
     label after the variable has been initialized.  This only warns
     about variables that are initialized when they are declared. 
  [/quote]

we're jumping forwards here, but the last sentance suggests it would
only warn if you are jumping over the variable declaration. ie if we
had

    testQemuMonitorJSONSimpleFuncData simpleFunc;

in the middle of the method, instead of top of the method. So either
they've intentionally expanded the scope of the warning, or this is
a bug in GCC.

Either way the simple solution is probably just:

    testQemuMonitorJSONSimpleFuncData simpleFunc = NULL;

> 
> By moving the cleanup section up near the top of the function we can
> avoid this.  I think a better way might be to disable the warning.
> ---
>  tests/qemumonitorjsontest.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
> index 1a8a31717f..299c5f0cbe 100644
> --- a/tests/qemumonitorjsontest.c
> +++ b/tests/qemumonitorjsontest.c
> @@ -2900,8 +2900,12 @@ mymain(void)
>  
>      if (!(qapiData.schema = testQEMUSchemaLoad())) {
>          VIR_TEST_VERBOSE("failed to load qapi schema\n");
> -        ret = -1;
> -        goto cleanup;
> +    cleanup:
> +        VIR_FREE(metaschemastr);
> +        virJSONValueFree(metaschema);
> +        virHashFree(qapiData.schema);
> +        qemuTestDriverFree(&driver);
> +        return -1;
>      }
>  
>  #define DO_TEST(name) \
> @@ -3098,7 +3102,6 @@ mymain(void)
>      if (!(metaschema = testQEMUSchemaGetLatest()) ||
>          !(metaschemastr = virJSONValueToString(metaschema, false))) {
>          VIR_TEST_VERBOSE("failed to load latest qapi schema\n");
> -        ret = -1;
>          goto cleanup;
>      }
>  
> @@ -3108,7 +3111,6 @@ mymain(void)
>  
>  #undef DO_TEST_QAPI_SCHEMA
>  
> - cleanup:
>      VIR_FREE(metaschemastr);
>      virJSONValueFree(metaschema);
>      virHashFree(qapiData.schema);
> -- 
> 2.20.1
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|