[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Mon, Jan 21, 2019 at 04:38:25PM +0100, Peter Krempa wrote:
> On Mon, Jan 21, 2019 at 15:13:21 +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);
> >       |     ^~~~~~~~~~~
> > 
> > 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;
> 
> We generally avoid jumps back. Also cleanup really should be at the end. 
> 
> Wouldn't be enough just to memset the few structs at the beginning?

No, this appears to be another GCC regression. IMHO the following should
not warn, yet it does:

$ cat demo.c

#include <stdlib.h>

struct demo {
    const char *cmd;
};

int main(void)
{
  struct demo demo = {0};

  if ((demo.cmd = getenv("FOO")) == NULL) {
    goto cleanup;
  }

  demo = (struct demo) { .cmd = "foo" };

 cleanup:
  return 0;
}


$ gcc -Wjump-misses-init  -o demo demo.c
demo.c: In function ‘main’:
demo.c:13:5: warning: jump skips variable initialization [-Wjump-misses-init]
   13 |     goto cleanup;
      |     ^~~~
demo.c:18:2: note: label ‘cleanup’ defined here
   18 |  cleanup:
      |  ^~~~~~~
demo.c:16:24: note: ‘({anonymous})’ declared here
   16 |   demo = (struct demo) { .cmd = "foo" };
      |                        ^


This doesn't appear to be reported to GCC upstream yet

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]