RE: [PATCH 1/1] tracing: Support reading trace event format file larger than PAGE_SIZE

[Shiju Jose <shiju.jose@huawei.com>]

>-----Original Message-----
>From: Steven Rostedt <rostedt@goodmis.org>
>Sent: 07 January 2025 23:02
>To: Shiju Jose <shiju.jose@huawei.com>
>Cc: mhiramat@kernel.org; mathieu.desnoyers@efficios.com; linux-trace-
>kernel@vger.kernel.org; linux-kernel@vger.kernel.org; Linuxarm
><linuxarm@huawei.com>; Jonathan Cameron
><jonathan.cameron@huawei.com>; tanxiaofei <tanxiaofei@huawei.com>;
>Zengtao (B) <prime.zeng@hisilicon.com>
>Subject: Re: [PATCH 1/1] tracing: Support reading trace event format file larger
>than PAGE_SIZE
>
>On Tue, 7 Jan 2025 11:04:57 +0000
>Shiju Jose <shiju.jose@huawei.com> wrote:
>
>> Please find attached format file, which might have failed to parse
>> because of the unsupported formats in the libtraceevent you mentioned in the
>following thread.
>>
>https://lore.kernel.org/lkml/20241127104132.6c1729e1@gandalf.local.home/#t
>>
>> Please find the improvement in the libraceevent which I mentioned for
>> not returning error when parsing failed,
>> ===========================================
>> diff --git a/src/event-parse.c b/src/event-parse.c index
>> 0427061..b9264cb 100644
>> --- a/src/event-parse.c
>> +++ b/src/event-parse.c
>> @@ -7905,9 +7905,14 @@ __parse_event(struct tep_handle *tep,
>>               const char *buf, unsigned long size,
>>               const char *sys)
>>  {
>> -       int ret = parse_format(eventp, tep, buf, size, sys);
>> -       struct tep_event *event = *eventp;
>> +       int ret;
>> +       struct tep_event *event;
>> +
>> +       ret = parse_format(eventp, tep, buf, size, sys);
>> +       if (ret)
>> +               return ret;
>>
>> +       event = *eventp;
>>         if (event == NULL)
>>                 return ret;
>
>Actually, this is the proper patch:
>
>diff --git a/src/event-parse.c b/src/event-parse.c index
>33ed7fb47fff..f2e50b0e8992 100644
>--- a/src/event-parse.c
>+++ b/src/event-parse.c
>@@ -7841,7 +7841,7 @@ static enum tep_errno parse_format(struct tep_event
>**eventp,
> 	ret = event_read_format(event);
> 	if (ret < 0) {
> 		ret = TEP_ERRNO__READ_FORMAT_FAILED;
>-		goto event_parse_failed;
>+		goto event_alloc_failed;
> 	}
>
> 	/*
>
>
>As it's OK if it returns that it failed to parse the print format, as it can still read
>the event. But if it fails to read the fields, then it is basically useless. 
Thanks.
Your patch worked fine for returning error to the user space tool for the incomplete format data case as
the following check become true in the __parse_event()  after freeing 'event' in the goto event_alloc_failed.
 if (event == NULL)
     return ret;
>
>-- Steve

Thanks,
Shiju