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

[Liam Merwick <liam.merwick@oracle.com>]

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