Re: [Qemu-devel] [PATCH v2 3/3] tpm_tis: fix format string specifier in tpm_tis_show_buffer()

[Liam Merwick]

On 12/02/2019 13:27, Stefan Berger wrote:
> On 2/12/19 7:31 AM, Philippe Mathieu-Daudé wrote:
> > On 2/11/19 10:13 PM, Stefan Berger wrote:
> > > On 2/11/19 3:09 PM, Liam Merwick wrote:
> > > > On 11/02/2019 19:56, Stefan Berger wrote:
> > > > > On 2/11/19 11:02 AM, Philippe Mathieu-Daudé wrote:
> > > > > > On 2/11/19 4:03 PM, Liam Merwick wrote:
> > [...]
> > > > > > > -    printf("tpm_tis: %s length = %d\n", string, len);
> > > > > > > +    printf("tpm_tis: %s length = %u\n", string, len);
> > > > > > So here the format is '%zu'.
> > > > > > However in code cleanup we try go get ride of printf() calls and
> > > > > > replace them with trace points.
> > > > >
> > > > > This code is only used for debugging if DEBUG_TIS has been #defined.
> > > > > No need to add tracing here.
> > > > I'd come up the attached change (but that seems like overkill).
> > >
> > > I don't think we need tracing for this.
> > So if you think the code is mature enough, let's remove the DEBUG calls!
> >
> > Else we prefer to convert DEBUG printf to trace events because (at 
> > least):
> > - no need to recompile to enable debugging
> > - when compiled with debugging, you don't mess with STDIO which can be
> > used as a chardev backend.
> Fine. Then I withdraw my reviewed-by.


I don't see a way of removing the DEBUG calls without adding the 
overhead of a call to tpm_tis_show_buffer() each time even if tracing is 
not enabled (the 3 trace calls are interdependent and need a for loop).
Is there any example of a trace point that calls a function that then 
does non-trivial printing?

I could send a v3 with the patch I attached previously with the 3 printf 
calls changed to trace points but still wrapped in 'if (DEBUG_TIS)' and 
optimised out in non-debug.

Regards,
Liam