| Message ID | 1500645206-13559-6-git-send-email-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 07/21/2017 08:53 AM, Markus Armbruster wrote: > Leaving interpolation into JSON to qmp() is more robust than building > QMP input manually, as explained in the previous commit. > > The case in usb_test_hotplug() slightly more complicated: it s/()/() is/ > interpolates *into* JSON values. Clean it up by building the values > separately, so we can again leave interpolation to qmp(). > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > tests/libqos/usb.c | 30 ++++++++++++++---------------- > 1 file changed, 14 insertions(+), 16 deletions(-) > > diff --git a/tests/libqos/usb.c b/tests/libqos/usb.c > index 0cdfaec..f88d4a6 100644 > --- a/tests/libqos/usb.c > +++ b/tests/libqos/usb.c > @@ -40,18 +40,20 @@ void uhci_port_test(struct qhc *hc, int port, uint16_t expect) > void usb_test_hotplug(const char *hcd_id, const int port, > void (*port_check)(void)) > { > + char id[32]; > + char *bus; > QDict *response; > - char *cmd; > > - cmd = g_strdup_printf("{'execute': 'device_add'," > - " 'arguments': {" > - " 'driver': 'usb-tablet'," > - " 'port': '%d'," > - " 'bus': '%s.0'," > - " 'id': 'usbdev%d'" > - "}}", port, hcd_id, port); > - response = qmp(cmd); > - g_free(cmd); > + sprintf(id, "usbdev%d", port); I know this fits, but do any of our compilers issue dumb warnings about possible buffer overflow? I guess we'll deal with that if someone reports a problem. Reviewed-by: Eric Blake <eblake@redhat.com>
Eric Blake <eblake@redhat.com> writes: > On 07/21/2017 08:53 AM, Markus Armbruster wrote: >> Leaving interpolation into JSON to qmp() is more robust than building >> QMP input manually, as explained in the previous commit. >> >> The case in usb_test_hotplug() slightly more complicated: it > > s/()/() is/ Will fix. >> interpolates *into* JSON values. Clean it up by building the values >> separately, so we can again leave interpolation to qmp(). >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> tests/libqos/usb.c | 30 ++++++++++++++---------------- >> 1 file changed, 14 insertions(+), 16 deletions(-) >> >> diff --git a/tests/libqos/usb.c b/tests/libqos/usb.c >> index 0cdfaec..f88d4a6 100644 >> --- a/tests/libqos/usb.c >> +++ b/tests/libqos/usb.c >> @@ -40,18 +40,20 @@ void uhci_port_test(struct qhc *hc, int port, uint16_t expect) >> void usb_test_hotplug(const char *hcd_id, const int port, >> void (*port_check)(void)) >> { >> + char id[32]; >> + char *bus; >> QDict *response; >> - char *cmd; >> >> - cmd = g_strdup_printf("{'execute': 'device_add'," >> - " 'arguments': {" >> - " 'driver': 'usb-tablet'," >> - " 'port': '%d'," >> - " 'bus': '%s.0'," >> - " 'id': 'usbdev%d'" >> - "}}", port, hcd_id, port); >> - response = qmp(cmd); >> - g_free(cmd); >> + sprintf(id, "usbdev%d", port); > > I know this fits, but do any of our compilers issue dumb warnings about > possible buffer overflow? I guess we'll deal with that if someone > reports a problem. We do similar things elsewhere, just grep for sprintf. > Reviewed-by: Eric Blake <eblake@redhat.com> Thanks!
On Fri, Jul 21, 2017 at 03:53:22PM +0200, Markus Armbruster wrote: > Leaving interpolation into JSON to qmp() is more robust than building > QMP input manually, as explained in the previous commit. > > The case in usb_test_hotplug() slightly more complicated: it > interpolates *into* JSON values. Clean it up by building the values > separately, so we can again leave interpolation to qmp(). > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > tests/libqos/usb.c | 30 ++++++++++++++---------------- > 1 file changed, 14 insertions(+), 16 deletions(-) > > diff --git a/tests/libqos/usb.c b/tests/libqos/usb.c > index 0cdfaec..f88d4a6 100644 > --- a/tests/libqos/usb.c > +++ b/tests/libqos/usb.c > @@ -40,18 +40,20 @@ void uhci_port_test(struct qhc *hc, int port, uint16_t expect) > void usb_test_hotplug(const char *hcd_id, const int port, > void (*port_check)(void)) > { > + char id[32]; > + char *bus; > QDict *response; > - char *cmd; > > - cmd = g_strdup_printf("{'execute': 'device_add'," > - " 'arguments': {" > - " 'driver': 'usb-tablet'," > - " 'port': '%d'," > - " 'bus': '%s.0'," > - " 'id': 'usbdev%d'" > - "}}", port, hcd_id, port); > - response = qmp(cmd); > - g_free(cmd); > + sprintf(id, "usbdev%d", port); > + bus = g_strdup_printf("%s.0", hcd_id); > + response = qmp("{'execute': 'device_add'," > + " 'arguments': {" > + " 'driver': 'usb-tablet'," > + " 'port': %s," > + " 'bus': %s," > + " 'id': %s" > + " }}", id + 6, bus, id); Or: id + strlen("usbdev") Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
diff --git a/tests/libqos/usb.c b/tests/libqos/usb.c index 0cdfaec..f88d4a6 100644 --- a/tests/libqos/usb.c +++ b/tests/libqos/usb.c @@ -40,18 +40,20 @@ void uhci_port_test(struct qhc *hc, int port, uint16_t expect) void usb_test_hotplug(const char *hcd_id, const int port, void (*port_check)(void)) { + char id[32]; + char *bus; QDict *response; - char *cmd; - cmd = g_strdup_printf("{'execute': 'device_add'," - " 'arguments': {" - " 'driver': 'usb-tablet'," - " 'port': '%d'," - " 'bus': '%s.0'," - " 'id': 'usbdev%d'" - "}}", port, hcd_id, port); - response = qmp(cmd); - g_free(cmd); + sprintf(id, "usbdev%d", port); + bus = g_strdup_printf("%s.0", hcd_id); + response = qmp("{'execute': 'device_add'," + " 'arguments': {" + " 'driver': 'usb-tablet'," + " 'port': %s," + " 'bus': %s," + " 'id': %s" + " }}", id + 6, bus, id); + g_free(bus); g_assert(response); g_assert(!qdict_haskey(response, "error")); QDECREF(response); @@ -60,12 +62,8 @@ void usb_test_hotplug(const char *hcd_id, const int port, port_check(); } - cmd = g_strdup_printf("{'execute': 'device_del'," - " 'arguments': {" - " 'id': 'usbdev%d'" - "}}", port); - response = qmp(cmd); - g_free(cmd); + response = qmp("{'execute': 'device_del', 'arguments': { 'id': %s }}", + id); g_assert(response); g_assert(qdict_haskey(response, "event")); g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED"));
Leaving interpolation into JSON to qmp() is more robust than building QMP input manually, as explained in the previous commit. The case in usb_test_hotplug() slightly more complicated: it interpolates *into* JSON values. Clean it up by building the values separately, so we can again leave interpolation to qmp(). Signed-off-by: Markus Armbruster <armbru@redhat.com> --- tests/libqos/usb.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-)