| Message ID | 20230829154248.3762-1-laoar.shao@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next] bpftool: Fix build error with -Werror=type-limits | expand |
On 29/08/2023 16:42, Yafang Shao wrote: > Quentin reported a build error as follows, Just a warning :) > > link.c: In function ‘perf_config_hw_cache_str’: > link.c:86:18: warning: comparison of unsigned expression in ‘>= 0’ is always true [-Wtype-limits] > 86 | if ((id) >= 0 && (id) < ARRAY_SIZE(array)) \ > | ^~ > link.c:320:20: note: in expansion of macro ‘perf_event_name’ > 320 | hw_cache = perf_event_name(evsel__hw_cache, config & 0xff); > | ^~~~~~~~~~~~~~~ > [... more of the same for the other calls to perf_event_name ...] > > He also pointed out the reason and the solution: > > We're always passing unsigned, so it should be safe to drop the check on > (id) >= 0. > > Fixes: 62b57e3ddd64 ("bpftool: Add perf event names") > Reported-by: Quentin Monnet <quentin@isovalent.com> > Closes: https://lore.kernel.org/bpf/a35d9a2d-54a0-49ec-9ed1-8fcf1369d3cc@isovalent.com > Suggested-by: Quentin Monnet <quentin@isovalent.com> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> Thank you! Acked-by: Quentin Monnet <quentin@isovalent.com>
On Tue, Aug 29, 2023 at 11:51 PM Quentin Monnet <quentin@isovalent.com> wrote: > > On 29/08/2023 16:42, Yafang Shao wrote: > > Quentin reported a build error as follows, > > Just a warning :) Will reword it in the next version. > > > > > link.c: In function ‘perf_config_hw_cache_str’: > > link.c:86:18: warning: comparison of unsigned expression in ‘>= 0’ is always true [-Wtype-limits] > > 86 | if ((id) >= 0 && (id) < ARRAY_SIZE(array)) \ > > | ^~ > > link.c:320:20: note: in expansion of macro ‘perf_event_name’ > > 320 | hw_cache = perf_event_name(evsel__hw_cache, config & 0xff); > > | ^~~~~~~~~~~~~~~ > > [... more of the same for the other calls to perf_event_name ...] > > > > He also pointed out the reason and the solution: > > > > We're always passing unsigned, so it should be safe to drop the check on > > (id) >= 0. > > > > Fixes: 62b57e3ddd64 ("bpftool: Add perf event names") > > Reported-by: Quentin Monnet <quentin@isovalent.com> > > Closes: https://lore.kernel.org/bpf/a35d9a2d-54a0-49ec-9ed1-8fcf1369d3cc@isovalent.com > > Suggested-by: Quentin Monnet <quentin@isovalent.com> > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > > Thank you! > > Acked-by: Quentin Monnet <quentin@isovalent.com> > Thanks for your review.
diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c index 0b214f6ab5c8..2e5c231e08ac 100644 --- a/tools/bpf/bpftool/link.c +++ b/tools/bpf/bpftool/link.c @@ -83,7 +83,7 @@ const char *evsel__hw_cache_result[PERF_COUNT_HW_CACHE_RESULT_MAX] = { #define perf_event_name(array, id) ({ \ const char *event_str = NULL; \ \ - if ((id) >= 0 && (id) < ARRAY_SIZE(array)) \ + if ((id) < ARRAY_SIZE(array)) \ event_str = array[id]; \ event_str; \ })
Quentin reported a build error as follows, link.c: In function ‘perf_config_hw_cache_str’: link.c:86:18: warning: comparison of unsigned expression in ‘>= 0’ is always true [-Wtype-limits] 86 | if ((id) >= 0 && (id) < ARRAY_SIZE(array)) \ | ^~ link.c:320:20: note: in expansion of macro ‘perf_event_name’ 320 | hw_cache = perf_event_name(evsel__hw_cache, config & 0xff); | ^~~~~~~~~~~~~~~ [... more of the same for the other calls to perf_event_name ...] He also pointed out the reason and the solution: We're always passing unsigned, so it should be safe to drop the check on (id) >= 0. Fixes: 62b57e3ddd64 ("bpftool: Add perf event names") Reported-by: Quentin Monnet <quentin@isovalent.com> Closes: https://lore.kernel.org/bpf/a35d9a2d-54a0-49ec-9ed1-8fcf1369d3cc@isovalent.com Suggested-by: Quentin Monnet <quentin@isovalent.com> Signed-off-by: Yafang Shao <laoar.shao@gmail.com> --- tools/bpf/bpftool/link.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)