* [PATCH 1/3] libtracefs: Fix spelling in test name @ 2022-02-24 23:48 Ian Rogers 2022-02-24 23:48 ` [PATCH 2/3] libtracefs: Avoid null access Ian Rogers 2022-02-24 23:48 ` [PATCH 3/3] libtracefs: Move initialization below a null test Ian Rogers 0 siblings, 2 replies; 7+ messages in thread From: Ian Rogers @ 2022-02-24 23:48 UTC (permalink / raw) To: linux-trace-devel, Tzvetomir Stoyanov, Steven Rostedt; +Cc: Ian Rogers Synthetic rather than syntetic. Signed-off-by: Ian Rogers <irogers@google.com> --- utest/tracefs-utest.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/utest/tracefs-utest.c b/utest/tracefs-utest.c index 0903243..5163004 100644 --- a/utest/tracefs-utest.c +++ b/utest/tracefs-utest.c @@ -436,7 +436,7 @@ static void test_synth_compare(struct test_synth *synth, struct tracefs_dynevent CU_TEST(devents[i] == NULL); } -static void test_instance_syntetic(struct tracefs_instance *instance) +static void test_instance_synthetic(struct tracefs_instance *instance) { struct test_synth sevents[] = { {"synth_1", "sched", "sched_waking", "sched", "sched_switch", "pid", "next_pid", "pid_match"}, @@ -496,7 +496,7 @@ static void test_instance_syntetic(struct tracefs_instance *instance) static void test_synthetic(void) { - test_instance_syntetic(test_instance); + test_instance_synthetic(test_instance); } static void test_trace_file(void) @@ -1706,6 +1706,6 @@ void test_tracefs_lib(void) CU_add_test(suite, "ftrace marker", test_ftrace_marker); CU_add_test(suite, "kprobes", test_kprobes); - CU_add_test(suite, "syntetic events", test_synthetic); + CU_add_test(suite, "synthetic events", test_synthetic); CU_add_test(suite, "eprobes", test_eprobes); } -- 2.35.1.574.g5d30c73bfb-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] libtracefs: Avoid null access 2022-02-24 23:48 [PATCH 1/3] libtracefs: Fix spelling in test name Ian Rogers @ 2022-02-24 23:48 ` Ian Rogers 2022-02-24 23:48 ` [PATCH 3/3] libtracefs: Move initialization below a null test Ian Rogers 1 sibling, 0 replies; 7+ messages in thread From: Ian Rogers @ 2022-02-24 23:48 UTC (permalink / raw) To: linux-trace-devel, Tzvetomir Stoyanov, Steven Rostedt; +Cc: Ian Rogers The for loop handles devents == NULL, but the test afterwards needs to also. Signed-off-by: Ian Rogers <irogers@google.com> --- utest/tracefs-utest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utest/tracefs-utest.c b/utest/tracefs-utest.c index 5163004..7042fa9 100644 --- a/utest/tracefs-utest.c +++ b/utest/tracefs-utest.c @@ -433,7 +433,7 @@ static void test_synth_compare(struct test_synth *synth, struct tracefs_dynevent free(event); free(format); } - CU_TEST(devents[i] == NULL); + CU_TEST(devents == NULL || devents[i] == NULL); } static void test_instance_synthetic(struct tracefs_instance *instance) -- 2.35.1.574.g5d30c73bfb-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] libtracefs: Move initialization below a null test. 2022-02-24 23:48 [PATCH 1/3] libtracefs: Fix spelling in test name Ian Rogers 2022-02-24 23:48 ` [PATCH 2/3] libtracefs: Avoid null access Ian Rogers @ 2022-02-24 23:48 ` Ian Rogers 2022-02-24 23:55 ` Steven Rostedt 1 sibling, 1 reply; 7+ messages in thread From: Ian Rogers @ 2022-02-24 23:48 UTC (permalink / raw) To: linux-trace-devel, Tzvetomir Stoyanov, Steven Rostedt; +Cc: Ian Rogers Computing the address from a NULL pointer results in undefined behavior which things like undefined behavior sanitizer promote into real failures. Signed-off-by: Ian Rogers <irogers@google.com> --- src/tracefs-dynevents.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/tracefs-dynevents.c b/src/tracefs-dynevents.c index 61804b9..8fee474 100644 --- a/src/tracefs-dynevents.c +++ b/src/tracefs-dynevents.c @@ -724,13 +724,18 @@ tracefs_dynevent_info(struct tracefs_dynevent *dynevent, char **system, char **event, char **prefix, char **addr, char **format) { char **lv[] = { system, event, prefix, addr, format }; - char **rv[] = { &dynevent->system, &dynevent->event, &dynevent->prefix, - &dynevent->address, &dynevent->format }; + char **rv[] = { NULL, NULL, NULL, NULL, NULL }; int i; if (!dynevent) return TRACEFS_DYNEVENT_UNKNOWN; + rv[0] = &dynevent->system; + rv[1] = &dynevent->event; + rv[2] = &dynevent->prefix; + rv[3] = &dynevent->address; + rv[4] = &dynevent->format; + for (i = 0; i < ARRAY_SIZE(lv); i++) { if (lv[i]) { if (*rv[i]) { -- 2.35.1.574.g5d30c73bfb-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] libtracefs: Move initialization below a null test. 2022-02-24 23:48 ` [PATCH 3/3] libtracefs: Move initialization below a null test Ian Rogers @ 2022-02-24 23:55 ` Steven Rostedt 2022-02-25 0:09 ` Ian Rogers 0 siblings, 1 reply; 7+ messages in thread From: Steven Rostedt @ 2022-02-24 23:55 UTC (permalink / raw) To: Ian Rogers; +Cc: linux-trace-devel, Tzvetomir Stoyanov On Thu, 24 Feb 2022 15:48:23 -0800 Ian Rogers <irogers@google.com> wrote: > Computing the address from a NULL pointer results in undefined behavior > which things like undefined behavior sanitizer promote into real > failures. > > Signed-off-by: Ian Rogers <irogers@google.com> > --- > src/tracefs-dynevents.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/src/tracefs-dynevents.c b/src/tracefs-dynevents.c > index 61804b9..8fee474 100644 > --- a/src/tracefs-dynevents.c > +++ b/src/tracefs-dynevents.c > @@ -724,13 +724,18 @@ tracefs_dynevent_info(struct tracefs_dynevent *dynevent, char **system, > char **event, char **prefix, char **addr, char **format) > { > char **lv[] = { system, event, prefix, addr, format }; > - char **rv[] = { &dynevent->system, &dynevent->event, &dynevent->prefix, > - &dynevent->address, &dynevent->format }; > + char **rv[] = { NULL, NULL, NULL, NULL, NULL }; > int i; I don't really like the NULL initialization done like this. What about just adding a helper function. That is. diff --git a/src/tracefs-dynevents.c b/src/tracefs-dynevents.c index 61804b9d9d5e..48bb26a96c58 100644 --- a/src/tracefs-dynevents.c +++ b/src/tracefs-dynevents.c @@ -704,24 +704,9 @@ __hidden int dynevent_get_count(unsigned int types, const char *system) return all; } -/** - * tracefs_dynevent_info - return details of a dynamic event - * @dynevent: A dynamic event context, describing given dynamic event. - * @group: return, group in which the dynamic event is configured - * @event: return, name of the dynamic event - * @prefix: return, prefix string of the dynamic event - * @addr: return, the function and offset (or address) of the dynamic event - * @format: return, the format string of the dynamic event - * - * Returns the type of the dynamic event, or TRACEFS_DYNEVENT_UNKNOWN in case of an error. - * Any of the @group, @event, @prefix, @addr and @format parameters are optional. - * If a valid pointer is passed, in case of success - a string is allocated and returned. - * These strings must be freed with free(). - */ - -enum tracefs_dynevent_type -tracefs_dynevent_info(struct tracefs_dynevent *dynevent, char **system, - char **event, char **prefix, char **addr, char **format) +static enum tracefs_dynevent_type +dynevent_info(struct tracefs_dynevent *dynevent, char **system, + char **event, char **prefix, char **addr, char **format) { char **lv[] = { system, event, prefix, addr, format }; char **rv[] = { &dynevent->system, &dynevent->event, &dynevent->prefix, @@ -754,6 +739,30 @@ error: return TRACEFS_DYNEVENT_UNKNOWN; } +/** + * tracefs_dynevent_info - return details of a dynamic event + * @dynevent: A dynamic event context, describing given dynamic event. + * @group: return, group in which the dynamic event is configured + * @event: return, name of the dynamic event + * @prefix: return, prefix string of the dynamic event + * @addr: return, the function and offset (or address) of the dynamic event + * @format: return, the format string of the dynamic event + * + * Returns the type of the dynamic event, or TRACEFS_DYNEVENT_UNKNOWN in case of an error. + * Any of the @group, @event, @prefix, @addr and @format parameters are optional. + * If a valid pointer is passed, in case of success - a string is allocated and returned. + * These strings must be freed with free(). + */ +enum tracefs_dynevent_type +tracefs_dynevent_info(struct tracefs_dynevent *dynevent, char **system, + char **event, char **prefix, char **addr, char **format) +{ + if (!dynevent) + return TRACEFS_DYNEVENT_UNKNOWN; + + return dynevent_info(dynevent, system, event, prefix, addr, format); +} + /** * tracefs_dynevent_get_event - return tep event representing the given dynamic event * @tep: a handle to the trace event parser context that holds the events -- Steve ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] libtracefs: Move initialization below a null test. 2022-02-24 23:55 ` Steven Rostedt @ 2022-02-25 0:09 ` Ian Rogers 2022-02-25 0:20 ` Steven Rostedt 0 siblings, 1 reply; 7+ messages in thread From: Ian Rogers @ 2022-02-25 0:09 UTC (permalink / raw) To: Steven Rostedt; +Cc: linux-trace-devel, Tzvetomir Stoyanov On Thu, Feb 24, 2022 at 3:56 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Thu, 24 Feb 2022 15:48:23 -0800 > Ian Rogers <irogers@google.com> wrote: > > > Computing the address from a NULL pointer results in undefined behavior > > which things like undefined behavior sanitizer promote into real > > failures. > > > > Signed-off-by: Ian Rogers <irogers@google.com> > > --- > > src/tracefs-dynevents.c | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/src/tracefs-dynevents.c b/src/tracefs-dynevents.c > > index 61804b9..8fee474 100644 > > --- a/src/tracefs-dynevents.c > > +++ b/src/tracefs-dynevents.c > > @@ -724,13 +724,18 @@ tracefs_dynevent_info(struct tracefs_dynevent *dynevent, char **system, > > char **event, char **prefix, char **addr, char **format) > > { > > char **lv[] = { system, event, prefix, addr, format }; > > - char **rv[] = { &dynevent->system, &dynevent->event, &dynevent->prefix, > > - &dynevent->address, &dynevent->format }; > > + char **rv[] = { NULL, NULL, NULL, NULL, NULL }; > > int i; > > I don't really like the NULL initialization done like this. > > What about just adding a helper function. That is. > > diff --git a/src/tracefs-dynevents.c b/src/tracefs-dynevents.c > index 61804b9d9d5e..48bb26a96c58 100644 > --- a/src/tracefs-dynevents.c > +++ b/src/tracefs-dynevents.c > @@ -704,24 +704,9 @@ __hidden int dynevent_get_count(unsigned int types, const char *system) > return all; > } > > -/** > - * tracefs_dynevent_info - return details of a dynamic event > - * @dynevent: A dynamic event context, describing given dynamic event. > - * @group: return, group in which the dynamic event is configured > - * @event: return, name of the dynamic event > - * @prefix: return, prefix string of the dynamic event > - * @addr: return, the function and offset (or address) of the dynamic event > - * @format: return, the format string of the dynamic event > - * > - * Returns the type of the dynamic event, or TRACEFS_DYNEVENT_UNKNOWN in case of an error. > - * Any of the @group, @event, @prefix, @addr and @format parameters are optional. > - * If a valid pointer is passed, in case of success - a string is allocated and returned. > - * These strings must be freed with free(). > - */ > - > -enum tracefs_dynevent_type > -tracefs_dynevent_info(struct tracefs_dynevent *dynevent, char **system, > - char **event, char **prefix, char **addr, char **format) > +static enum tracefs_dynevent_type > +dynevent_info(struct tracefs_dynevent *dynevent, char **system, > + char **event, char **prefix, char **addr, char **format) > { > char **lv[] = { system, event, prefix, addr, format }; > char **rv[] = { &dynevent->system, &dynevent->event, &dynevent->prefix, > @@ -754,6 +739,30 @@ error: > return TRACEFS_DYNEVENT_UNKNOWN; > } > > +/** > + * tracefs_dynevent_info - return details of a dynamic event > + * @dynevent: A dynamic event context, describing given dynamic event. > + * @group: return, group in which the dynamic event is configured > + * @event: return, name of the dynamic event > + * @prefix: return, prefix string of the dynamic event > + * @addr: return, the function and offset (or address) of the dynamic event > + * @format: return, the format string of the dynamic event > + * > + * Returns the type of the dynamic event, or TRACEFS_DYNEVENT_UNKNOWN in case of an error. > + * Any of the @group, @event, @prefix, @addr and @format parameters are optional. > + * If a valid pointer is passed, in case of success - a string is allocated and returned. > + * These strings must be freed with free(). > + */ > +enum tracefs_dynevent_type > +tracefs_dynevent_info(struct tracefs_dynevent *dynevent, char **system, > + char **event, char **prefix, char **addr, char **format) > +{ > + if (!dynevent) > + return TRACEFS_DYNEVENT_UNKNOWN; > + > + return dynevent_info(dynevent, system, event, prefix, addr, format); > +} > + > /** > * tracefs_dynevent_get_event - return tep event representing the given dynamic event > * @tep: a handle to the trace event parser context that holds the events > LGTM. Thanks, Ian > -- Steve ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] libtracefs: Move initialization below a null test. 2022-02-25 0:09 ` Ian Rogers @ 2022-02-25 0:20 ` Steven Rostedt 2022-02-25 0:21 ` Ian Rogers 0 siblings, 1 reply; 7+ messages in thread From: Steven Rostedt @ 2022-02-25 0:20 UTC (permalink / raw) To: Ian Rogers; +Cc: linux-trace-devel, Tzvetomir Stoyanov On Thu, 24 Feb 2022 16:09:29 -0800 Ian Rogers <irogers@google.com> wrote: > > /** > > * tracefs_dynevent_get_event - return tep event representing the given dynamic event > > * @tep: a handle to the trace event parser context that holds the events > > > > LGTM. Thanks, Did you want to resend, or do you want me to just do this myself? I'm good either way. -- Steve ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] libtracefs: Move initialization below a null test. 2022-02-25 0:20 ` Steven Rostedt @ 2022-02-25 0:21 ` Ian Rogers 0 siblings, 0 replies; 7+ messages in thread From: Ian Rogers @ 2022-02-25 0:21 UTC (permalink / raw) To: Steven Rostedt; +Cc: linux-trace-devel, Tzvetomir Stoyanov On Thu, Feb 24, 2022 at 4:20 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Thu, 24 Feb 2022 16:09:29 -0800 > Ian Rogers <irogers@google.com> wrote: > > > > /** > > > * tracefs_dynevent_get_event - return tep event representing the given dynamic event > > > * @tep: a handle to the trace event parser context that holds the events > > > > > > > LGTM. Thanks, > > Did you want to resend, or do you want me to just do this myself? I'm good > either way. Happy to let you do it. Thanks, Ian > -- Steve ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-02-25 0:21 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-02-24 23:48 [PATCH 1/3] libtracefs: Fix spelling in test name Ian Rogers 2022-02-24 23:48 ` [PATCH 2/3] libtracefs: Avoid null access Ian Rogers 2022-02-24 23:48 ` [PATCH 3/3] libtracefs: Move initialization below a null test Ian Rogers 2022-02-24 23:55 ` Steven Rostedt 2022-02-25 0:09 ` Ian Rogers 2022-02-25 0:20 ` Steven Rostedt 2022-02-25 0:21 ` Ian Rogers
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).