diff mbox

[9/9] tests/libqtest: Enable compile-time format string checking

Message ID 1500645206-13559-10-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster July 21, 2017, 1:53 p.m. UTC 
qtest_qmp() & friends pass their format string and variable arguments
to qobject_from_jsonv().  Unlike qobject_from_jsonv(), they aren't
decorated with GCC_FMT_ATTR().  Fix that to get compile-time format
string checking.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/libqtest.h | 40 ++++++++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 16 deletions(-)

Comments

Eric Blake July 21, 2017, 3:04 p.m. UTC | #1 
On 07/21/2017 08:53 AM, Markus Armbruster wrote:
> qtest_qmp() & friends pass their format string and variable arguments
> to qobject_from_jsonv().  Unlike qobject_from_jsonv(), they aren't
> decorated with GCC_FMT_ATTR().  Fix that to get compile-time format
> string checking.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  tests/libqtest.h | 40 ++++++++++++++++++++++++----------------
>  1 file changed, 24 insertions(+), 16 deletions(-)

Don't know how much of this to merge with my fixed version of 2/9.  But
matches what I had locally after my version 1, before I ripped it all
back out again prior to posting my v2 when dealing with gcc fallout,
that you've now corrected.

> @@ -65,7 +66,8 @@ void qtest_qmp_discard_response(QTestState *s, const char *fmt, ...);
>   *
>   * Sends a QMP message to QEMU and returns the response.
>   */
> -QDict *qtest_qmp(QTestState *s, const char *fmt, ...);
> +QDict *qtest_qmp(QTestState *s, const char *fmt, ...)
> +    GCC_FMT_ATTR(2, 3);

This would fit on one line; any reason we need the line wrap?
Stefan Hajnoczi July 25, 2017, 2:09 p.m. UTC | #2 
On Fri, Jul 21, 2017 at 03:53:26PM +0200, Markus Armbruster wrote:
> qtest_qmp() & friends pass their format string and variable arguments
> to qobject_from_jsonv().  Unlike qobject_from_jsonv(), they aren't
> decorated with GCC_FMT_ATTR().  Fix that to get compile-time format
> string checking.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  tests/libqtest.h | 40 ++++++++++++++++++++++++----------------
>  1 file changed, 24 insertions(+), 16 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Markus Armbruster July 27, 2017, 8:36 a.m. UTC | #3 
Eric Blake <eblake@redhat.com> writes:

> On 07/21/2017 08:53 AM, Markus Armbruster wrote:
>> qtest_qmp() & friends pass their format string and variable arguments
>> to qobject_from_jsonv().  Unlike qobject_from_jsonv(), they aren't
>> decorated with GCC_FMT_ATTR().  Fix that to get compile-time format
>> string checking.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  tests/libqtest.h | 40 ++++++++++++++++++++++++----------------
>>  1 file changed, 24 insertions(+), 16 deletions(-)
>
> Don't know how much of this to merge with my fixed version of 2/9.  But
> matches what I had locally after my version 1, before I ripped it all
> back out again prior to posting my v2 when dealing with gcc fallout,
> that you've now corrected.
>
>> @@ -65,7 +66,8 @@ void qtest_qmp_discard_response(QTestState *s, const char *fmt, ...);
>>   *
>>   * Sends a QMP message to QEMU and returns the response.
>>   */
>> -QDict *qtest_qmp(QTestState *s, const char *fmt, ...);
>> +QDict *qtest_qmp(QTestState *s, const char *fmt, ...)
>> +    GCC_FMT_ATTR(2, 3);
>
> This would fit on one line; any reason we need the line wrap?

Pretty sure I didn't have reasons.  But I can make some up after the
fact!  Putting the attribute on a separate line can be a bit more
legible.  It can also confuse simple tools to recognize a definition
instead of a declaration.  Anyway, I don't really care about this line
break.
diff mbox

Patch

diff --git a/tests/libqtest.h b/tests/libqtest.h
index ae57282..435012d 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -55,7 +55,8 @@  void qtest_quit(QTestState *s);
  *
  * Sends a QMP message to QEMU and consumes the response.
  */
-void qtest_qmp_discard_response(QTestState *s, const char *fmt, ...);
+void qtest_qmp_discard_response(QTestState *s, const char *fmt, ...)
+    GCC_FMT_ATTR(2, 3);
 
 /**
  * qtest_qmp:
@@ -65,7 +66,8 @@  void qtest_qmp_discard_response(QTestState *s, const char *fmt, ...);
  *
  * Sends a QMP message to QEMU and returns the response.
  */
-QDict *qtest_qmp(QTestState *s, const char *fmt, ...);
+QDict *qtest_qmp(QTestState *s, const char *fmt, ...)
+    GCC_FMT_ATTR(2, 3);
 
 /**
  * qtest_async_qmp:
@@ -75,7 +77,8 @@  QDict *qtest_qmp(QTestState *s, const char *fmt, ...);
  *
  * Sends a QMP message to QEMU and leaves the response in the stream.
  */
-void qtest_async_qmp(QTestState *s, const char *fmt, ...);
+void qtest_async_qmp(QTestState *s, const char *fmt, ...)
+    GCC_FMT_ATTR(2, 3);
 
 /**
  * qtest_qmpv_discard_response:
@@ -85,7 +88,8 @@  void qtest_async_qmp(QTestState *s, const char *fmt, ...);
  *
  * Sends a QMP message to QEMU and consumes the response.
  */
-void qtest_qmpv_discard_response(QTestState *s, const char *fmt, va_list ap);
+void qtest_qmpv_discard_response(QTestState *s, const char *fmt, va_list ap)
+    GCC_FMT_ATTR(2, 0);
 
 /**
  * qtest_qmpv:
@@ -95,7 +99,8 @@  void qtest_qmpv_discard_response(QTestState *s, const char *fmt, va_list ap);
  *
  * Sends a QMP message to QEMU and returns the response.
  */
-QDict *qtest_qmpv(QTestState *s, const char *fmt, va_list ap);
+QDict *qtest_qmpv(QTestState *s, const char *fmt, va_list ap)
+    GCC_FMT_ATTR(2, 0);
 
 /**
  * qtest_async_qmpv:
@@ -105,7 +110,8 @@  QDict *qtest_qmpv(QTestState *s, const char *fmt, va_list ap);
  *
  * Sends a QMP message to QEMU and leaves the response in the stream.
  */
-void qtest_async_qmpv(QTestState *s, const char *fmt, va_list ap);
+void qtest_async_qmpv(QTestState *s, const char *fmt, va_list ap)
+    GCC_FMT_ATTR(2, 0);
 
 /**
  * qtest_receive:
@@ -144,7 +150,8 @@  QDict *qtest_qmp_eventwait_ref(QTestState *s, const char *event);
  *
  * Returns: the command's output.  The caller should g_free() it.
  */
-char *qtest_hmp(QTestState *s, const char *fmt, ...);
+char *qtest_hmp(QTestState *s, const char *fmt, ...)
+    GCC_FMT_ATTR(2, 3);
 
 /**
  * qtest_hmpv:
@@ -157,7 +164,8 @@  char *qtest_hmp(QTestState *s, const char *fmt, ...);
  *
  * Returns: the command's output.  The caller should g_free() it.
  */
-char *qtest_hmpv(QTestState *s, const char *fmt, va_list ap);
+char *qtest_hmpv(QTestState *s, const char *fmt, va_list ap)
+    GCC_FMT_ATTR(2, 0);
 
 /**
  * qtest_get_irq:
@@ -543,7 +551,7 @@  static inline void qtest_end(void)
  *
  * Sends a QMP message to QEMU and returns the response.
  */
-QDict *qmp(const char *fmt, ...);
+QDict *qmp(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 
 /**
  * qmp_async:
@@ -552,7 +560,7 @@  QDict *qmp(const char *fmt, ...);
  *
  * Sends a QMP message to QEMU and leaves the response in the stream.
  */
-void qmp_async(const char *fmt, ...);
+void qmp_async(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 
 /**
  * qmp_discard_response:
@@ -561,7 +569,7 @@  void qmp_async(const char *fmt, ...);
  *
  * Sends a QMP message to QEMU and consumes the response.
  */
-void qmp_discard_response(const char *fmt, ...);
+void qmp_discard_response(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 
 /**
  * qmp_receive:
@@ -604,7 +612,7 @@  static inline QDict *qmp_eventwait_ref(const char *event)
  *
  * Returns: the command's output.  The caller should g_free() it.
  */
-char *hmp(const char *fmt, ...);
+char *hmp(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 
 /**
  * get_irq:
@@ -920,10 +928,10 @@  static inline int64_t clock_set(int64_t val)
 }
 
 QDict *qmp_fd_receive(int fd);
-void qmp_fd_sendv(int fd, const char *fmt, va_list ap);
-void qmp_fd_send(int fd, const char *fmt, ...);
-QDict *qmp_fdv(int fd, const char *fmt, va_list ap);
-QDict *qmp_fd(int fd, const char *fmt, ...);
+void qmp_fd_sendv(int fd, const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0);
+void qmp_fd_send(int fd, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
+QDict *qmp_fdv(int fd, const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0);
+QDict *qmp_fd(int fd, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
 
 /**
  * qtest_cb_for_every_machine: