[PATCH] tools/libxc: Annotate xc_report_error with __attribute__((format))

[PATCH] tools/libxc: Annotate xc_report_error with __attribute__((format))

[Andrew Cooper]

This helps the compiler spot printf formatting errors.

Fix up all errors discovered.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 tools/libxc/xc_core.c           |    2 +-
 tools/libxc/xc_domain_restore.c |    6 +++---
 tools/libxc/xc_gnttab.c         |    2 +-
 tools/libxc/xc_private.h        |    3 ++-
 4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/tools/libxc/xc_core.c b/tools/libxc/xc_core.c
index 4bc1abb..dfa424b 100644
--- a/tools/libxc/xc_core.c
+++ b/tools/libxc/xc_core.c
@@ -497,7 +497,7 @@ xc_domain_dumpcore_via_callback(xc_interface *xch,
     ctxt = calloc(sizeof(*ctxt), info.max_vcpu_id + 1);
     if ( !ctxt )
     {
-        PERROR("Could not allocate vcpu context array", domid);
+        PERROR("Could not allocate vcpu context array");
         goto out;
     }
 
diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c
index 89af1ad..d950639 100644
--- a/tools/libxc/xc_domain_restore.c
+++ b/tools/libxc/xc_domain_restore.c
@@ -102,7 +102,7 @@ static ssize_t rdexact(xc_interface *xch, struct restore_ctx *ctx,
             errno = 0;
         }
         if ( len <= 0 ) {
-            ERROR("%s failed (read rc: %d, errno: %d)", __func__, len, errno);
+            ERROR("%s failed (read rc: %ld, errno: %d)", __func__, (long)len, errno);
             return -1;
         }
         offset += len;
@@ -443,7 +443,7 @@ static int compat_buffer_qemu(xc_interface *xch, struct restore_ctx *ctx,
     }
 
     if ( memcmp(qbuf, "QEVM", 4) ) {
-        ERROR("Invalid QEMU magic: 0x%08x", *(unsigned long*)qbuf);
+        ERROR("Invalid QEMU magic: 0x%08x", *(uint32_t*)qbuf);
         free(qbuf);
         return -1;
     }
@@ -1802,7 +1802,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
     } else if (pagebuf.acpi_ioport_location == 0) {
         DBGPRINTF("Use old firmware ioport from the checkpoint\n");
     } else {
-        ERROR("Error, unknow acpi ioport location (%i)", pagebuf.acpi_ioport_location);
+        ERROR("Error, unknow acpi ioport location (%"PRId64")", pagebuf.acpi_ioport_location);
     }
 
     tdatatmp = tdata;
diff --git a/tools/libxc/xc_gnttab.c b/tools/libxc/xc_gnttab.c
index 3b6058c..4076e47 100644
--- a/tools/libxc/xc_gnttab.c
+++ b/tools/libxc/xc_gnttab.c
@@ -75,7 +75,7 @@ static void *_gnttab_map_table(xc_interface *xch, int domid, int *gnt_num)
 
     if ( rc || (query.status != GNTST_okay) )
     {
-        ERROR("Could not query dom's grant size\n", domid);
+        ERROR("Could not query dom%d's grant size\n", domid);
         return NULL;
     }
 
diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h
index 670a82d..29e926c 100644
--- a/tools/libxc/xc_private.h
+++ b/tools/libxc/xc_private.h
@@ -93,7 +93,8 @@ struct xc_interface_core {
     xc_osdep_handle  ops_handle; /* opaque data for xc_osdep_ops */
 };
 
-void xc_report_error(xc_interface *xch, int code, const char *fmt, ...);
+void xc_report_error(xc_interface *xch, int code, const char *fmt, ...)
+    __attribute__((format(printf,3,4)));
 void xc_reportv(xc_interface *xch, xentoollog_logger *lg, xentoollog_level,
                 int code, const char *fmt, va_list args)
      __attribute__((format(printf,5,0)));
-- 
1.7.10.4

Re: [PATCH] tools/libxc: Annotate xc_report_error with __attribute__((format))

[Ian Campbell]

On Tue, 2014-06-03 at 18:45 +0100, Andrew Cooper wrote:
> diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c
> index 89af1ad..d950639 100644
> --- a/tools/libxc/xc_domain_restore.c
> +++ b/tools/libxc/xc_domain_restore.c
> @@ -102,7 +102,7 @@ static ssize_t rdexact(xc_interface *xch, struct restore_ctx *ctx,
>              errno = 0;
>          }
>          if ( len <= 0 ) {
> -            ERROR("%s failed (read rc: %d, errno: %d)", __func__, len, errno);
> +            ERROR("%s failed (read rc: %ld, errno: %d)", __func__, (long)len, errno);

The correct way to print an ssize_t is %zd.

>              return -1;
>          }
>          offset += len;
> @@ -443,7 +443,7 @@ static int compat_buffer_qemu(xc_interface *xch, struct restore_ctx *ctx,
>      }
>  
>      if ( memcmp(qbuf, "QEVM", 4) ) {
> -        ERROR("Invalid QEMU magic: 0x%08x", *(unsigned long*)qbuf);
> +        ERROR("Invalid QEMU magic: 0x%08x", *(uint32_t*)qbuf);

0x%08lx with unsigned long or 0x%PRIx32 with uint32_t, not some hybrid,
please. (I thin %..lx is the right answer here)

>          free(qbuf);
>          return -1;
>      }

Ian.

Re: [PATCH] tools/libxc: Annotate xc_report_error with __attribute__((format))

[Andrew Cooper]

On 05/06/14 14:11, Ian Campbell wrote:
> On Tue, 2014-06-03 at 18:45 +0100, Andrew Cooper wrote:
>> diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c
>> index 89af1ad..d950639 100644
>> --- a/tools/libxc/xc_domain_restore.c
>> +++ b/tools/libxc/xc_domain_restore.c
>> @@ -102,7 +102,7 @@ static ssize_t rdexact(xc_interface *xch, struct restore_ctx *ctx,
>>              errno = 0;
>>          }
>>          if ( len <= 0 ) {
>> -            ERROR("%s failed (read rc: %d, errno: %d)", __func__, len, errno);
>> +            ERROR("%s failed (read rc: %ld, errno: %d)", __func__, (long)len, errno);
> The correct way to print an ssize_t is %zd.

Odd - I thought I tried that and it failed, but clearly I failed to try
it correctly.

>
>>              return -1;
>>          }
>>          offset += len;
>> @@ -443,7 +443,7 @@ static int compat_buffer_qemu(xc_interface *xch, struct restore_ctx *ctx,
>>      }
>>  
>>      if ( memcmp(qbuf, "QEVM", 4) ) {
>> -        ERROR("Invalid QEMU magic: 0x%08x", *(unsigned long*)qbuf);
>> +        ERROR("Invalid QEMU magic: 0x%08x", *(uint32_t*)qbuf);
> 0x%08lx with unsigned long or 0x%PRIx32 with uint32_t, not some hybrid,
> please. (I thin %..lx is the right answer here)

qbuf is strictly 4 bytes, as identified in the line above.  PRIx32 it
is.  (I remember deciding on PRIx32 and clearly failed to implement it
properly)

v2 on its way.

~Andrew