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

[Yunseong Kim <yskelg@gmail.com>]

Hi Ian,

On 6/20/24 4:03 오전, Ian Rogers wrote:
> 
> 
> On Wed, Jun 19, 2024, 11:39 AM <yskelg@gmail.com
> <mailto:yskelg@gmail.com>> wrote:
> 
>     From: Yunseong Kim <yskelg@gmail.com <mailto:yskelg@gmail.com>>
> 
>     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@gmail.com <mailto:yskelg@gmail.com>>
> 
> 
> 
> 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@google.com <mailto:irogers@google.com>>
> 
> 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