* [PATCH] perf bpf-loader: Add missing '*' for key_scan_pos @ 2020-05-20 3:32 Wang ShaoBo 2020-05-20 7:05 ` Jiri Olsa 0 siblings, 1 reply; 5+ messages in thread From: Wang ShaoBo @ 2020-05-20 3:32 UTC (permalink / raw) Cc: cj.chengjian, huawei.libin, xiexiuqi, mark.rutland, bobo.shaobowang, guohanjun, acme, alexander.shishkin, jolsa, wangnan0, bpf, linux-kernel, linux-perf-users key_scan_pos is a pointer for getting scan position in bpf__obj_config_map() for each BPF map configuration term, but it's misused when error not happened. Fixes: 066dacbf2a32 ("perf bpf: Add API to set values to map entries in a bpf object") Signed-off-by: Wang ShaoBo <bobo.shaobowang@huawei.com> --- tools/perf/util/bpf-loader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c index 10c187b8b8ea..460056bc072c 100644 --- a/tools/perf/util/bpf-loader.c +++ b/tools/perf/util/bpf-loader.c @@ -1225,7 +1225,7 @@ bpf__obj_config_map(struct bpf_object *obj, out: free(map_name); if (!err) - key_scan_pos += strlen(map_opt); + *key_scan_pos += strlen(map_opt); return err; } -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] perf bpf-loader: Add missing '*' for key_scan_pos 2020-05-20 3:32 [PATCH] perf bpf-loader: Add missing '*' for key_scan_pos Wang ShaoBo @ 2020-05-20 7:05 ` Jiri Olsa 2020-05-20 10:22 ` Wangshaobo (bobo) 0 siblings, 1 reply; 5+ messages in thread From: Jiri Olsa @ 2020-05-20 7:05 UTC (permalink / raw) To: Wang ShaoBo Cc: cj.chengjian, huawei.libin, xiexiuqi, mark.rutland, guohanjun, acme, alexander.shishkin, wangnan0, bpf, linux-kernel, linux-perf-users On Wed, May 20, 2020 at 11:32:16AM +0800, Wang ShaoBo wrote: > key_scan_pos is a pointer for getting scan position in > bpf__obj_config_map() for each BPF map configuration term, > but it's misused when error not happened. > > Fixes: 066dacbf2a32 ("perf bpf: Add API to set values to map entries in a bpf object") > Signed-off-by: Wang ShaoBo <bobo.shaobowang@huawei.com> > --- > tools/perf/util/bpf-loader.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c > index 10c187b8b8ea..460056bc072c 100644 > --- a/tools/perf/util/bpf-loader.c > +++ b/tools/perf/util/bpf-loader.c > @@ -1225,7 +1225,7 @@ bpf__obj_config_map(struct bpf_object *obj, > out: > free(map_name); > if (!err) > - key_scan_pos += strlen(map_opt); > + *key_scan_pos += strlen(map_opt); seems good, was there something failing because of this? Acked-by: Jiri Olsa <jolsa@redhat.com> thanks, jirka > return err; > } > > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf bpf-loader: Add missing '*' for key_scan_pos 2020-05-20 7:05 ` Jiri Olsa @ 2020-05-20 10:22 ` Wangshaobo (bobo) 2020-05-20 10:30 ` Jiri Olsa 2020-05-20 14:30 ` Arnaldo Carvalho de Melo 0 siblings, 2 replies; 5+ messages in thread From: Wangshaobo (bobo) @ 2020-05-20 10:22 UTC (permalink / raw) To: Jiri Olsa Cc: cj.chengjian, huawei.libin, xiexiuqi, mark.rutland, guohanjun, acme, alexander.shishkin, wangnan0, bpf, linux-kernel, linux-perf-users 在 2020/5/20 15:05, Jiri Olsa 写道: > On Wed, May 20, 2020 at 11:32:16AM +0800, Wang ShaoBo wrote: >> key_scan_pos is a pointer for getting scan position in >> bpf__obj_config_map() for each BPF map configuration term, >> but it's misused when error not happened. >> >> Fixes: 066dacbf2a32 ("perf bpf: Add API to set values to map entries in a bpf object") >> Signed-off-by: Wang ShaoBo <bobo.shaobowang@huawei.com> >> --- >> tools/perf/util/bpf-loader.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c >> index 10c187b8b8ea..460056bc072c 100644 >> --- a/tools/perf/util/bpf-loader.c >> +++ b/tools/perf/util/bpf-loader.c >> @@ -1225,7 +1225,7 @@ bpf__obj_config_map(struct bpf_object *obj, >> out: >> free(map_name); >> if (!err) >> - key_scan_pos += strlen(map_opt); >> + *key_scan_pos += strlen(map_opt); > seems good, was there something failing because of this? > > Acked-by: Jiri Olsa <jolsa@redhat.com> > > thanks, > jirka I found this problem when i checked this code, I think it is an implicit question, but if we delete the two line, the problem also no longer exists. thanks, Wang ShaoBo ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf bpf-loader: Add missing '*' for key_scan_pos 2020-05-20 10:22 ` Wangshaobo (bobo) @ 2020-05-20 10:30 ` Jiri Olsa 2020-05-20 14:30 ` Arnaldo Carvalho de Melo 1 sibling, 0 replies; 5+ messages in thread From: Jiri Olsa @ 2020-05-20 10:30 UTC (permalink / raw) To: Wangshaobo (bobo) Cc: cj.chengjian, huawei.libin, xiexiuqi, mark.rutland, guohanjun, acme, alexander.shishkin, wangnan0, bpf, linux-kernel, linux-perf-users On Wed, May 20, 2020 at 06:22:12PM +0800, Wangshaobo (bobo) wrote: > > 在 2020/5/20 15:05, Jiri Olsa 写道: > > On Wed, May 20, 2020 at 11:32:16AM +0800, Wang ShaoBo wrote: > > > key_scan_pos is a pointer for getting scan position in > > > bpf__obj_config_map() for each BPF map configuration term, > > > but it's misused when error not happened. > > > > > > Fixes: 066dacbf2a32 ("perf bpf: Add API to set values to map entries in a bpf object") > > > Signed-off-by: Wang ShaoBo <bobo.shaobowang@huawei.com> > > > --- > > > tools/perf/util/bpf-loader.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c > > > index 10c187b8b8ea..460056bc072c 100644 > > > --- a/tools/perf/util/bpf-loader.c > > > +++ b/tools/perf/util/bpf-loader.c > > > @@ -1225,7 +1225,7 @@ bpf__obj_config_map(struct bpf_object *obj, > > > out: > > > free(map_name); > > > if (!err) > > > - key_scan_pos += strlen(map_opt); > > > + *key_scan_pos += strlen(map_opt); > > seems good, was there something failing because of this? > > > > Acked-by: Jiri Olsa <jolsa@redhat.com> > > > > thanks, > > jirka > > I found this problem when i checked this code, I think it is > > an implicit question, but if we delete the two line, the problem > > also no longer exists. and what's the actual problem, what's broken? jirka > > thanks, > > Wang ShaoBo > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf bpf-loader: Add missing '*' for key_scan_pos 2020-05-20 10:22 ` Wangshaobo (bobo) 2020-05-20 10:30 ` Jiri Olsa @ 2020-05-20 14:30 ` Arnaldo Carvalho de Melo 1 sibling, 0 replies; 5+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-05-20 14:30 UTC (permalink / raw) To: Wangshaobo (bobo) Cc: Jiri Olsa, cj.chengjian, huawei.libin, xiexiuqi, mark.rutland, guohanjun, alexander.shishkin, wangnan0, bpf, linux-kernel, linux-perf-users Em Wed, May 20, 2020 at 06:22:12PM +0800, Wangshaobo (bobo) escreveu: > > 在 2020/5/20 15:05, Jiri Olsa 写道: > > On Wed, May 20, 2020 at 11:32:16AM +0800, Wang ShaoBo wrote: > > > key_scan_pos is a pointer for getting scan position in > > > bpf__obj_config_map() for each BPF map configuration term, > > > but it's misused when error not happened. > > > > > > Fixes: 066dacbf2a32 ("perf bpf: Add API to set values to map entries in a bpf object") > > > Signed-off-by: Wang ShaoBo <bobo.shaobowang@huawei.com> > > > --- > > > tools/perf/util/bpf-loader.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c > > > index 10c187b8b8ea..460056bc072c 100644 > > > --- a/tools/perf/util/bpf-loader.c > > > +++ b/tools/perf/util/bpf-loader.c > > > @@ -1225,7 +1225,7 @@ bpf__obj_config_map(struct bpf_object *obj, > > > out: > > > free(map_name); > > > if (!err) > > > - key_scan_pos += strlen(map_opt); > > > + *key_scan_pos += strlen(map_opt); > > seems good, was there something failing because of this? > > > > Acked-by: Jiri Olsa <jolsa@redhat.com> > > > > thanks, > > jirka > > I found this problem when i checked this code, I think it is > > an implicit question, but if we delete the two line, the problem > > also no longer exists. The point is that the only user of this is: tools/perf/util/parse-events.c err = bpf__config_obj(obj, term, parse_state->evlist, &error_pos); if (err) bpf__strerror_config_obj(obj, term, parse_state->evlist, &error_pos, err, errbuf, sizeof(errbuf)); And then: int bpf__strerror_config_obj(struct bpf_object *obj __maybe_unused, struct parse_events_term *term __maybe_unused, struct evlist *evlist __maybe_unused, int *error_pos __maybe_unused, int err, char *buf, size_t size) { bpf__strerror_head(err, buf, size); bpf__strerror_entry(BPF_LOADER_ERRNO__OBJCONF_MAP_TYPE, "Can't use this config term with this map type"); bpf__strerror_end(buf, size); return 0; } So this is infrastructure that Wang Nan put in place for providing better error messages but that he ended up not using, so I'll apply the fix, its correct even not fixing any real problem at this time. - Arnaldo ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-05-20 14:30 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-20 3:32 [PATCH] perf bpf-loader: Add missing '*' for key_scan_pos Wang ShaoBo 2020-05-20 7:05 ` Jiri Olsa 2020-05-20 10:22 ` Wangshaobo (bobo) 2020-05-20 10:30 ` Jiri Olsa 2020-05-20 14:30 ` Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).