Message ID | 20190405184037.16799-1-dgilbert@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | test qgraph.c: Fix segs due to out of scope default | expand |
On 4/5/19 1:40 PM, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > The test uses the trick: > if (!opts) { > opts = &(QOSGraph...Options) { }; > } > > in a couple of places, however the temporary created > by the &() {} goes out of scope at the bottom of the if, > and results in a seg or assert when opts-> fields are > used (on fedora 30's gcc 9). > > Fixes: fc281c802022cb3a73a5 > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > tests/libqos/qgraph.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) Fixing it for 4.0 would be nice (we can't promise that everything in 4.0 is clean for gcc 9, but as long as we have to do -rc3 anyways, fixing undefined behavior into something well-defined like this seems reasonable) Reviewed-by: Eric Blake <eblake@redhat.com>
On 05/04/19 20:40, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > The test uses the trick: > if (!opts) { > opts = &(QOSGraph...Options) { }; > } > > in a couple of places, however the temporary created > by the &() {} goes out of scope at the bottom of the if, > and results in a seg or assert when opts-> fields are > used (on fedora 30's gcc 9). > > Fixes: fc281c802022cb3a73a5 > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Thomas, can you pick this up? Paolo > --- > tests/libqos/qgraph.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/tests/libqos/qgraph.c b/tests/libqos/qgraph.c > index 122efc1b7b..b149caaaa9 100644 > --- a/tests/libqos/qgraph.c > +++ b/tests/libqos/qgraph.c > @@ -77,6 +77,7 @@ static void add_edge(const char *source, const char *dest, > { > char *key; > QOSGraphEdgeList *list = g_hash_table_lookup(edge_table, source); > + QOSGraphEdgeOptions def_opts = { }; > > if (!list) { > list = g_new0(QOSGraphEdgeList, 1); > @@ -85,7 +86,7 @@ static void add_edge(const char *source, const char *dest, > } > > if (!opts) { > - opts = &(QOSGraphEdgeOptions) { }; > + opts = &def_opts; > } > > QOSGraphEdge *edge = g_new0(QOSGraphEdge, 1); > @@ -590,9 +591,10 @@ void qos_add_test(const char *name, const char *interface, > { > QOSGraphNode *node; > char *test_name = g_strdup_printf("%s-tests/%s", interface, name);; > + QOSGraphTestOptions def_opts = { }; > > if (!opts) { > - opts = &(QOSGraphTestOptions) { }; > + opts = &def_opts; > } > node = create_node(test_name, QNODE_TEST); > node->u.test.function = test_func; >
On 05/04/2019 23.16, Paolo Bonzini wrote: > On 05/04/19 20:40, Dr. David Alan Gilbert (git) wrote: >> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> >> >> The test uses the trick: >> if (!opts) { >> opts = &(QOSGraph...Options) { }; >> } >> >> in a couple of places, however the temporary created >> by the &() {} goes out of scope at the bottom of the if, >> and results in a seg or assert when opts-> fields are >> used (on fedora 30's gcc 9). >> >> Fixes: fc281c802022cb3a73a5 >> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > Thomas, can you pick this up? Sure, queued it to my qtest-next branch now: https://gitlab.com/huth/qemu/tree/qtest-next I'll send a PULL request on Monday. Thomas
diff --git a/tests/libqos/qgraph.c b/tests/libqos/qgraph.c index 122efc1b7b..b149caaaa9 100644 --- a/tests/libqos/qgraph.c +++ b/tests/libqos/qgraph.c @@ -77,6 +77,7 @@ static void add_edge(const char *source, const char *dest, { char *key; QOSGraphEdgeList *list = g_hash_table_lookup(edge_table, source); + QOSGraphEdgeOptions def_opts = { }; if (!list) { list = g_new0(QOSGraphEdgeList, 1); @@ -85,7 +86,7 @@ static void add_edge(const char *source, const char *dest, } if (!opts) { - opts = &(QOSGraphEdgeOptions) { }; + opts = &def_opts; } QOSGraphEdge *edge = g_new0(QOSGraphEdge, 1); @@ -590,9 +591,10 @@ void qos_add_test(const char *name, const char *interface, { QOSGraphNode *node; char *test_name = g_strdup_printf("%s-tests/%s", interface, name);; + QOSGraphTestOptions def_opts = { }; if (!opts) { - opts = &(QOSGraphTestOptions) { }; + opts = &def_opts; } node = create_node(test_name, QNODE_TEST); node->u.test.function = test_func;