Re: [PATCH v3 6/6] trace-cmd: Add new libtrasefs API to get the current trace clock

[Steven Rostedt <rostedt@goodmis.org>]

On Thu, 12 Nov 2020 11:11:09 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

>  static void report_clock(struct buffer_instance *instance)
>  {
> -	char *str;
> -	char *cont;
>  	char *clock;
>  
> -	str = get_instance_file_content(instance, "trace_clock");
> -	if (!str)
> -		return;
> -
> -	clock = strstr(str, "[");
> -	if (!clock)
> -		goto out;
> -	clock++;
> -
> -	cont = strstr(clock, "]");
> -	if (!cont) /* should never happen */
> -		goto out;
> -
> -	*cont = '\0';
> +	if (instance)
> +		clock = tracefs_get_clock(instance->tracefs);
> +	else
> +		clock = tracefs_get_clock(NULL);

Small nit, but I find it cleaner in a case that both sides of an if
condition do basically the same thing with a different variable, to just
set the variable:

	struct tracefs_instance *tracefs = instance ? instance->tracefs : NULL;

	[..]

	clock = tracefs_get_clock(tracefs);

Other than that, the rest looks good.

Thanks Tzvetomir!

-- Steve


>  
>  	/* Default clock is "local", only show others */
> -	if (strcmp(clock, "local") == 0)
> -		goto out;
> +	if (clock && !strcmp(clock, "local") == 0)
> +		printf("\nClock: %s\n", clock);
>  
> -	printf("\nClock: %s\n", clock);
> -
> - out:
> -	free(str);
> +	free(clock);
>  }
>  
>  static void report_cpumask(struct buffer_instance *instance)
> diff --git a/utest/tracefs-utest.c b/utest/tracefs-utest.c
> index c42fea12..78eda481 100644
> --- a/utest/tracefs-utest.c
> +++ b/utest/tracefs-utest.c
> @@ -511,6 +511,33 @@ static void test_instances_walk(void)
>  	}
>  }
>