Re: [PATCH 5/5] perf intel-pt: Support itrace option flag d+e to log on error

[Adrian Hunter]

On 2/09/22 04:34, Namhyung Kim wrote:
> On Thu, Sep 1, 2022 at 9:29 AM Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
>>
>> On 1/09/22 17:31, Andi Kleen wrote:
>>>
>>> On 9/1/2022 4:00 AM, Adrian Hunter wrote:
> 
> [SNIP]
>>>> +
>>>> +static void log_buf__dump(struct log_buf *b)
>>>> +{
>>>> +    if (!b->buf)
>>>> +        return;
>>>> +
>>>> +    fflush(f);
>>>> +    fprintf(b->backend, "Dumping debug log buffer (first line may be sliced)\n");
>>>
>>>
>>> Should be easy to skip the first line, no?
>>
>> Not as easy as typing " (first line may be sliced)" ;-)
>>
>> Still not sure it is worth having the extra complication, but here
>> is the change as a separate patch:
>>
>> From: Adrian Hunter <adrian.hunter@xxxxxxxxx>
>> Date: Thu, 1 Sep 2022 19:01:33 +0300
>> Subject: [PATCH] perf intel-pt: Remove first line of log dumped on error
>>
>> Instead of printing "(first line may be sliced)", always remove the
>> first line of the debug log when dumping on error.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
>> ---
>>  .../perf/util/intel-pt-decoder/intel-pt-log.c | 27 ++++++++++++++++---
>>  1 file changed, 24 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-log.c b/tools/perf/util/intel-pt-decoder/intel-pt-log.c
>> index ea96dcae187a7..6cc465d1f7a9e 100644
>> --- a/tools/perf/util/intel-pt-decoder/intel-pt-log.c
>> +++ b/tools/perf/util/intel-pt-decoder/intel-pt-log.c
>> @@ -143,16 +143,37 @@ static FILE *log_buf__open(struct log_buf *b, FILE *backend, unsigned int sz)
>>         return file;
>>  }
>>
>> +static bool remove_first_line(const char **p, size_t *n)
>> +{
>> +       for (; *n && **p != '\n'; ++*p, --*n)
>> +               ;
>> +       if (*n) {
>> +               *p += 1;
>> +               *n -= 1;
>> +               return true;
>> +       }
>> +       return false;
>> +}
>> +
>> +static void write_lines(const char *p, size_t n, FILE *fp, bool *remove_first)
>> +{
>> +       if (*remove_first)
>> +               *remove_first = !remove_first_line(&p, &n);
>> +       fwrite(p, n, 1, fp);
>> +}
>> +
>>  static void log_buf__dump(struct log_buf *b)
>>  {
>> +       bool remove_first = true;
> 
> Isn't it only required when the buf is wrapped?

Very true! Thanks for spotting that!

I will send a new version.

> 
> Thanks,
> Namhyung
> 
> 
>> +
>>         if (!b->buf)
>>                 return;
>>
>>         fflush(f);
>> -       fprintf(b->backend, "Dumping debug log buffer (first line may be sliced)\n");
>> +       fprintf(b->backend, "Dumping debug log buffer\n");
>>         if (b->wrapped)
>> -               fwrite(b->buf + b->head, b->buf_sz - b->head, 1, b->backend);
>> -       fwrite(b->buf, b->head, 1, b->backend);
>> +               write_lines(b->buf + b->head, b->buf_sz - b->head, b->backend, &remove_first);
>> +       write_lines(b->buf, b->head, b->backend, &remove_first);
>>         fprintf(b->backend, "End of debug log buffer dump\n");
>>
>>         b->head = 0;
>> --
>> 2.34.1
>>