| Message ID | 20201103151452.416784-4-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | deprecate short-form boolean options | expand |
On 03/11/2020 16.14, Paolo Bonzini wrote: > device-introspect-test uses HMP, so it should escape the device name > properly. Because of this, a few devices that had commas in their > names were escaping testing. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > tests/qtest/device-introspect-test.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/tests/qtest/device-introspect-test.c b/tests/qtest/device-introspect-test.c > index 9f22340ee5..f471b0e0dd 100644 > --- a/tests/qtest/device-introspect-test.c > +++ b/tests/qtest/device-introspect-test.c > @@ -104,7 +104,9 @@ static QList *device_type_list(QTestState *qts, bool abstract) > static void test_one_device(QTestState *qts, const char *type) > { > QDict *resp; > - char *help; > + g_autofree char *help; > + g_autofree GRegex *comma; > + g_autofree char *escaped; > > g_test_message("Testing device '%s'", type); > > @@ -113,8 +115,9 @@ static void test_one_device(QTestState *qts, const char *type) > type); > qobject_unref(resp); > > - help = qtest_hmp(qts, "device_add \"%s,help\"", type); > - g_free(help); > + comma = g_regex_new(",", 0, 0, NULL); > + escaped = g_regex_replace_literal(comma, type, -1, 0, ",,", 0, NULL); > + help = qtest_hmp(qts, "device_add \"%s,help\"", escaped); > } Having "help =" as final statement now, this looks somewhat weird at a first glance (until you look at the g_autofree at the beginning of the function). Maybe it's better to drop the help variable completely and just do: g_free(gtest_hmp(...)) ? Thomas
I will just drop autofree usage completely, also because valgrind showed that GRegex does not support it and apparently is leaked. Paolo Il mer 4 nov 2020, 08:44 Thomas Huth <thuth@redhat.com> ha scritto: > On 03/11/2020 16.14, Paolo Bonzini wrote: > > device-introspect-test uses HMP, so it should escape the device name > > properly. Because of this, a few devices that had commas in their > > names were escaping testing. > > > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > --- > > tests/qtest/device-introspect-test.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/tests/qtest/device-introspect-test.c > b/tests/qtest/device-introspect-test.c > > index 9f22340ee5..f471b0e0dd 100644 > > --- a/tests/qtest/device-introspect-test.c > > +++ b/tests/qtest/device-introspect-test.c > > @@ -104,7 +104,9 @@ static QList *device_type_list(QTestState *qts, bool > abstract) > > static void test_one_device(QTestState *qts, const char *type) > > { > > QDict *resp; > > - char *help; > > + g_autofree char *help; > > + g_autofree GRegex *comma; > > + g_autofree char *escaped; > > > > g_test_message("Testing device '%s'", type); > > > > @@ -113,8 +115,9 @@ static void test_one_device(QTestState *qts, const > char *type) > > type); > > qobject_unref(resp); > > > > - help = qtest_hmp(qts, "device_add \"%s,help\"", type); > > - g_free(help); > > + comma = g_regex_new(",", 0, 0, NULL); > > + escaped = g_regex_replace_literal(comma, type, -1, 0, ",,", 0, > NULL); > > + help = qtest_hmp(qts, "device_add \"%s,help\"", escaped); > > } > > Having "help =" as final statement now, this looks somewhat weird at a > first > glance (until you look at the g_autofree at the beginning of the function). > Maybe it's better to drop the help variable completely and just do: > g_free(gtest_hmp(...)) ? > > Thomas > >
Paolo Bonzini <pbonzini@redhat.com> writes: > device-introspect-test uses HMP, so it should escape the device name > properly. Because of this, a few devices that had commas in their > names were escaping testing. > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> $ git-grep '\.name *= *"[^"]*,' | cat hw/block/fdc.c: .name = "SUNW,fdtwo" Any others?
On 06/11/20 14:15, Markus Armbruster wrote: > Paolo Bonzini <pbonzini@redhat.com> writes: > >> device-introspect-test uses HMP, so it should escape the device name >> properly. Because of this, a few devices that had commas in their >> names were escaping testing. >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > $ git-grep '\.name *= *"[^"]*,' | cat > hw/block/fdc.c: .name = "SUNW,fdtwo" > > Any others? Not that I know, but this is a bug anyway. :) Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: > On 06/11/20 14:15, Markus Armbruster wrote: >> Paolo Bonzini <pbonzini@redhat.com> writes: >> >>> device-introspect-test uses HMP, so it should escape the device name >>> properly. Because of this, a few devices that had commas in their >>> names were escaping testing. >>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> >> $ git-grep '\.name *= *"[^"]*,' | cat >> hw/block/fdc.c: .name = "SUNW,fdtwo" >> Any others? > > Not that I know, but this is a bug anyway. :) Yes, but "a few devices" made me curious.
diff --git a/tests/qtest/device-introspect-test.c b/tests/qtest/device-introspect-test.c index 9f22340ee5..f471b0e0dd 100644 --- a/tests/qtest/device-introspect-test.c +++ b/tests/qtest/device-introspect-test.c @@ -104,7 +104,9 @@ static QList *device_type_list(QTestState *qts, bool abstract) static void test_one_device(QTestState *qts, const char *type) { QDict *resp; - char *help; + g_autofree char *help; + g_autofree GRegex *comma; + g_autofree char *escaped; g_test_message("Testing device '%s'", type); @@ -113,8 +115,9 @@ static void test_one_device(QTestState *qts, const char *type) type); qobject_unref(resp); - help = qtest_hmp(qts, "device_add \"%s,help\"", type); - g_free(help); + comma = g_regex_new(",", 0, 0, NULL); + escaped = g_regex_replace_literal(comma, type, -1, 0, ",,", 0, NULL); + help = qtest_hmp(qts, "device_add \"%s,help\"", escaped); } static void test_device_intro_list(void)
device-introspect-test uses HMP, so it should escape the device name properly. Because of this, a few devices that had commas in their names were escaping testing. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- tests/qtest/device-introspect-test.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)