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

[Philippe Mathieu-Daudé]

On 2/12/19 2:43 PM, Liam Merwick wrote:
> 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.

You can look at commit 56853498648.

Thanks!

Phil.