diff mbox series

[v2,09/13] util/hexdump: Replace sprintf() by g_string_append_printf()

Message ID 20240411101550.99392-10-philmd@linaro.org
State New
Headers show
Series misc: Remove sprintf() due to macOS deprecation | expand

Commit Message

Philippe Mathieu-Daudé April 11, 2024, 10:15 a.m. UTC 
sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
resulting in painful developper experience.

Replace sprintf() by GString API in order to avoid:

  [426/1310] Compiling C object libqemuutil.a.p/util_hexdump.c.o
  util/hexdump.c:35:21: warning: 'sprintf' is deprecated:
    This function is provided for compatibility reasons only.
    Due to security concerns inherent in the design of sprintf(3),
    it is highly recommended that you use snprintf(3) instead.
    [-Wdeprecated-declarations]
            line += sprintf(line, " %02x", (unsigned char)buf[b + i]);
                    ^
  util/hexdump.c:37:21: warning: 'sprintf' is deprecated:
            line += sprintf(line, "   ");
                    ^
  2 warnings generated.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 util/hexdump.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

Comments

Philippe Mathieu-Daudé April 11, 2024, 8:43 p.m. UTC | #1 
On 11/4/24 12:15, Philippe Mathieu-Daudé wrote:
> sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
> resulting in painful developper experience.
> 
> Replace sprintf() by GString API in order to avoid:
> 
>    [426/1310] Compiling C object libqemuutil.a.p/util_hexdump.c.o
>    util/hexdump.c:35:21: warning: 'sprintf' is deprecated:
>      This function is provided for compatibility reasons only.
>      Due to security concerns inherent in the design of sprintf(3),
>      it is highly recommended that you use snprintf(3) instead.
>      [-Wdeprecated-declarations]
>              line += sprintf(line, " %02x", (unsigned char)buf[b + i]);
>                      ^
>    util/hexdump.c:37:21: warning: 'sprintf' is deprecated:
>              line += sprintf(line, "   ");
>                      ^
>    2 warnings generated.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   util/hexdump.c | 17 ++++++++---------
>   1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/util/hexdump.c b/util/hexdump.c
> index b6f70e93bb..2ec1171de3 100644
> --- a/util/hexdump.c
> +++ b/util/hexdump.c
> @@ -19,7 +19,7 @@
>   char *qemu_hexdump_line(const void *bufptr, unsigned offset,
>                           unsigned int len, bool ascii)
>   {
> -    char linebuf[QEMU_HEXDUMP_LINE_BYTES], *line = linebuf;
> +    g_autoptr(GString) gs = g_string_sized_new(QEMU_HEXDUMP_LINE_BYTES);
>       const char *buf = bufptr;
>       int i, c;
>   
> @@ -27,30 +27,29 @@ char *qemu_hexdump_line(const void *bufptr, unsigned offset,
>           len = QEMU_HEXDUMP_LINE_BYTES;
>       }
>   
> -    line += snprintf(line, 6, "%04x:", offset);
> +    g_string_append_printf(gs, "%04x:", offset);
>       for (i = 0; i < QEMU_HEXDUMP_LINE_BYTES; i++) {
>           if ((i % 4) == 0) {
> -            *line++ = ' ';
> +            g_string_append_c(gs, ' ');
>           }
>           if (i < len) {
> -            line += sprintf(line, " %02x", (unsigned char)buf[offset + i]);
> +            g_string_append_printf(gs, " %02x", (unsigned char)buf[offset + i]);

I find using g_string_append_printf() simpler than checking snprintf()
return value, and don't expect this function to be in hot path, but if
preferred I can try to not use the GString API.

>           } else {
> -            line += sprintf(line, "   ");
> +            g_string_append(gs, "   ");
>           }
>       }
Richard Henderson April 11, 2024, 8:50 p.m. UTC | #2 
On 4/11/24 13:43, Philippe Mathieu-Daudé wrote:
> On 11/4/24 12:15, Philippe Mathieu-Daudé wrote:
>> sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
>> resulting in painful developper experience.
>>
>> Replace sprintf() by GString API in order to avoid:
>>
>>    [426/1310] Compiling C object libqemuutil.a.p/util_hexdump.c.o
>>    util/hexdump.c:35:21: warning: 'sprintf' is deprecated:
>>      This function is provided for compatibility reasons only.
>>      Due to security concerns inherent in the design of sprintf(3),
>>      it is highly recommended that you use snprintf(3) instead.
>>      [-Wdeprecated-declarations]
>>              line += sprintf(line, " %02x", (unsigned char)buf[b + i]);
>>                      ^
>>    util/hexdump.c:37:21: warning: 'sprintf' is deprecated:
>>              line += sprintf(line, "   ");
>>                      ^
>>    2 warnings generated.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   util/hexdump.c | 17 ++++++++---------
>>   1 file changed, 8 insertions(+), 9 deletions(-)
>>
>> diff --git a/util/hexdump.c b/util/hexdump.c
>> index b6f70e93bb..2ec1171de3 100644
>> --- a/util/hexdump.c
>> +++ b/util/hexdump.c
>> @@ -19,7 +19,7 @@
>>   char *qemu_hexdump_line(const void *bufptr, unsigned offset,
>>                           unsigned int len, bool ascii)
>>   {
>> -    char linebuf[QEMU_HEXDUMP_LINE_BYTES], *line = linebuf;
>> +    g_autoptr(GString) gs = g_string_sized_new(QEMU_HEXDUMP_LINE_BYTES);
>>       const char *buf = bufptr;
>>       int i, c;
>> @@ -27,30 +27,29 @@ char *qemu_hexdump_line(const void *bufptr, unsigned offset,
>>           len = QEMU_HEXDUMP_LINE_BYTES;
>>       }
>> -    line += snprintf(line, 6, "%04x:", offset);
>> +    g_string_append_printf(gs, "%04x:", offset);
>>       for (i = 0; i < QEMU_HEXDUMP_LINE_BYTES; i++) {
>>           if ((i % 4) == 0) {
>> -            *line++ = ' ';
>> +            g_string_append_c(gs, ' ');
>>           }
>>           if (i < len) {
>> -            line += sprintf(line, " %02x", (unsigned char)buf[offset + i]);
>> +            g_string_append_printf(gs, " %02x", (unsigned char)buf[offset + i]);
> 
> I find using g_string_append_printf() simpler than checking snprintf()
> return value, and don't expect this function to be in hot path, but if
> preferred I can try to not use the GString API.

GString api is pretty good.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~
diff mbox series

Patch

diff --git a/util/hexdump.c b/util/hexdump.c
index b6f70e93bb..2ec1171de3 100644
--- a/util/hexdump.c
+++ b/util/hexdump.c
@@ -19,7 +19,7 @@ 
 char *qemu_hexdump_line(const void *bufptr, unsigned offset,
                         unsigned int len, bool ascii)
 {
-    char linebuf[QEMU_HEXDUMP_LINE_BYTES], *line = linebuf;
+    g_autoptr(GString) gs = g_string_sized_new(QEMU_HEXDUMP_LINE_BYTES);
     const char *buf = bufptr;
     int i, c;
 
@@ -27,30 +27,29 @@  char *qemu_hexdump_line(const void *bufptr, unsigned offset,
         len = QEMU_HEXDUMP_LINE_BYTES;
     }
 
-    line += snprintf(line, 6, "%04x:", offset);
+    g_string_append_printf(gs, "%04x:", offset);
     for (i = 0; i < QEMU_HEXDUMP_LINE_BYTES; i++) {
         if ((i % 4) == 0) {
-            *line++ = ' ';
+            g_string_append_c(gs, ' ');
         }
         if (i < len) {
-            line += sprintf(line, " %02x", (unsigned char)buf[offset + i]);
+            g_string_append_printf(gs, " %02x", (unsigned char)buf[offset + i]);
         } else {
-            line += sprintf(line, "   ");
+            g_string_append(gs, "   ");
         }
     }
     if (ascii) {
-        *line++ = ' ';
+        g_string_append_c(gs, ' ');
         for (i = 0; i < len; i++) {
             c = buf[offset + i];
             if (c < ' ' || c > '~') {
                 c = '.';
             }
-            *line++ = c;
+            g_string_append_c(gs, c);
         }
     }
-    *line = '\0';
 
-    return g_strdup(linebuf);
+    return g_strdup(gs->str);
 }
 
 void qemu_hexdump(FILE *fp, const char *prefix,