* [PATCH v1] libtracefs: An API to set the filtering of functions @ 2021-03-16 17:44 Sameeruddin shaik 2021-03-16 11:37 ` Tzvetomir Stoyanov 0 siblings, 1 reply; 3+ messages in thread From: Sameeruddin shaik @ 2021-03-16 17:44 UTC (permalink / raw) To: rostedt; +Cc: linux-trace-devel, Sameeruddin shaik This new API will write the function filters into the set_ftrace_filter file. tracefs_function_filter() https://bugzilla.kernel.org/show_bug.cgi?id=210643 Signed-off-by: Sameeruddin shaik <sameeruddin.shaik8@gmail.com> --- changed snprintf to asprintf for string formation handled the lines over 80 characters diff --git a/include/tracefs.h b/include/tracefs.h index f3eec62..c1f07b0 100644 --- a/include/tracefs.h +++ b/include/tracefs.h @@ -145,5 +145,6 @@ bool tracefs_option_is_enabled(struct tracefs_instance *instance, enum tracefs_o int tracefs_option_enable(struct tracefs_instance *instance, enum tracefs_option_id id); int tracefs_option_diasble(struct tracefs_instance *instance, enum tracefs_option_id id); const char *tracefs_option_name(enum tracefs_option_id id); - +int tracefs_function_filter(struct tracefs_instance *instance, const char **filters, + const char *module, bool reset, const char ***errs); #endif /* _TRACE_FS_H */ diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c index e2dfc7b..6b33321 100644 --- a/src/tracefs-tools.c +++ b/src/tracefs-tools.c @@ -18,6 +18,7 @@ #include "tracefs-local.h" #define TRACE_CTRL "tracing_on" +#define TRACE_FILTER "set_ftrace_filter" static const char * const options_map[] = { "unknown", @@ -387,3 +388,93 @@ void tracefs_option_clear(struct tracefs_options_mask *options, enum tracefs_opt if (options && id > TRACEFS_OPTION_INVALID) options->mask &= ~(1ULL << (id - 1)); } + +static int controlled_write(const char *filter_path, const char **filters, + const char *module, bool reset, const char ***errs) +{ + int flags = reset ? O_TRUNC : O_APPEND; + const char **e = NULL; + char *each_str = NULL; + int write_size = 0; + int size = 0; + int fd = -1; + int ret = 0; + int j = 0; + int i; + + fd = open(filter_path, O_WRONLY | flags); + if (fd < 0) + return 1; + + for (i = 0; filters[i]; i++) { + if (module) + write_size = asprintf(&each_str, "%s:mod:%s ", filters[i], module); + else + write_size = asprintf(&each_str, "%s ", filters[i]); + if (write_size < 0) + continue; + size = write(fd, each_str, write_size); + /* compare written bytes*/ + if (size < write_size) { + if (errs) { + e = realloc(e, (j + 1) * (sizeof(char *))); + e[j++] = filters[i]; + ret -= 1; + } + } + free(each_str); + } + if (errs) { + e = realloc(e, (j + 1) * (sizeof(char *))); + e[j] = NULL; + *errs = e; + } + close(fd); + return ret; +} + +/** + * tracefs_function_filter - write to set_ftrace_filter file to trace + * particular functions + * @instance: ftrace instance, can be NULL for top tracing instance + * @filters: An array of function names ending with a NULL pointer + * @module: Module to be traced + * @reset: set to true to reset the file before applying the filter + * @errs: A pointer to array of constant strings that will be allocated + * on negative return of this function, pointing to the filters that + * failed.May be NULL, in which case this field will be ignored. + * + * The @filters is an array of strings, where each string will be used + * to set a function or functions to be traced. + * + * If @reset is true, then all functions in the filter are cleared + * before adding functions from @filters. Otherwise, the functions set + * by @filters will be appended to the filter file + * + * returns -x on filter errors (where x is number of failed filter + * srtings) and if @errs is not NULL will be an allocated string array + * pointing to the strings in @filters that failed and must be freed + * with free(). + * + * returns 1 on general errors not realted to setting the filter. + * @errs is not set even if supplied. + * + * return 0 on success and @errs is not set. + */ +int tracefs_function_filter(struct tracefs_instance *instance, const char **filters, + const char *module, bool reset, const char ***errs) +{ + char *ftrace_filter_path; + int ret = 0; + + if (!filters) + return 1; + + ftrace_filter_path = tracefs_instance_get_file(instance, TRACE_FILTER); + if (!ftrace_filter_path) + return 1; + + ret = controlled_write(ftrace_filter_path, filters, module, reset, errs); + tracefs_put_tracing_file(ftrace_filter_path); + return ret; +} -- 2.7.4 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v1] libtracefs: An API to set the filtering of functions 2021-03-16 17:44 [PATCH v1] libtracefs: An API to set the filtering of functions Sameeruddin shaik @ 2021-03-16 11:37 ` Tzvetomir Stoyanov 2021-03-16 11:55 ` Sameeruddin Shaik 0 siblings, 1 reply; 3+ messages in thread From: Tzvetomir Stoyanov @ 2021-03-16 11:37 UTC (permalink / raw) To: Sameeruddin shaik; +Cc: Steven Rostedt, Linux Trace Devel Hi Sameer, I think we are almost there, I have a few comments on the error handling, that should be addressed before merging the patch. On Mon, Mar 15, 2021 at 7:49 PM Sameeruddin shaik <sameeruddin.shaik8@gmail.com> wrote: > > This new API will write the function filters into the > set_ftrace_filter file. > > tracefs_function_filter() > > https://bugzilla.kernel.org/show_bug.cgi?id=210643 > > Signed-off-by: Sameeruddin shaik <sameeruddin.shaik8@gmail.com> > --- > changed snprintf to asprintf for string formation > handled the lines over 80 characters > > diff --git a/include/tracefs.h b/include/tracefs.h > index f3eec62..c1f07b0 100644 > --- a/include/tracefs.h > +++ b/include/tracefs.h > @@ -145,5 +145,6 @@ bool tracefs_option_is_enabled(struct tracefs_instance *instance, enum tracefs_o > int tracefs_option_enable(struct tracefs_instance *instance, enum tracefs_option_id id); > int tracefs_option_diasble(struct tracefs_instance *instance, enum tracefs_option_id id); > const char *tracefs_option_name(enum tracefs_option_id id); > - > +int tracefs_function_filter(struct tracefs_instance *instance, const char **filters, > + const char *module, bool reset, const char ***errs); > #endif /* _TRACE_FS_H */ > diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c > index e2dfc7b..6b33321 100644 > --- a/src/tracefs-tools.c > +++ b/src/tracefs-tools.c > @@ -18,6 +18,7 @@ > #include "tracefs-local.h" > > #define TRACE_CTRL "tracing_on" > +#define TRACE_FILTER "set_ftrace_filter" > > static const char * const options_map[] = { > "unknown", > @@ -387,3 +388,93 @@ void tracefs_option_clear(struct tracefs_options_mask *options, enum tracefs_opt > if (options && id > TRACEFS_OPTION_INVALID) > options->mask &= ~(1ULL << (id - 1)); > } > + > +static int controlled_write(const char *filter_path, const char **filters, > + const char *module, bool reset, const char ***errs) > +{ > + int flags = reset ? O_TRUNC : O_APPEND; > + const char **e = NULL; > + char *each_str = NULL; > + int write_size = 0; > + int size = 0; > + int fd = -1; > + int ret = 0; > + int j = 0; > + int i; > + > + fd = open(filter_path, O_WRONLY | flags); > + if (fd < 0) > + return 1; > + > + for (i = 0; filters[i]; i++) { > + if (module) > + write_size = asprintf(&each_str, "%s:mod:%s ", filters[i], module); > + else > + write_size = asprintf(&each_str, "%s ", filters[i]); > + if (write_size < 0) > + continue; I think we can consider asprintf() failure as a major problem. There are two options - break the loop or goto error label. > + size = write(fd, each_str, write_size); > + /* compare written bytes*/ > + if (size < write_size) { > + if (errs) { > + e = realloc(e, (j + 1) * (sizeof(char *))); The realloc return value must be checked. In case of an error - break the loop or goto error label. > + e[j++] = filters[i]; > + ret -= 1; > + } > + } > + free(each_str); Set each_str to NULL after the free, This will help with proper resources cleanup, in case of an error. > + } If you choose the option to break the loop in case of an error, check here if filters[i] is not NULL, i.e. if the loop exit with an error. If there was an error - clean up all internally allocated resources (fd, e, each_str) and return error. The other option is to add "error" or "out" label at the end of the function where to do this resource cleanups. > + if (errs) { > + e = realloc(e, (j + 1) * (sizeof(char *))); Check that realloc return value too. > + e[j] = NULL; > + *errs = e; > + } > + close(fd); Note: in case of an internal error exit, free e and each_str, if they were allocated, and do not modify err. > + return ret; > +} > + > +/** > + * tracefs_function_filter - write to set_ftrace_filter file to trace > + * particular functions > + * @instance: ftrace instance, can be NULL for top tracing instance > + * @filters: An array of function names ending with a NULL pointer > + * @module: Module to be traced > + * @reset: set to true to reset the file before applying the filter > + * @errs: A pointer to array of constant strings that will be allocated > + * on negative return of this function, pointing to the filters that > + * failed.May be NULL, in which case this field will be ignored. > + * > + * The @filters is an array of strings, where each string will be used > + * to set a function or functions to be traced. > + * > + * If @reset is true, then all functions in the filter are cleared > + * before adding functions from @filters. Otherwise, the functions set > + * by @filters will be appended to the filter file > + * > + * returns -x on filter errors (where x is number of failed filter > + * srtings) and if @errs is not NULL will be an allocated string array > + * pointing to the strings in @filters that failed and must be freed > + * with free(). > + * > + * returns 1 on general errors not realted to setting the filter. > + * @errs is not set even if supplied. > + * > + * return 0 on success and @errs is not set. Note: in case of an error exit, free e and each_str, if they were allocated > + */ > +int tracefs_function_filter(struct tracefs_instance *instance, const char **filters, > + const char *module, bool reset, const char ***errs) > +{ > + char *ftrace_filter_path; > + int ret = 0; > + > + if (!filters) > + return 1; > + > + ftrace_filter_path = tracefs_instance_get_file(instance, TRACE_FILTER); > + if (!ftrace_filter_path) > + return 1; > + > + ret = controlled_write(ftrace_filter_path, filters, module, reset, errs); > + tracefs_put_tracing_file(ftrace_filter_path); > + return ret; > +} > -- > 2.7.4 > Thanks, Sameer! -- Tzvetomir (Ceco) Stoyanov VMware Open Source Technology Center ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v1] libtracefs: An API to set the filtering of functions 2021-03-16 11:37 ` Tzvetomir Stoyanov @ 2021-03-16 11:55 ` Sameeruddin Shaik 0 siblings, 0 replies; 3+ messages in thread From: Sameeruddin Shaik @ 2021-03-16 11:55 UTC (permalink / raw) To: Tzvetomir Stoyanov; +Cc: Steven Rostedt, Linux Trace Devel Tzvetomir, Thanks for your review, I will address your comments in the next patch. steve, Do you have any other comments on this patch? Thanks, sameer. On Tue, Mar 16, 2021 at 5:07 PM Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote: > > Hi Sameer, > I think we are almost there, I have a few comments on the error > handling, that should be addressed before merging the patch. > > On Mon, Mar 15, 2021 at 7:49 PM Sameeruddin shaik > <sameeruddin.shaik8@gmail.com> wrote: > > > > This new API will write the function filters into the > > set_ftrace_filter file. > > > > tracefs_function_filter() > > > > https://bugzilla.kernel.org/show_bug.cgi?id=210643 > > > > Signed-off-by: Sameeruddin shaik <sameeruddin.shaik8@gmail.com> > > --- > > changed snprintf to asprintf for string formation > > handled the lines over 80 characters > > > > diff --git a/include/tracefs.h b/include/tracefs.h > > index f3eec62..c1f07b0 100644 > > --- a/include/tracefs.h > > +++ b/include/tracefs.h > > @@ -145,5 +145,6 @@ bool tracefs_option_is_enabled(struct tracefs_instance *instance, enum tracefs_o > > int tracefs_option_enable(struct tracefs_instance *instance, enum tracefs_option_id id); > > int tracefs_option_diasble(struct tracefs_instance *instance, enum tracefs_option_id id); > > const char *tracefs_option_name(enum tracefs_option_id id); > > - > > +int tracefs_function_filter(struct tracefs_instance *instance, const char **filters, > > + const char *module, bool reset, const char ***errs); > > #endif /* _TRACE_FS_H */ > > diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c > > index e2dfc7b..6b33321 100644 > > --- a/src/tracefs-tools.c > > +++ b/src/tracefs-tools.c > > @@ -18,6 +18,7 @@ > > #include "tracefs-local.h" > > > > #define TRACE_CTRL "tracing_on" > > +#define TRACE_FILTER "set_ftrace_filter" > > > > static const char * const options_map[] = { > > "unknown", > > @@ -387,3 +388,93 @@ void tracefs_option_clear(struct tracefs_options_mask *options, enum tracefs_opt > > if (options && id > TRACEFS_OPTION_INVALID) > > options->mask &= ~(1ULL << (id - 1)); > > } > > + > > +static int controlled_write(const char *filter_path, const char **filters, > > + const char *module, bool reset, const char ***errs) > > +{ > > + int flags = reset ? O_TRUNC : O_APPEND; > > + const char **e = NULL; > > + char *each_str = NULL; > > + int write_size = 0; > > + int size = 0; > > + int fd = -1; > > + int ret = 0; > > + int j = 0; > > + int i; > > + > > + fd = open(filter_path, O_WRONLY | flags); > > + if (fd < 0) > > + return 1; > > + > > + for (i = 0; filters[i]; i++) { > > + if (module) > > + write_size = asprintf(&each_str, "%s:mod:%s ", filters[i], module); > > + else > > + write_size = asprintf(&each_str, "%s ", filters[i]); > > + if (write_size < 0) > > + continue; > > I think we can consider asprintf() failure as a major problem. There > are two options - break the loop or goto error label. > > > + size = write(fd, each_str, write_size); > > + /* compare written bytes*/ > > + if (size < write_size) { > > + if (errs) { > > + e = realloc(e, (j + 1) * (sizeof(char *))); > > The realloc return value must be checked. In case of an error - break > the loop or goto error label. > > > + e[j++] = filters[i]; > > + ret -= 1; > > + } > > + } > > + free(each_str); > > Set each_str to NULL after the free, This will help with proper > resources cleanup, in case of an error. > > > + } > > If you choose the option to break the loop in case of an error, check > here if filters[i] is not NULL, i.e. if the loop exit with an error. > If there was an error - clean up all internally allocated resources > (fd, e, each_str) and return error. > The other option is to add "error" or "out" label at the end of the > function where to do this resource cleanups. > > > + if (errs) { > > + e = realloc(e, (j + 1) * (sizeof(char *))); > > Check that realloc return value too. > > > + e[j] = NULL; > > + *errs = e; > > + } > > + close(fd); > > Note: in case of an internal error exit, free e and each_str, if they > were allocated, and do not modify err. > > > + return ret; > > +} > > + > > +/** > > + * tracefs_function_filter - write to set_ftrace_filter file to trace > > + * particular functions > > + * @instance: ftrace instance, can be NULL for top tracing instance > > + * @filters: An array of function names ending with a NULL pointer > > + * @module: Module to be traced > > + * @reset: set to true to reset the file before applying the filter > > + * @errs: A pointer to array of constant strings that will be allocated > > + * on negative return of this function, pointing to the filters that > > + * failed.May be NULL, in which case this field will be ignored. > > + * > > + * The @filters is an array of strings, where each string will be used > > + * to set a function or functions to be traced. > > + * > > + * If @reset is true, then all functions in the filter are cleared > > + * before adding functions from @filters. Otherwise, the functions set > > + * by @filters will be appended to the filter file > > + * > > + * returns -x on filter errors (where x is number of failed filter > > + * srtings) and if @errs is not NULL will be an allocated string array > > + * pointing to the strings in @filters that failed and must be freed > > + * with free(). > > + * > > + * returns 1 on general errors not realted to setting the filter. > > + * @errs is not set even if supplied. > > + * > > + * return 0 on success and @errs is not set. > Note: in case of an error exit, free e and each_str, if they were allocated > > + */ > > +int tracefs_function_filter(struct tracefs_instance *instance, const char **filters, > > + const char *module, bool reset, const char ***errs) > > +{ > > + char *ftrace_filter_path; > > + int ret = 0; > > + > > + if (!filters) > > + return 1; > > + > > + ftrace_filter_path = tracefs_instance_get_file(instance, TRACE_FILTER); > > + if (!ftrace_filter_path) > > + return 1; > > + > > + ret = controlled_write(ftrace_filter_path, filters, module, reset, errs); > > + tracefs_put_tracing_file(ftrace_filter_path); > > + return ret; > > +} > > -- > > 2.7.4 > > > > Thanks, Sameer! > > -- > Tzvetomir (Ceco) Stoyanov > VMware Open Source Technology Center ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-03-16 11:56 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-03-16 17:44 [PATCH v1] libtracefs: An API to set the filtering of functions Sameeruddin shaik 2021-03-16 11:37 ` Tzvetomir Stoyanov 2021-03-16 11:55 ` Sameeruddin Shaik
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).