Re: [PATCH v3 03/27] util/hexdump: Use a GString for qemu_hexdump_line

[Philippe Mathieu-Daudé]

On 12/4/24 20:59, Richard Henderson wrote:
> On 4/12/24 10:41, Philippe Mathieu-Daudé wrote:
> > > -void qemu_hexdump_line(char *line, const void *bufptr, size_t len)
> > > +GString *qemu_hexdump_line(GString *str, const void *vbuf, size_t len)
> > >   {
> > > -    const char *buf = bufptr;
> > > -    int i, c;
> > > +    const uint8_t *buf = vbuf;
> > > +    size_t i;
> > > -    if (len > QEMU_HEXDUMP_LINE_BYTES) {
> > > -        len = QEMU_HEXDUMP_LINE_BYTES;
> > > +    if (str == NULL) {
> > > +        /* Estimate the length of the output to avoid reallocs. */
> > > +        i = len * 3 + len / 4;
> > > +        str = g_string_sized_new(i + 1);
> > >       }
> >
> > [*]
> >           else {
> >             g_string_truncate(str, 0);
> >           }
> ...
> > > @@ -49,24 +52,26 @@ static void asciidump_line(char *line, const void 
> > > *bufptr, size_t len)
> > >       *line = '\0';
> > >   }
> > > +#define QEMU_HEXDUMP_LINE_BYTES 16
> > >   #define QEMU_HEXDUMP_LINE_WIDTH \
> > >       (QEMU_HEXDUMP_LINE_BYTES * 2 + QEMU_HEXDUMP_LINE_BYTES / 4)
> > >   void qemu_hexdump(FILE *fp, const char *prefix,
> > >                     const void *bufptr, size_t size)
> > >   {
> > > -    char line[QEMU_HEXDUMP_LINE_LEN];
> > > +    g_autoptr(GString) str = 
> > > g_string_sized_new(QEMU_HEXDUMP_LINE_WIDTH + 1);
> > >       char ascii[QEMU_HEXDUMP_LINE_BYTES + 1];
> > >       size_t b, len;
> > >       for (b = 0; b < size; b += len) {
> > >           len = MIN(size - b, QEMU_HEXDUMP_LINE_BYTES);
> > > -        qemu_hexdump_line(line, bufptr + b, len);
> > > +        g_string_truncate(str, 0);
> >
> > Shouldn't we truncate in [*] ?
>
> The usage in tpm puts several lines together in one string,
> adding \n in between, for output in one go.

I see the trace_tpm_util_show_buffer() call. However this
isn't a recommended use of the tracing API (Cc'ing Stefan).
It breaks the "log" backend output, and is sub-optimal for
all other backends.

IMHO the TPM buffer should be traced by multiple calls of
(offset, hexbuf) instead.

Regards,

Phil.