> On Mar 19, 2019, at 8:55 AM, Tzvetomir Stoyanov <tstoyanov@xxxxxxxxxx> wrote:
>
> In order to reuse the code inside the trace-cmd application, few
> functions from trace-record.c are refactored:
> - make_instances() and tracecmd_remove_instances() are splited.
> New ones are created: tracecmd_make_instance() and tracecmd_remove_instance(),
> which are visible outside the trace-record.c
> - Following functions are made non-static: tracecmd_init_instance()
> get_instance_dir(), write_instance_file(), write_tracing_on(),
> tracecmd_set_clock()
> - New function is implemented: tracecmd_local_cpu_count(), an internal
> API to get local_cpu_count.
>
> Signed-off-by: Tzvetomir Stoyanov <tstoyanov@xxxxxxxxxx>
> ---
> tracecmd/include/trace-local.h | 9 ++++
> tracecmd/trace-record.c | 87 +++++++++++++++++++---------------
> 2 files changed, 58 insertions(+), 38 deletions(-)
>
> diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
> index d7bdb1f..8413054 100644
> --- a/tracecmd/include/trace-local.h
> +++ b/tracecmd/include/trace-local.h
> @@ -235,6 +235,15 @@ void update_first_instance(struct buffer_instance *instance, int topt);
> void show_instance_file(struct buffer_instance *instance, const char *name);
>
> int count_cpus(void);
> +void write_tracing_on(struct buffer_instance *instance, int on);
> +char *get_instance_dir(struct buffer_instance *instance);
> +int write_instance_file(struct buffer_instance *instance,
> + const char *file, const char *str, const char *type);
> +void tracecmd_init_instance(struct buffer_instance *instance);
> +void tracecmd_make_instance(struct buffer_instance *instance);
> +int tracecmd_local_cpu_count(void);
> +void tracecmd_set_clock(struct buffer_instance *instance);
> +void tracecmd_remove_instance(struct buffer_instance *instance);
>
> /* No longer in event-utils.h */
> void __noreturn die(const char *fmt, ...); /* Can be overriden */
> diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
> index 177060d..3950242 100644
> --- a/tracecmd/trace-record.c
> +++ b/tracecmd/trace-record.c
>
<snipped>
> -static void set_clock(struct buffer_instance *instance)
> +void tracecmd_set_clock(struct buffer_instance *instance)
> {
> char *path;
> char *content;
> @@ -4316,49 +4314,57 @@ static void clear_func_filters(void)
> }
> }
>
> -static void make_instances(void)
> +void tracecmd_make_instance(struct buffer_instance *instance)
> {
> - struct buffer_instance *instance;
> struct stat st;
> char *path;
> int ret;
>
> + path = get_instance_dir(instance);
> + ret = stat(path, &st);
> + if (ret < 0) {
> + ret = mkdir(path, 0777);
Here is the mkdir ...
> + if (ret < 0)
> + die("mkdir %s", path);
> + } else
> + /* Don't delete instances that already exist */
> + instance->flags |= BUFFER_FL_KEEP;
> + tracecmd_put_tracing_file(path);
> +
> +}
> +
> +static void make_instances(void)
> +{
> + struct buffer_instance *instance;
> +
> for_each_instance(instance) {
> if (is_guest(instance))
> continue;
> + tracecmd_make_instance(instance);
> + }
> +}
>
> - path = get_instance_dir(instance);
> - ret = stat(path, &st);
> - if (ret < 0) {
> - ret = mkdir(path, 0777);
> - if (ret < 0)
> - die("mkdir %s", path);
> - } else
> - /* Don't delete instances that already exist */
> - instance->flags |= BUFFER_FL_KEEP;
> - tracecmd_put_tracing_file(path);
> +void tracecmd_remove_instance(struct buffer_instance *instance)
> +{
> + char *path;
> +
> + if (instance->tracing_on_fd > 0) {
> + close(instance->tracing_on_fd);
> + instance->tracing_on_fd = 0;
> }
> + path = get_instance_dir(instance);
> + tracecmd_put_tracing_file(path);
> }
>
> void tracecmd_remove_instances(void)
> {
> struct buffer_instance *instance;
> - char *path;
> - int ret;
>
> for_each_instance(instance) {
> /* Only delete what we created */
> if (is_guest(instance) || (instance->flags & BUFFER_FL_KEEP))
> continue;
> - if (instance->tracing_on_fd > 0) {
> - close(instance->tracing_on_fd);
> - instance->tracing_on_fd = 0;
> - }
> - path = get_instance_dir(instance);
> - ret = rmdir(path);
But this rmdir is not moved to tracecmd_remove_instance() — it’s lost. This looks like it’s introducing a bug.
> - if (ret < 0)
> - die("rmdir %s", path);
> - tracecmd_put_tracing_file(path);
> + tracecmd_remove_instance(instance);
> }
> }
<snipped>
The rest looks fine.
Cheers,
-Matt Helsley
|