| Message ID | 20200225180733.89344-2-tz.stoyanov@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Steven Rostedt |
| Headers | show |
| Series | Extend coverage of "trace-cmd reset" command | expand |
On Tue, 25 Feb 2020 20:07:30 +0200 "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > Some ftrace files and directories are optional, depending on specific kernel configuration or version. > It is a good practice to check if the file / directory exist, before trying to access it. There are a lot of places in trace-cmd implementation with such checks. > The new libtracefs APIs can be used for this, they are ftarce instance aware: > bool tracefs_file_exist(struct tracefs_instance *instance, char *name); > bool tracefs_dir_exist(struct tracefs_instance *instance, char *name); > > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> > --- > include/tracefs/tracefs.h | 3 +++ > lib/tracefs/tracefs-instance.c | 49 +++++++++++++++++++++++++++++++++- > 2 files changed, 51 insertions(+), 1 deletion(-) > > diff --git a/include/tracefs/tracefs.h b/include/tracefs/tracefs.h > index 85690b66..bc8bebcb 100644 > --- a/include/tracefs/tracefs.h > +++ b/include/tracefs/tracefs.h > @@ -33,6 +33,9 @@ int tracefs_instance_file_write(struct tracefs_instance *instance, > char *tracefs_instance_file_read(struct tracefs_instance *instance, > char *file, int *psize); > > +bool tracefs_file_exist(struct tracefs_instance *instance, char *name); > +bool tracefs_dir_exist(struct tracefs_instance *instance, char *name); > + > /* events */ > void tracefs_list_free(char **list); > char **tracefs_event_systems(const char *tracing_dir); > diff --git a/lib/tracefs/tracefs-instance.c b/lib/tracefs/tracefs-instance.c > index b96ab61c..67123e7c 100644 > --- a/lib/tracefs/tracefs-instance.c > +++ b/lib/tracefs/tracefs-instance.c > @@ -13,7 +13,7 @@ > #include <errno.h> > #include <sys/stat.h> > #include <fcntl.h> > - > +#include <linux/limits.h> > #include "tracefs.h" > #include "tracefs-local.h" > > @@ -247,3 +247,50 @@ char *tracefs_instance_file_read(struct tracefs_instance *instance, > > return buf; > } > + > +static bool check_file_exist(struct tracefs_instance *instance, > + char *name, bool dir) > +{ > + char file[PATH_MAX]; > + struct stat st; > + char *path; > + int ret; > + > + path = tracefs_instance_get_dir(instance); > + snprintf(file, PATH_MAX, "%s/%s", path, name); > + tracefs_put_tracing_file(path); > + ret = stat(file, &st); > + if (ret < 0) > + return false; > + > + if (dir && !S_ISDIR(st.st_mode)) > + return false; > + if (!dir && S_ISDIR(st.st_mode)) > + return false; > + > + return true; BTW, the above can be converted to just: return !dir == !S_ISDIR(st.st_mode); > +} > + > +/** > + * tracefs_file_exist - Check if a file with given name exists in given instance > + * @instance: ftrace instance, can be NULL for the top instance > + * @name: name of the file > + * > + * Returns true if the file exists, false otherwise Probably should comment that this will return false if the "file" exists but is a directory. Is there any reason we would not want to return true here if it was a directory? A directory is just another kind of "file" (in the Unix world). -- Steve > + */ > +bool tracefs_file_exist(struct tracefs_instance *instance, char *name) > +{ > + return check_file_exist(instance, name, false); > +} > + > +/** > + * tracefs_dir_exist - Check if a directory with given name exists in given instance > + * @instance: ftrace instance, can be NULL for the top instance > + * @name: name of the directory > + * > + * Returns true if the directory exists, false otherwise > + */ > +bool tracefs_dir_exist(struct tracefs_instance *instance, char *name) > +{ > + return check_file_exist(instance, name, true); > +}
On Thu, Apr 9, 2020 at 9:22 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Tue, 25 Feb 2020 20:07:30 +0200 > "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > > > Some ftrace files and directories are optional, depending on specific kernel configuration or version. > > It is a good practice to check if the file / directory exist, before trying to access it. There are a lot of places in trace-cmd implementation with such checks. > > The new libtracefs APIs can be used for this, they are ftarce instance aware: > > bool tracefs_file_exist(struct tracefs_instance *instance, char *name); > > bool tracefs_dir_exist(struct tracefs_instance *instance, char *name); > > > > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> > > --- > > include/tracefs/tracefs.h | 3 +++ > > lib/tracefs/tracefs-instance.c | 49 +++++++++++++++++++++++++++++++++- > > 2 files changed, 51 insertions(+), 1 deletion(-) > > > > diff --git a/include/tracefs/tracefs.h b/include/tracefs/tracefs.h > > index 85690b66..bc8bebcb 100644 > > --- a/include/tracefs/tracefs.h > > +++ b/include/tracefs/tracefs.h > > @@ -33,6 +33,9 @@ int tracefs_instance_file_write(struct tracefs_instance *instance, > > char *tracefs_instance_file_read(struct tracefs_instance *instance, > > char *file, int *psize); > > > > +bool tracefs_file_exist(struct tracefs_instance *instance, char *name); > > +bool tracefs_dir_exist(struct tracefs_instance *instance, char *name); > > + > > /* events */ > > void tracefs_list_free(char **list); > > char **tracefs_event_systems(const char *tracing_dir); > > diff --git a/lib/tracefs/tracefs-instance.c b/lib/tracefs/tracefs-instance.c > > index b96ab61c..67123e7c 100644 > > --- a/lib/tracefs/tracefs-instance.c > > +++ b/lib/tracefs/tracefs-instance.c > > @@ -13,7 +13,7 @@ > > #include <errno.h> > > #include <sys/stat.h> > > #include <fcntl.h> > > - > > +#include <linux/limits.h> > > #include "tracefs.h" > > #include "tracefs-local.h" > > > > @@ -247,3 +247,50 @@ char *tracefs_instance_file_read(struct tracefs_instance *instance, > > > > return buf; > > } > > + > > +static bool check_file_exist(struct tracefs_instance *instance, > > + char *name, bool dir) > > +{ > > + char file[PATH_MAX]; > > + struct stat st; > > + char *path; > > + int ret; > > + > > + path = tracefs_instance_get_dir(instance); > > + snprintf(file, PATH_MAX, "%s/%s", path, name); > > + tracefs_put_tracing_file(path); > > + ret = stat(file, &st); > > + if (ret < 0) > > + return false; > > + > > + if (dir && !S_ISDIR(st.st_mode)) > > + return false; > > + if (!dir && S_ISDIR(st.st_mode)) > > + return false; > > + > > + return true; > > BTW, the above can be converted to just: > > return !dir == !S_ISDIR(st.st_mode); > > > > +} > > + > > +/** > > + * tracefs_file_exist - Check if a file with given name exists in given instance > > + * @instance: ftrace instance, can be NULL for the top instance > > + * @name: name of the file > > + * > > + * Returns true if the file exists, false otherwise > > Probably should comment that this will return false if the "file" exists > but is a directory. > > Is there any reason we would not want to return true here if it was a > directory? A directory is just another kind of "file" (in the Unix world). > The use case of the API is to check if a given file exists in tracefs, before writing to / reading from it. If the user expects file, but it is a directory - the operation will fail. We can think of adding additional input parameter to the API, to let the user specify the desired behavior of the API ? Something like : bool tracefs_file_exist(struct tracefs_instance *instance, char *name, bool any); > -- Steve > > > + */ > > +bool tracefs_file_exist(struct tracefs_instance *instance, char *name) > > +{ > > + return check_file_exist(instance, name, false); > > +} > > + > > +/** > > + * tracefs_dir_exist - Check if a directory with given name exists in given instance > > + * @instance: ftrace instance, can be NULL for the top instance > > + * @name: name of the directory > > + * > > + * Returns true if the directory exists, false otherwise > > + */ > > +bool tracefs_dir_exist(struct tracefs_instance *instance, char *name) > > +{ > > + return check_file_exist(instance, name, true); > > +} >
diff --git a/include/tracefs/tracefs.h b/include/tracefs/tracefs.h index 85690b66..bc8bebcb 100644 --- a/include/tracefs/tracefs.h +++ b/include/tracefs/tracefs.h @@ -33,6 +33,9 @@ int tracefs_instance_file_write(struct tracefs_instance *instance, char *tracefs_instance_file_read(struct tracefs_instance *instance, char *file, int *psize); +bool tracefs_file_exist(struct tracefs_instance *instance, char *name); +bool tracefs_dir_exist(struct tracefs_instance *instance, char *name); + /* events */ void tracefs_list_free(char **list); char **tracefs_event_systems(const char *tracing_dir); diff --git a/lib/tracefs/tracefs-instance.c b/lib/tracefs/tracefs-instance.c index b96ab61c..67123e7c 100644 --- a/lib/tracefs/tracefs-instance.c +++ b/lib/tracefs/tracefs-instance.c @@ -13,7 +13,7 @@ #include <errno.h> #include <sys/stat.h> #include <fcntl.h> - +#include <linux/limits.h> #include "tracefs.h" #include "tracefs-local.h" @@ -247,3 +247,50 @@ char *tracefs_instance_file_read(struct tracefs_instance *instance, return buf; } + +static bool check_file_exist(struct tracefs_instance *instance, + char *name, bool dir) +{ + char file[PATH_MAX]; + struct stat st; + char *path; + int ret; + + path = tracefs_instance_get_dir(instance); + snprintf(file, PATH_MAX, "%s/%s", path, name); + tracefs_put_tracing_file(path); + ret = stat(file, &st); + if (ret < 0) + return false; + + if (dir && !S_ISDIR(st.st_mode)) + return false; + if (!dir && S_ISDIR(st.st_mode)) + return false; + + return true; +} + +/** + * tracefs_file_exist - Check if a file with given name exists in given instance + * @instance: ftrace instance, can be NULL for the top instance + * @name: name of the file + * + * Returns true if the file exists, false otherwise + */ +bool tracefs_file_exist(struct tracefs_instance *instance, char *name) +{ + return check_file_exist(instance, name, false); +} + +/** + * tracefs_dir_exist - Check if a directory with given name exists in given instance + * @instance: ftrace instance, can be NULL for the top instance + * @name: name of the directory + * + * Returns true if the directory exists, false otherwise + */ +bool tracefs_dir_exist(struct tracefs_instance *instance, char *name) +{ + return check_file_exist(instance, name, true); +}
Some ftrace files and directories are optional, depending on specific kernel configuration or version. It is a good practice to check if the file / directory exist, before trying to access it. There are a lot of places in trace-cmd implementation with such checks. The new libtracefs APIs can be used for this, they are ftarce instance aware: bool tracefs_file_exist(struct tracefs_instance *instance, char *name); bool tracefs_dir_exist(struct tracefs_instance *instance, char *name); Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> --- include/tracefs/tracefs.h | 3 +++ lib/tracefs/tracefs-instance.c | 49 +++++++++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 1 deletion(-)