On Thu, Feb 24, 2022 at 3:56 PM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>
> On Thu, 24 Feb 2022 15:48:23 -0800
> Ian Rogers <irogers@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > 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
|