diff mbox

[v2,3/4] qtest: Avoid passing raw strings through hmp()

Message ID 20170720214008.28494-4-eblake@redhat.com
State New
Headers show

Commit Message

Eric Blake July 20, 2017, 9:40 p.m. UTC
The next patch will add __attribute__((__format__)) to hmp(), which
in turn causes gcc to warn about non-literal format strings.  Rather
than risk an arbitrary string containing % being mis-handled, always
pass variable strings along with a %s format.  It also makes it
easier to prove correctness locally, rather than auditing all the
source strings.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 tests/test-hmp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Markus Armbruster July 21, 2017, 1:50 p.m. UTC | #1
Eric Blake <eblake@redhat.com> writes:

> The next patch will add __attribute__((__format__)) to hmp(), which

It did in v1, but no more.  Intentional?

> in turn causes gcc to warn about non-literal format strings.  Rather
> than risk an arbitrary string containing % being mis-handled, always
> pass variable strings along with a %s format.  It also makes it
> easier to prove correctness locally, rather than auditing all the
> source strings.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> ---
>  tests/test-hmp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/test-hmp.c b/tests/test-hmp.c
> index d77b3c8710..0af066487c 100644
> --- a/tests/test-hmp.c
> +++ b/tests/test-hmp.c
> @@ -80,7 +80,7 @@ static void test_commands(void)
>          if (verbose) {
>              fprintf(stderr, "\t%s\n", hmp_cmds[i]);
>          }
> -        response = hmp(hmp_cmds[i]);
> +        response = hmp("%s", hmp_cmds[i]);
>          g_free(response);
>      }
>
> @@ -103,7 +103,7 @@ static void test_info_commands(void)
>          if (verbose) {
>              fprintf(stderr, "\t%s\n", info);
>          }
> -        resp = hmp(info);
> +        resp = hmp("%s", info);
>          g_free(resp);
>          /* And move forward to the next line */
>          info = strchr(endp + 1, '\n');
Eric Blake July 21, 2017, 2:04 p.m. UTC | #2
On 07/21/2017 08:50 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> The next patch will add __attribute__((__format__)) to hmp(), which
> 
> It did in v1, but no more.  Intentional?

Oh shoot.  I got so busy deleting the attribute for qmp() and friends
that I also deleted it for hmp().  I'm not having a very good run at
this, am I?
Markus Armbruster July 21, 2017, 2:32 p.m. UTC | #3
Eric Blake <eblake@redhat.com> writes:

> On 07/21/2017 08:50 AM, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> writes:
>> 
>>> The next patch will add __attribute__((__format__)) to hmp(), which
>> 
>> It did in v1, but no more.  Intentional?
>
> Oh shoot.  I got so busy deleting the attribute for qmp() and friends
> that I also deleted it for hmp().  I'm not having a very good run at
> this, am I?

I've done worse :)
diff mbox

Patch

diff --git a/tests/test-hmp.c b/tests/test-hmp.c
index d77b3c8710..0af066487c 100644
--- a/tests/test-hmp.c
+++ b/tests/test-hmp.c
@@ -80,7 +80,7 @@  static void test_commands(void)
         if (verbose) {
             fprintf(stderr, "\t%s\n", hmp_cmds[i]);
         }
-        response = hmp(hmp_cmds[i]);
+        response = hmp("%s", hmp_cmds[i]);
         g_free(response);
     }

@@ -103,7 +103,7 @@  static void test_info_commands(void)
         if (verbose) {
             fprintf(stderr, "\t%s\n", info);
         }
-        resp = hmp(info);
+        resp = hmp("%s", info);
         g_free(resp);
         /* And move forward to the next line */
         info = strchr(endp + 1, '\n');