| Message ID | 20210402131947.346235-6-y.karadz@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Refactor the APIs for tracing options | expand |
On Fri, Apr 2, 2021 at 4:20 PM Yordan Karadzhov (VMware) <y.karadz@gmail.com> wrote: > > If the instance owns two mask objects, we no longer need to > dynamically allocate memory in tracefs_options_get_supported() and > tracefs_options_get_enabled(). This will simplify the code on the > caller side, since the user is no longer responsible for freeing > those masks. > > Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com> > --- > Documentation/libtracefs-option-get.txt | 4 +--- > include/tracefs.h | 6 +++--- > src/tracefs-instance.c | 24 ++++++++++++------------ > 3 files changed, 16 insertions(+), 18 deletions(-) > > diff --git a/Documentation/libtracefs-option-get.txt b/Documentation/libtracefs-option-get.txt > index 9b3cb56..3290f24 100644 > --- a/Documentation/libtracefs-option-get.txt > +++ b/Documentation/libtracefs-option-get.txt > @@ -52,14 +52,13 @@ EXAMPLE > -- > #include <tracefs.h> > ... > -struct tracefs_options_mask *options; > +const struct tracefs_options_mask *options; > ... > options = tracefs_options_get_supported(NULL); > if (!options) { > /* Failed to get supported options */ > } else { > ... > - free(options); > } > .. The description of the return value of tracefs_options_get_supported() and tracefs_options_get_enabled() should be changed also, as it suggests to free the return value; > options = tracefs_options_get_enabled(NULL); > @@ -67,7 +66,6 @@ if (!options) { > /* Failed to get options, enabled in the top instance */ > } else { > ... > - free(options); > } > ... > > diff --git a/include/tracefs.h b/include/tracefs.h > index 0665e8d..9efa91a 100644 > --- a/include/tracefs.h > +++ b/include/tracefs.h > @@ -132,11 +132,11 @@ enum tracefs_option_id { > #define TRACEFS_OPTION_MAX (TRACEFS_OPTION_VERBOSE + 1) > > struct tracefs_options_mask; > -bool tracefs_option_is_set(struct tracefs_options_mask *options, > +bool tracefs_option_is_set(const struct tracefs_options_mask *options, > enum tracefs_option_id id); > -struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instance *instance); > +const struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instance *instance); > bool tracefs_option_is_supported(struct tracefs_instance *instance, enum tracefs_option_id id); > -struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance *instance); > +const struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance *instance); > bool tracefs_option_is_enabled(struct tracefs_instance *instance, enum tracefs_option_id id); > 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); > diff --git a/src/tracefs-instance.c b/src/tracefs-instance.c > index 49645eb..9cb4b6d 100644 > --- a/src/tracefs-instance.c > +++ b/src/tracefs-instance.c > @@ -27,6 +27,8 @@ struct tracefs_instance { > char *trace_dir; > char *name; > int flags; > + struct tracefs_options_mask supported_opts; > + struct tracefs_options_mask enabled_opts; > }; > > /** > @@ -695,9 +697,8 @@ static struct tracefs_options_mask *trace_get_options(struct tracefs_instance *i > DIR *dir = NULL; > long long val; > > - bitmask = calloc(1, sizeof(struct tracefs_options_mask)); > - if (!bitmask) > - return NULL; > + bitmask = enabled? &instance->enabled_opts : &instance->supported_opts; This makes the API not thread safe. There should be no problem with supported options, as they cannot be changed at runtime. I think it is OK to limit the APIs , as I see no use case to call them from multiple threads on the same instance, but this should be described in the documentation. > + > dname = tracefs_instance_get_file(instance, "options"); > if (!dname) > goto error; > @@ -727,7 +728,6 @@ error: > if (dir) > closedir(dir); > tracefs_put_tracing_file(dname); > - free(bitmask); > return NULL; > } > > @@ -735,10 +735,10 @@ error: > * tracefs_options_get_supported - Get all supported trace options in given instance > * @instance: ftrace instance, can be NULL for the top instance > * > - * Returns allocated bitmask structure with all trace options, supported in given > - * instance, or NULL in case of an error. The returned structure must be freed with free() > + * Returns bitmask structure with all trace options, supported in given instance, > + * or NULL in case of an error. > */ > -struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instance *instance) > +const struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instance *instance) > { > return trace_get_options(instance, false); As supported options cannot be changed at runtime, they can be read only once. The next calls can return the already initialised bitmask with supported options. > } > @@ -747,10 +747,10 @@ struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instan > * tracefs_options_get_enabled - Get all currently enabled trace options in given instance > * @instance: ftrace instance, can be NULL for the top instance > * > - * Returns allocated bitmask structure with all trace options, enabled in given > - * instance, or NULL in case of an error. The returned structure must be freed with free() > + * Returns allocated bitmask structure with all trace options, enabled in given instance, > + * or NULL in case of an error. > */ > -struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance *instance) > +const struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance *instance) > { > return trace_get_options(instance, true); > } > @@ -758,7 +758,7 @@ struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance > static int trace_config_option(struct tracefs_instance *instance, > enum tracefs_option_id id, bool set) > { > - char *set_str = set ? "1" : "0"; > + const char *set_str = set ? "1" : "0"; > char file[PATH_MAX]; > const char *name; > > @@ -846,7 +846,7 @@ bool tracefs_option_is_enabled(struct tracefs_instance *instance, enum tracefs_o > * Returns true if an option with given id is set in the bitmask, > * false if it is not set. > */ > -bool tracefs_option_is_set(struct tracefs_options_mask *options, > +bool tracefs_option_is_set(const struct tracefs_options_mask *options, > enum tracefs_option_id id) > { > if (id > TRACEFS_OPTION_INVALID) > -- > 2.27.0 >
On Fri, Apr 2, 2021 at 4:20 PM Yordan Karadzhov (VMware) <y.karadz@gmail.com> wrote: > > If the instance owns two mask objects, we no longer need to > dynamically allocate memory in tracefs_options_get_supported() and > tracefs_options_get_enabled(). This will simplify the code on the > caller side, since the user is no longer responsible for freeing > those masks. > > Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com> > --- > Documentation/libtracefs-option-get.txt | 4 +--- > include/tracefs.h | 6 +++--- > src/tracefs-instance.c | 24 ++++++++++++------------ > 3 files changed, 16 insertions(+), 18 deletions(-) > > diff --git a/Documentation/libtracefs-option-get.txt b/Documentation/libtracefs-option-get.txt > index 9b3cb56..3290f24 100644 > --- a/Documentation/libtracefs-option-get.txt > +++ b/Documentation/libtracefs-option-get.txt > @@ -52,14 +52,13 @@ EXAMPLE > -- > #include <tracefs.h> > ... > -struct tracefs_options_mask *options; > +const struct tracefs_options_mask *options; > ... > options = tracefs_options_get_supported(NULL); > if (!options) { > /* Failed to get supported options */ > } else { > ... > - free(options); > } > ... > options = tracefs_options_get_enabled(NULL); > @@ -67,7 +66,6 @@ if (!options) { > /* Failed to get options, enabled in the top instance */ > } else { > ... > - free(options); > } > ... > > diff --git a/include/tracefs.h b/include/tracefs.h > index 0665e8d..9efa91a 100644 > --- a/include/tracefs.h > +++ b/include/tracefs.h > @@ -132,11 +132,11 @@ enum tracefs_option_id { > #define TRACEFS_OPTION_MAX (TRACEFS_OPTION_VERBOSE + 1) > > struct tracefs_options_mask; > -bool tracefs_option_is_set(struct tracefs_options_mask *options, > +bool tracefs_option_is_set(const struct tracefs_options_mask *options, > enum tracefs_option_id id); > -struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instance *instance); > +const struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instance *instance); > bool tracefs_option_is_supported(struct tracefs_instance *instance, enum tracefs_option_id id); > -struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance *instance); > +const struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance *instance); > bool tracefs_option_is_enabled(struct tracefs_instance *instance, enum tracefs_option_id id); > 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); > diff --git a/src/tracefs-instance.c b/src/tracefs-instance.c > index 49645eb..9cb4b6d 100644 > --- a/src/tracefs-instance.c > +++ b/src/tracefs-instance.c > @@ -27,6 +27,8 @@ struct tracefs_instance { > char *trace_dir; > char *name; > int flags; > + struct tracefs_options_mask supported_opts; > + struct tracefs_options_mask enabled_opts; > }; > > /** > @@ -695,9 +697,8 @@ static struct tracefs_options_mask *trace_get_options(struct tracefs_instance *i > DIR *dir = NULL; > long long val; > > - bitmask = calloc(1, sizeof(struct tracefs_options_mask)); > - if (!bitmask) > - return NULL; > + bitmask = enabled? &instance->enabled_opts : &instance->supported_opts; The "instance" argument can be NULL, for the top instance. There should be global enabled_opts and supported_opts, to be used in that case. [..]
On 5.04.21 г. 13:50, Tzvetomir Stoyanov wrote: > On Fri, Apr 2, 2021 at 4:20 PM Yordan Karadzhov (VMware) > <y.karadz@gmail.com> wrote: >> If the instance owns two mask objects, we no longer need to >> dynamically allocate memory in tracefs_options_get_supported() and >> tracefs_options_get_enabled(). This will simplify the code on the >> caller side, since the user is no longer responsible for freeing >> those masks. >> >> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com> >> --- >> Documentation/libtracefs-option-get.txt | 4 +--- >> include/tracefs.h | 6 +++--- >> src/tracefs-instance.c | 24 ++++++++++++------------ >> 3 files changed, 16 insertions(+), 18 deletions(-) >> >> diff --git a/Documentation/libtracefs-option-get.txt b/Documentation/libtracefs-option-get.txt >> index 9b3cb56..3290f24 100644 >> --- a/Documentation/libtracefs-option-get.txt >> +++ b/Documentation/libtracefs-option-get.txt >> @@ -52,14 +52,13 @@ EXAMPLE >> -- >> #include <tracefs.h> >> ... >> -struct tracefs_options_mask *options; >> +const struct tracefs_options_mask *options; >> ... >> options = tracefs_options_get_supported(NULL); >> if (!options) { >> /* Failed to get supported options */ >> } else { >> ... >> - free(options); >> } >> .. > The description of the return value of tracefs_options_get_supported() > and tracefs_options_get_enabled() should be changed also, as it > suggests to free the return value; > >> options = tracefs_options_get_enabled(NULL); >> @@ -67,7 +66,6 @@ if (!options) { >> /* Failed to get options, enabled in the top instance */ >> } else { >> ... >> - free(options); >> } >> ... >> >> diff --git a/include/tracefs.h b/include/tracefs.h >> index 0665e8d..9efa91a 100644 >> --- a/include/tracefs.h >> +++ b/include/tracefs.h >> @@ -132,11 +132,11 @@ enum tracefs_option_id { >> #define TRACEFS_OPTION_MAX (TRACEFS_OPTION_VERBOSE + 1) >> >> struct tracefs_options_mask; >> -bool tracefs_option_is_set(struct tracefs_options_mask *options, >> +bool tracefs_option_is_set(const struct tracefs_options_mask *options, >> enum tracefs_option_id id); >> -struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instance *instance); >> +const struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instance *instance); >> bool tracefs_option_is_supported(struct tracefs_instance *instance, enum tracefs_option_id id); >> -struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance *instance); >> +const struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance *instance); >> bool tracefs_option_is_enabled(struct tracefs_instance *instance, enum tracefs_option_id id); >> 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); >> diff --git a/src/tracefs-instance.c b/src/tracefs-instance.c >> index 49645eb..9cb4b6d 100644 >> --- a/src/tracefs-instance.c >> +++ b/src/tracefs-instance.c >> @@ -27,6 +27,8 @@ struct tracefs_instance { >> char *trace_dir; >> char *name; >> int flags; >> + struct tracefs_options_mask supported_opts; >> + struct tracefs_options_mask enabled_opts; >> }; >> >> /** >> @@ -695,9 +697,8 @@ static struct tracefs_options_mask *trace_get_options(struct tracefs_instance *i >> DIR *dir = NULL; >> long long val; >> >> - bitmask = calloc(1, sizeof(struct tracefs_options_mask)); >> - if (!bitmask) >> - return NULL; >> + bitmask = enabled? &instance->enabled_opts : &instance->supported_opts; > This makes the API not thread safe. There should be no problem with > supported options, as they cannot be changed at runtime. I think it is > OK to limit the APIs , as I see no use case to call them from multiple > threads on the same instance, but this should be described in the > documentation. Discussing is this API is thread safe or not is not really relevant. Even if you make tracefs_options_get_enabled() thread safe, this does not change anything since the options can be already changed by the time when you examine the bits of the mask. > >> + >> dname = tracefs_instance_get_file(instance, "options"); >> if (!dname) >> goto error; >> @@ -727,7 +728,6 @@ error: >> if (dir) >> closedir(dir); >> tracefs_put_tracing_file(dname); >> - free(bitmask); >> return NULL; >> } >> >> @@ -735,10 +735,10 @@ error: >> * tracefs_options_get_supported - Get all supported trace options in given instance >> * @instance: ftrace instance, can be NULL for the top instance >> * >> - * Returns allocated bitmask structure with all trace options, supported in given >> - * instance, or NULL in case of an error. The returned structure must be freed with free() >> + * Returns bitmask structure with all trace options, supported in given instance, >> + * or NULL in case of an error. >> */ >> -struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instance *instance) >> +const struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instance *instance) >> { >> return trace_get_options(instance, false); > As supported options cannot be changed at runtime, they can be read > only once. The next calls can return the already initialised bitmask > with supported options. If you already have the mask, you do not need to call this function second time. May be if the user intentionally calls the function again, we should allow him to regenerate the mask. What do you think? Thanks! Yordan > >> } >> @@ -747,10 +747,10 @@ struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instan >> * tracefs_options_get_enabled - Get all currently enabled trace options in given instance >> * @instance: ftrace instance, can be NULL for the top instance >> * >> - * Returns allocated bitmask structure with all trace options, enabled in given >> - * instance, or NULL in case of an error. The returned structure must be freed with free() >> + * Returns allocated bitmask structure with all trace options, enabled in given instance, >> + * or NULL in case of an error. >> */ >> -struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance *instance) >> +const struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance *instance) >> { >> return trace_get_options(instance, true); >> } >> @@ -758,7 +758,7 @@ struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance >> static int trace_config_option(struct tracefs_instance *instance, >> enum tracefs_option_id id, bool set) >> { >> - char *set_str = set ? "1" : "0"; >> + const char *set_str = set ? "1" : "0"; >> char file[PATH_MAX]; >> const char *name; >> >> @@ -846,7 +846,7 @@ bool tracefs_option_is_enabled(struct tracefs_instance *instance, enum tracefs_o >> * Returns true if an option with given id is set in the bitmask, >> * false if it is not set. >> */ >> -bool tracefs_option_is_set(struct tracefs_options_mask *options, >> +bool tracefs_option_is_set(const struct tracefs_options_mask *options, >> enum tracefs_option_id id) >> { >> if (id > TRACEFS_OPTION_INVALID) >> -- >> 2.27.0 >> >
On 5.04.21 г. 13:56, Tzvetomir Stoyanov wrote: > On Fri, Apr 2, 2021 at 4:20 PM Yordan Karadzhov (VMware) > <y.karadz@gmail.com> wrote: >> If the instance owns two mask objects, we no longer need to >> dynamically allocate memory in tracefs_options_get_supported() and >> tracefs_options_get_enabled(). This will simplify the code on the >> caller side, since the user is no longer responsible for freeing >> those masks. >> >> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com> >> --- >> Documentation/libtracefs-option-get.txt | 4 +--- >> include/tracefs.h | 6 +++--- >> src/tracefs-instance.c | 24 ++++++++++++------------ >> 3 files changed, 16 insertions(+), 18 deletions(-) >> >> diff --git a/Documentation/libtracefs-option-get.txt b/Documentation/libtracefs-option-get.txt >> index 9b3cb56..3290f24 100644 >> --- a/Documentation/libtracefs-option-get.txt >> +++ b/Documentation/libtracefs-option-get.txt >> @@ -52,14 +52,13 @@ EXAMPLE >> -- >> #include <tracefs.h> >> ... >> -struct tracefs_options_mask *options; >> +const struct tracefs_options_mask *options; >> ... >> options = tracefs_options_get_supported(NULL); >> if (!options) { >> /* Failed to get supported options */ >> } else { >> ... >> - free(options); >> } >> ... >> options = tracefs_options_get_enabled(NULL); >> @@ -67,7 +66,6 @@ if (!options) { >> /* Failed to get options, enabled in the top instance */ >> } else { >> ... >> - free(options); >> } >> ... >> >> diff --git a/include/tracefs.h b/include/tracefs.h >> index 0665e8d..9efa91a 100644 >> --- a/include/tracefs.h >> +++ b/include/tracefs.h >> @@ -132,11 +132,11 @@ enum tracefs_option_id { >> #define TRACEFS_OPTION_MAX (TRACEFS_OPTION_VERBOSE + 1) >> >> struct tracefs_options_mask; >> -bool tracefs_option_is_set(struct tracefs_options_mask *options, >> +bool tracefs_option_is_set(const struct tracefs_options_mask *options, >> enum tracefs_option_id id); >> -struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instance *instance); >> +const struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instance *instance); >> bool tracefs_option_is_supported(struct tracefs_instance *instance, enum tracefs_option_id id); >> -struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance *instance); >> +const struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance *instance); >> bool tracefs_option_is_enabled(struct tracefs_instance *instance, enum tracefs_option_id id); >> 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); >> diff --git a/src/tracefs-instance.c b/src/tracefs-instance.c >> index 49645eb..9cb4b6d 100644 >> --- a/src/tracefs-instance.c >> +++ b/src/tracefs-instance.c >> @@ -27,6 +27,8 @@ struct tracefs_instance { >> char *trace_dir; >> char *name; >> int flags; >> + struct tracefs_options_mask supported_opts; >> + struct tracefs_options_mask enabled_opts; >> }; >> >> /** >> @@ -695,9 +697,8 @@ static struct tracefs_options_mask *trace_get_options(struct tracefs_instance *i >> DIR *dir = NULL; >> long long val; >> >> - bitmask = calloc(1, sizeof(struct tracefs_options_mask)); >> - if (!bitmask) >> - return NULL; >> + bitmask = enabled? &instance->enabled_opts : &instance->supported_opts; > The "instance" argument can be NULL, for the top instance. There > should be global enabled_opts and supported_opts, to be used in that > case. Yes, this is a bug and I have it in v2 as well. Seems like I will need to send v3. Thanks a lot! Y. > > [..] > >
On Mon, 5 Apr 2021 18:01:41 +0300 Yordan Karadzhov <y.karadz@gmail.com> wrote: > > This makes the API not thread safe. There should be no problem with > > supported options, as they cannot be changed at runtime. I think it is > > OK to limit the APIs , as I see no use case to call them from multiple > > threads on the same instance, but this should be described in the > > documentation. > > Discussing is this API is thread safe or not is not really relevant. > Even if you make tracefs_options_get_enabled() thread safe, this does > not change anything since the options can be already changed by the time > when you examine the bits of the mask. You are correct, but you also do not want the update of the bitmask to be corrupted. Thus I think a pthread_mutex() should be added to the updating of the value, such that we don't have a read/modify/write corruption. Sure the reader of the mask may get some stale data if its being updated. But it shouldn't get corrupted data. void tracefs_option_set(struct tracefs_options_mask *options, enum tracefs_option_id id) { if (options && id > TRACEFS_OPTION_INVALID) options->mask |= (1ULL << (id - 1)); } If two threads are doing this to the same mask, you can have an option set that gets cleared by another writer, and even though the option is set after the call, it will be clear in the mask. That is, it's not stale data, it's incorrect data. That is, the readers after that corruption will get an option not set, even though it is. -- Steve
diff --git a/Documentation/libtracefs-option-get.txt b/Documentation/libtracefs-option-get.txt index 9b3cb56..3290f24 100644 --- a/Documentation/libtracefs-option-get.txt +++ b/Documentation/libtracefs-option-get.txt @@ -52,14 +52,13 @@ EXAMPLE -- #include <tracefs.h> ... -struct tracefs_options_mask *options; +const struct tracefs_options_mask *options; ... options = tracefs_options_get_supported(NULL); if (!options) { /* Failed to get supported options */ } else { ... - free(options); } ... options = tracefs_options_get_enabled(NULL); @@ -67,7 +66,6 @@ if (!options) { /* Failed to get options, enabled in the top instance */ } else { ... - free(options); } ... diff --git a/include/tracefs.h b/include/tracefs.h index 0665e8d..9efa91a 100644 --- a/include/tracefs.h +++ b/include/tracefs.h @@ -132,11 +132,11 @@ enum tracefs_option_id { #define TRACEFS_OPTION_MAX (TRACEFS_OPTION_VERBOSE + 1) struct tracefs_options_mask; -bool tracefs_option_is_set(struct tracefs_options_mask *options, +bool tracefs_option_is_set(const struct tracefs_options_mask *options, enum tracefs_option_id id); -struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instance *instance); +const struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instance *instance); bool tracefs_option_is_supported(struct tracefs_instance *instance, enum tracefs_option_id id); -struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance *instance); +const struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance *instance); bool tracefs_option_is_enabled(struct tracefs_instance *instance, enum tracefs_option_id id); 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); diff --git a/src/tracefs-instance.c b/src/tracefs-instance.c index 49645eb..9cb4b6d 100644 --- a/src/tracefs-instance.c +++ b/src/tracefs-instance.c @@ -27,6 +27,8 @@ struct tracefs_instance { char *trace_dir; char *name; int flags; + struct tracefs_options_mask supported_opts; + struct tracefs_options_mask enabled_opts; }; /** @@ -695,9 +697,8 @@ static struct tracefs_options_mask *trace_get_options(struct tracefs_instance *i DIR *dir = NULL; long long val; - bitmask = calloc(1, sizeof(struct tracefs_options_mask)); - if (!bitmask) - return NULL; + bitmask = enabled? &instance->enabled_opts : &instance->supported_opts; + dname = tracefs_instance_get_file(instance, "options"); if (!dname) goto error; @@ -727,7 +728,6 @@ error: if (dir) closedir(dir); tracefs_put_tracing_file(dname); - free(bitmask); return NULL; } @@ -735,10 +735,10 @@ error: * tracefs_options_get_supported - Get all supported trace options in given instance * @instance: ftrace instance, can be NULL for the top instance * - * Returns allocated bitmask structure with all trace options, supported in given - * instance, or NULL in case of an error. The returned structure must be freed with free() + * Returns bitmask structure with all trace options, supported in given instance, + * or NULL in case of an error. */ -struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instance *instance) +const struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instance *instance) { return trace_get_options(instance, false); } @@ -747,10 +747,10 @@ struct tracefs_options_mask *tracefs_options_get_supported(struct tracefs_instan * tracefs_options_get_enabled - Get all currently enabled trace options in given instance * @instance: ftrace instance, can be NULL for the top instance * - * Returns allocated bitmask structure with all trace options, enabled in given - * instance, or NULL in case of an error. The returned structure must be freed with free() + * Returns allocated bitmask structure with all trace options, enabled in given instance, + * or NULL in case of an error. */ -struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance *instance) +const struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance *instance) { return trace_get_options(instance, true); } @@ -758,7 +758,7 @@ struct tracefs_options_mask *tracefs_options_get_enabled(struct tracefs_instance static int trace_config_option(struct tracefs_instance *instance, enum tracefs_option_id id, bool set) { - char *set_str = set ? "1" : "0"; + const char *set_str = set ? "1" : "0"; char file[PATH_MAX]; const char *name; @@ -846,7 +846,7 @@ bool tracefs_option_is_enabled(struct tracefs_instance *instance, enum tracefs_o * Returns true if an option with given id is set in the bitmask, * false if it is not set. */ -bool tracefs_option_is_set(struct tracefs_options_mask *options, +bool tracefs_option_is_set(const struct tracefs_options_mask *options, enum tracefs_option_id id) { if (id > TRACEFS_OPTION_INVALID)
If the instance owns two mask objects, we no longer need to dynamically allocate memory in tracefs_options_get_supported() and tracefs_options_get_enabled(). This will simplify the code on the caller side, since the user is no longer responsible for freeing those masks. Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com> --- Documentation/libtracefs-option-get.txt | 4 +--- include/tracefs.h | 6 +++--- src/tracefs-instance.c | 24 ++++++++++++------------ 3 files changed, 16 insertions(+), 18 deletions(-)