Re: [PATCH v4 05/23] trace-cmd: Add new local function to check if a trace clock is supported

[Steven Rostedt <rostedt@goodmis.org>]

On Thu, 25 Mar 2021 08:40:37 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> A new local function is added to check if a given trace clock is
> supported by the ftrace:
>  clock_is_supported()
> This function is used by the other patches from the set.
> The logic should be part of the tracefs library, when a tracefs API is
> implemeneted, this local funciton will be removed.
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  tracecmd/trace-record.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
> index 635897e1..c7197ba0 100644
> --- a/tracecmd/trace-record.c
> +++ b/tracecmd/trace-record.c
> @@ -5683,6 +5683,34 @@ check_instance_die(struct buffer_instance *instance, char *param)
>  		    tracefs_instance_get_name(instance->tracefs), param);
>  }
>  
> +static bool clock_is_supported(struct tracefs_instance *instance, const char *clock)
> +{
> +	char *all_clocks = NULL;
> +	char *ret = NULL;
> +
> +	all_clocks  = tracefs_instance_file_read(instance, "trace_clock", NULL);
> +	if (!all_clocks)
> +		return false;
> +
> +	ret = strstr(all_clocks, clock);
> +	if (ret && (ret == all_clocks || ret[-1] == ' ' || ret[-1] == '[')) {
> +		switch (ret[strlen(clock)]) {
> +		case ' ':
> +		case '\0':
> +		case ']':
> +		case '\n':
> +			break;
> +		default:
> +			ret = NULL;
> +		}
> +	} else {
> +		ret = NULL;
> +	}
> +	free(all_clocks);
> +
> +	return ret != NULL;
> +}
> +
>  static void parse_record_options(int argc,
>  				 char **argv,
>  				 enum trace_cmd curr_cmd,

This patch could add the check to the -C as well. I went ahead and did that
with the below patch. It doesn't affect the result of the series, it was
just that this functionality could be a stand alone.

I'll apply this entire series.

Thanks Tzvetomir!

-- Steve


From: "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com>
Date: Thu, 25 Mar 2021 08:40:37 +0200
Subject: [PATCH] trace-cmd: Add new local function to check if a trace clock
 is supported

A new local function is added to check if a given trace clock is
supported by the ftrace:
 clock_is_supported()
And test that the passed in clock is supported.
The logic should be part of the tracefs library, when a tracefs API is
implemeneted, this local funciton will be removed.

Link: https://lore.kernel.org/linux-trace-devel/20210325064055.539554-6-tz.stoyanov@gmail.com

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
[ Added test to make sure passed in clock is supported ]
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tracecmd/trace-record.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index e9039cd3..c6c14818 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -5687,6 +5687,34 @@ check_instance_die(struct buffer_instance *instance, char *param)
 		    tracefs_instance_get_name(instance->tracefs), param);
 }
 
+static bool clock_is_supported(struct tracefs_instance *instance, const char *clock)
+{
+	char *all_clocks = NULL;
+	char *ret = NULL;
+
+	all_clocks  = tracefs_instance_file_read(instance, "trace_clock", NULL);
+	if (!all_clocks)
+		return false;
+
+	ret = strstr(all_clocks, clock);
+	if (ret && (ret == all_clocks || ret[-1] == ' ' || ret[-1] == '[')) {
+		switch (ret[strlen(clock)]) {
+		case ' ':
+		case '\0':
+		case ']':
+		case '\n':
+			break;
+		default:
+			ret = NULL;
+		}
+	} else {
+		ret = NULL;
+	}
+	free(all_clocks);
+
+	return ret != NULL;
+}
+
 static void parse_record_options(int argc,
 				 char **argv,
 				 enum trace_cmd curr_cmd,
@@ -5882,6 +5910,8 @@ static void parse_record_options(int argc,
 		case 'C':
 			check_instance_die(ctx->instance, "-C");
 			ctx->instance->clock = optarg;
+			if (!clock_is_supported(NULL, ctx->instance->clock))
+				die("Clock %s is not supported", ctx->instance->clock);
 			ctx->instance->flags |= BUFFER_FL_HAS_CLOCK;
 			if (is_top_instance(ctx->instance))
 				guest_sync_set = true;
-- 
2.29.2