Re: [PATCH] util: constant -1 with expression of type char and allocation failure handling

From: Yunseong Kim
Date: Tue Jun 25 2024 - 01:23:09 EST


Hi Namhyung,

On 6/25/24 2:00 오후, Namhyung Kim wrote:
> Hello,
>
> On Thu, Jun 20, 2024 at 04:10:53AM +0900, Yunseong Kim wrote:
>> Hi Ian,
>>
>> On 6/20/24 4:03 오전, Ian Rogers wrote:
>>>
>>>
>>> On Wed, Jun 19, 2024, 11:39 AM <yskelg@xxxxxxxxx
>>> <mailto:yskelg@xxxxxxxxx>> wrote:
>>>
>>> From: Yunseong Kim <yskelg@xxxxxxxxx <mailto:yskelg@xxxxxxxxx>>
>>>
>>> This patch resolve this warning.
>>>
>>> tools/perf/util/evsel.c:1620:9: error: result of comparison of constant
>>> -1 with expression of type 'char' is always false
>>>  -Werror,-Wtautological-constant-out-of-range-compare
>>>  1620 |                 if (c == -1)
>>>       |                     ~ ^  ~~
>>>
>>> Add handling on unread_unwind_spec_debug_frame().
>>> This make caller find_proc_info() works well when the allocation
>>> failure.
>>>
>>> Signed-off-by: Yunseong Kim <yskelg@xxxxxxxxx <mailto:yskelg@xxxxxxxxx>>
>>>
>>>
>>>
>>> Both changes look good. Could you make them 2 commits? If so add my: 
>>
>> No problem! I'll send it right away.
>>
>> Thank you for the code review.
>>
>>> Reviewed-by: Ian Rogers <irogers@xxxxxxxxxx <mailto:irogers@xxxxxxxxxx>>
>
> You forgot to add Ian's Reviewed-by in the next patches.
> I can add it to them this time, but please do so next time.
>
> Thanks,
> Namhyung

Thank you Namhyung for the code review. Oops, I completely forgot about
that, I'm so sorry.

>>> Thanks, 
>>> Ian
>>>
>>> ---
>>>  tools/perf/util/evsel.c                  | 2 +-
>>>  tools/perf/util/unwind-libunwind-local.c | 5 +++++
>>>  2 files changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
>>> index 25857894c047..bc603193c477 100644
>>> --- a/tools/perf/util/evsel.c
>>> +++ b/tools/perf/util/evsel.c
>>> @@ -1620,7 +1620,7 @@ static int evsel__read_group(struct evsel
>>> *leader, int cpu_map_idx, int thread)
>>>
>>>  static bool read_until_char(struct io *io, char e)
>>>  {
>>> -       char c;
>>> +       int c;
>>>
>>>         do {
>>>                 c = io__get_char(io);
>>> diff --git a/tools/perf/util/unwind-libunwind-local.c
>>> b/tools/perf/util/unwind-libunwind-local.c
>>> index cde267ea3e99..a424eae6d308 100644
>>> --- a/tools/perf/util/unwind-libunwind-local.c
>>> +++ b/tools/perf/util/unwind-libunwind-local.c
>>> @@ -390,6 +390,11 @@ static int read_unwind_spec_debug_frame(struct
>>> dso *dso,
>>>                         char *debuglink = malloc(PATH_MAX);
>>>                         int ret = 0;
>>>
>>> +                       if (debuglink == NULL) {
>>> +                               pr_err("unwind: Can't read unwind
>>> spec debug frame.\n");
>>> +                               return -ENOMEM;
>>> +                       }
>>> +
>>>                         ret = dso__read_binary_type_filename(
>>>                                 dso, DSO_BINARY_TYPE__DEBUGLINK,
>>>                                 machine->root_dir, debuglink, PATH_MAX);
>>> --
>>> 2.44.0
>>>
>>
>> Warm Regards,
>> Yunseong Kim

Thank you for your hard working and for always being.

Best regards,

Yunseong Kim