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

[Richard Henderson]

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~