| Message ID | 20220224234823.1190425-3-irogers@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | aff006d4af0c76de6f3674266659de583a2c490f |
| Headers | show |
| Series | [1/3] libtracefs: Fix spelling in test name | expand |
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
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
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
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
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]) {
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(-)