Re: [PATCH 3/9] libtracefs man pages: APIs for locating trace directory and files.

[Tzvetomir Stoyanov <tz.stoyanov@gmail.com>]

On Mon, Dec 21, 2020 at 6:10 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Mon, 21 Dec 2020 05:44:45 +0200
> Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:
>
> > > How should it be freed?
> > I'll add " with free()" in the next version of the patch, but I was
> > wondering if the
> > user should use "tracefs_put_tracing_file()" instead ? These APIs are not
> > consistent, may be they should be renamed.
>
> Good point.
>
> > Now we have:
> >
> >    tracefs_get_tracing_file() / tracefs_put_tracing_file()
> >    tracefs_get_tracing_dir() / returns static, must not be feed.
> >    tracefs_find_tracing_dir() / free()
>
> Perhaps we should change tracefs_get_tracing_dir() to simply:
> tracefs_tracing_dir().
>
>
> Hmm, what's the difference between "tracefs_find_tracing_dir() and
> tracefs_get_tracing_dir() (or what we will call tracefs_tracing_dir())?
>
> The only difference I see from the two descriptions is that one returns
> a cached string and the other returns just an allocated string. Do we
> even need tracefs_find_tracing_dir()?

The tracefs_find_tracing_dir() function is used in trace-cmd, but I agree that
it can be removed. I'll rename tracefs_get_tracing_dir() to
tracefs_tracing_dir()
and remove tracefs_find_tracing_dir() API. Going to change trace-cmd to use
only tracefs_tracing_dir() API.

>
> >
> >
> > >
> > > > +
> > > > +The _tracefs_get_tracing_dir()_ function returns the full path to the
> > > > +trace file system. In the first function call, the mount point of the
> > > > +tracing file system is located, cached and returned. It will mount it,
> > > > +if it is not minted. On any subsequent call the cached path is returned.
>
> Just noticed the above typo. s/minted/mounted/
>
> Although,  I wonder if a minted dir tastes good? ;-)
>
> -- Steve
>
> > > > +The return string must _not_ be freed.
> > > > +
> > > > +RETURN VALUE
> > > > +------------
> > > > +The _tracefs_get_tracing_file()_ function returns a string or NULL in case
> > > > +of an error. The returned string must be freed with _tracefs_put_tracing_file()_.
> > > > +
> > > > +The _tracefs_find_tracing_dir()_ function returns a string or NULL in case
> > > > +of an error. The returned string must be freed.
> > >
> > > Should state how it should be freed. tracefs_put_tracing_file() or free() ?
> > >
> > > If it is free(), then stating:
> > >
> > >  "The returned string must be freed with free()"
> > >
> > > is fine.
> > >
> > > > +
> > > > +The _tracefs_get_tracing_dir()_ function returns a constant string or NULL
> > > > +in case of an error. The returned string must _not_ be freed.
> > > > +
> > > > +EXAMPLE
> > > > +-------
> > > > +[source,c]
> > > > +--
> > > > +#include <tracefs.h>
> > > > +...
> > > > +char *trace_on = tracefs_get_tracing_file("tracing_on");
> > > > +     if (trace_on) {
> > > > +             ...
> > > > +             tracefs_put_tracing_file(trace_on);
> > > > +     }
> > > > +...
> > > > +char *trace_dir = tracefs_find_tracing_dir();
> > > > +     if (trace_dir) {
> > > > +             ...
> > > > +             free(trace_dir);
> > > > +     }
> > > > +...
> > > > +const char *trace_dir = tracefs_get_tracing_dir();
> > > > +
> > >
> > > Not a very useful example ;-)  We we can fix examples at a later time. I
> > > plan on writing a lot of example code to post, and some can make their way
> > > into the man pages.
> > >
> > > -- Steve
> >
> >
> >
>


-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center