[PATCH 0/7] trace-cmd,libtraceevent: Rework of plugin options APIs

[PATCH 0/7] trace-cmd,libtraceevent: Rework of plugin options APIs

[Tzvetomir Stoyanov (VMware)]

In order to simplify libtracevents APIs, logic related to plugins and
plugin options is reworked:
 - Complex and not used APIs are removed.
 - Logic for printing various information for plugins and plugin options
   is moved from the library to the application.
 - Additional checks are added when loading new plugins and options, to
   avoid duplications.

Tzvetomir Stoyanov (VMware) (7):
  trace-cmd,libtraceevent: Plugin options rework
  trace-cmd,libtraceevent: Remove TEP_PLUGIN_OPTIONS
  trace-cmd,libtraceevent: Check for plugin options duplication
  trace-cmd,libtraceevent: Check for plugin duplication
  trace-cmd,libtraceevent: Remove API for plugin options print
  trace-cmd,libtraceevent: Remove API for plugin print
  trace-cmd, kernelshark, libtraceevent: Changed tep_plugin_add_option()
    API

 include/traceevent/event-parse.h         |  50 +---
 kernel-shark/src/libkshark.c             |   4 +-
 lib/trace-cmd/trace-ftrace.c             |   4 +-
 lib/trace-cmd/trace-plugin.c             |   5 -
 lib/traceevent/event-plugin.c            | 279 ++++++++++-------------
 lib/traceevent/plugins/plugin_function.c |   6 +-
 tracecmd/trace-list.c                    |  55 ++++-
 tracecmd/trace-read.c                    |   2 +-
 8 files changed, 188 insertions(+), 217 deletions(-)

-- 
2.24.1

[PATCH 1/7] trace-cmd,libtraceevent: Plugin options rework

[Tzvetomir Stoyanov (VMware)]

When registering a plugin option, the current API
allows to set an alias to the option's plugin.
This logic complicates the implementation, but is
not used by any existing plugin.
In order to simplify the libtracevent API, these
changes are introduced, related to plugin options:
  - Removed "plugin_alias" from options and all
    logic associated with it.
  - Renamed "file" field to "plugin", the new name
    describes more closely its purpose.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/traceevent/event-parse.h         | 13 +-------
 lib/trace-cmd/trace-ftrace.c             |  4 +--
 lib/trace-cmd/trace-plugin.c             |  5 ---
 lib/traceevent/event-plugin.c            | 41 +++++++-----------------
 lib/traceevent/plugins/plugin_function.c |  6 ++--
 5 files changed, 17 insertions(+), 52 deletions(-)

diff --git a/include/traceevent/event-parse.h b/include/traceevent/event-parse.h
index 52bafa5..72f7aaf 100644
--- a/include/traceevent/event-parse.h
+++ b/include/traceevent/event-parse.h
@@ -56,9 +56,8 @@ typedef int (*tep_plugin_unload_func)(struct tep_handle *tep);
 struct tep_plugin_option {
 	struct tep_plugin_option	*next;
 	void				*handle;
-	char				*file;
+	char				*plugin;
 	char				*name;
-	char				*plugin_alias;
 	char				*description;
 	const char			*value;
 	void				*priv;
@@ -84,7 +83,6 @@ struct tep_plugin_option {
  *   struct tep_plugin_option TEP_PLUGIN_OPTIONS[] = {
  *	{
  *		.name = "option-name",
- *		.plugin_alias = "override-file-name", (optional)
  *		.description = "description of option to show users",
  *	},
  *	{
@@ -94,27 +92,18 @@ struct tep_plugin_option {
  *
  *   Array must end with .name = NULL;
  *
- *
- *   .plugin_alias is used to give a shorter name to access
- *   the vairable. Useful if a plugin handles more than one event.
- *
  *   If .value is not set, then it is considered a boolean and only
  *   .set will be processed. If .value is defined, then it is considered
  *   a string option and .set will be ignored.
- *
- * TEP_PLUGIN_ALIAS: (optional)
- *   The name to use for finding options (uses filename if not defined)
  */
 #define TEP_PLUGIN_LOADER tep_plugin_loader
 #define TEP_PLUGIN_UNLOADER tep_plugin_unloader
 #define TEP_PLUGIN_OPTIONS tep_plugin_options
-#define TEP_PLUGIN_ALIAS tep_plugin_alias
 #define _MAKE_STR(x)	#x
 #define MAKE_STR(x)	_MAKE_STR(x)
 #define TEP_PLUGIN_LOADER_NAME MAKE_STR(TEP_PLUGIN_LOADER)
 #define TEP_PLUGIN_UNLOADER_NAME MAKE_STR(TEP_PLUGIN_UNLOADER)
 #define TEP_PLUGIN_OPTIONS_NAME MAKE_STR(TEP_PLUGIN_OPTIONS)
-#define TEP_PLUGIN_ALIAS_NAME MAKE_STR(TEP_PLUGIN_ALIAS)
 
 enum tep_format_flags {
 	TEP_FIELD_IS_ARRAY	= 1,
diff --git a/lib/trace-cmd/trace-ftrace.c b/lib/trace-cmd/trace-ftrace.c
index 20bf71f..f251ad1 100644
--- a/lib/trace-cmd/trace-ftrace.c
+++ b/lib/trace-cmd/trace-ftrace.c
@@ -12,14 +12,14 @@
 
 struct tep_plugin_option trace_ftrace_options[] = {
 	{
+		.plugin = "ftrace",
 		.name = "tailprint",
-		.plugin_alias = "fgraph",
 		.description =
 		"Print function name at function exit in function graph",
 	},
 	{
+		.plugin = "ftrace",
 		.name = "depth",
-		.plugin_alias = "fgraph",
 		.description =
 		"Show the depth of each entry",
 	},
diff --git a/lib/trace-cmd/trace-plugin.c b/lib/trace-cmd/trace-plugin.c
index 6bec18b..ce2f062 100644
--- a/lib/trace-cmd/trace-plugin.c
+++ b/lib/trace-cmd/trace-plugin.c
@@ -100,7 +100,6 @@ load_plugin(struct trace_plugin_context *trace, const char *path,
 	struct trace_plugin_list **plugin_list = data;
 	tracecmd_plugin_load_func func;
 	struct trace_plugin_list *list;
-	const char *alias;
 	char *plugin;
 	void *handle;
 	int ret;
@@ -118,10 +117,6 @@ load_plugin(struct trace_plugin_context *trace, const char *path,
 		goto out_free;
 	}
 
-	alias = dlsym(handle, TRACECMD_PLUGIN_ALIAS_NAME);
-	if (!alias)
-		alias = file;
-
 	func = dlsym(handle, TRACECMD_PLUGIN_LOADER_NAME);
 	if (!func) {
 		warning("could not find func '%s' in plugin '%s'\n%s\n",
diff --git a/lib/traceevent/event-plugin.c b/lib/traceevent/event-plugin.c
index 30c1526..bbe87d4 100644
--- a/lib/traceevent/event-plugin.c
+++ b/lib/traceevent/event-plugin.c
@@ -113,11 +113,10 @@ char **tep_plugin_list_options(void)
 
 	for (reg = registered_options; reg; reg = reg->next) {
 		for (op = reg->options; op->name; op++) {
-			char *alias = op->plugin_alias ? op->plugin_alias : op->file;
 			char **temp = list;
 			int ret;
 
-			ret = asprintf(&name, "%s:%s", alias, op->name);
+			ret = asprintf(&name, "%s:%s", op->plugin, op->name);
 			if (ret < 0)
 				goto err;
 
@@ -163,20 +162,14 @@ update_option(const char *file, struct tep_plugin_option *option)
 	struct trace_plugin_options *op;
 	char *plugin;
 	int ret = 0;
+	char *p;
 
-	if (option->plugin_alias) {
-		plugin = strdup(option->plugin_alias);
-		if (!plugin)
-			return -1;
-	} else {
-		char *p;
-		plugin = strdup(file);
-		if (!plugin)
-			return -1;
-		p = strstr(plugin, ".");
-		if (p)
-			*p = '\0';
-	}
+	plugin = strdup(file);
+	if (!plugin)
+		return -1;
+	p = strstr(plugin, ".");
+	if (p)
+		*p = '\0';
 
 	/* first look for named options */
 	for (op = trace_plugin_options; op; op = op->next) {
@@ -274,16 +267,10 @@ find_registered_option(const char *plugin, const char *option)
 {
 	struct registered_plugin_options *reg;
 	struct tep_plugin_option *op;
-	const char *op_plugin;
 
 	for (reg = registered_options; reg; reg = reg->next) {
 		for (op = reg->options; op->name; op++) {
-			if (op->plugin_alias)
-				op_plugin = op->plugin_alias;
-			else
-				op_plugin = op->file;
-
-			if (plugin && strcmp(plugin, op_plugin) != 0)
+			if (plugin && strcmp(plugin, op->plugin) != 0)
 				continue;
 			if (strcmp(option, op->name) != 0)
 				continue;
@@ -404,8 +391,7 @@ void tep_plugin_print_options(struct trace_seq *s)
 		for (op = reg->options; op->name; op++) {
 			if (op != reg->options)
 				trace_seq_printf(s, "------------\n");
-			print_op_data(s, "file", op->file);
-			print_op_data(s, "plugin", op->plugin_alias);
+			print_op_data(s, "plugin", op->plugin);
 			print_op_data(s, "option", op->name);
 			print_op_data(s, "desc", op->description);
 			print_op_data(s, "value", op->value);
@@ -444,7 +430,6 @@ load_plugin(struct tep_handle *tep, const char *path,
 	struct tep_plugin_option *options;
 	tep_plugin_load_func func;
 	struct tep_plugin_list *list;
-	const char *alias;
 	char *plugin;
 	void *handle;
 	int ret;
@@ -462,14 +447,10 @@ load_plugin(struct tep_handle *tep, const char *path,
 		goto out_free;
 	}
 
-	alias = dlsym(handle, TEP_PLUGIN_ALIAS_NAME);
-	if (!alias)
-		alias = file;
-
 	options = dlsym(handle, TEP_PLUGIN_OPTIONS_NAME);
 	if (options) {
 		while (options->name) {
-			ret = update_option(alias, options);
+			ret = update_option(file, options);
 			if (ret < 0)
 				goto out_free;
 			options++;
diff --git a/lib/traceevent/plugins/plugin_function.c b/lib/traceevent/plugins/plugin_function.c
index 938b741..8c36920 100644
--- a/lib/traceevent/plugins/plugin_function.c
+++ b/lib/traceevent/plugins/plugin_function.c
@@ -22,21 +22,21 @@ static int cpus = -1;
 struct tep_plugin_option plugin_options[] =
 {
 	{
+		.plugin = "ftrace",
 		.name = "parent",
-		.plugin_alias = "ftrace",
 		.description =
 		"Print parent of functions for function events",
 	},
 	{
+		.plugin = "ftrace",
 		.name = "indent",
-		.plugin_alias = "ftrace",
 		.description =
 		"Try to show function call indents, based on parents",
 		.set = 1,
 	},
 	{
+		.plugin = "ftrace",
 		.name = "offset",
-		.plugin_alias = "ftrace",
 		.description =
 		"Show function names as well as their offsets",
 		.set = 0,
-- 
2.24.1

[PATCH 2/7] trace-cmd,libtraceevent: Remove TEP_PLUGIN_OPTIONS

[Tzvetomir Stoyanov (VMware)]

When loading a plugin, the current API searches for
TEP_PLUGIN_OPTIONS array in plugin's symbols and loads
any options from it. This API duplicates
 tep_plugin_add_options()
In order to simplify the APIs, The duplicated TEP_PLUGIN_OPTIONS
is removed as an API, as it is not used by any existing plugin.
The tep_plugin_add_options() API is the only way to register
plugin options.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/traceevent/event-parse.h | 20 --------------------
 lib/traceevent/event-plugin.c    | 11 -----------
 2 files changed, 31 deletions(-)

diff --git a/include/traceevent/event-parse.h b/include/traceevent/event-parse.h
index 72f7aaf..cb1b46e 100644
--- a/include/traceevent/event-parse.h
+++ b/include/traceevent/event-parse.h
@@ -77,33 +77,13 @@ struct tep_plugin_option {
  *
  *   int TEP_PLUGIN_UNLOADER(struct tep_handle *tep)
  *
- * TEP_PLUGIN_OPTIONS:  (optional)
- *   Plugin options that can be set before loading
- *
- *   struct tep_plugin_option TEP_PLUGIN_OPTIONS[] = {
- *	{
- *		.name = "option-name",
- *		.description = "description of option to show users",
- *	},
- *	{
- *		.name = NULL,
- *	},
- *   };
- *
- *   Array must end with .name = NULL;
- *
- *   If .value is not set, then it is considered a boolean and only
- *   .set will be processed. If .value is defined, then it is considered
- *   a string option and .set will be ignored.
  */
 #define TEP_PLUGIN_LOADER tep_plugin_loader
 #define TEP_PLUGIN_UNLOADER tep_plugin_unloader
-#define TEP_PLUGIN_OPTIONS tep_plugin_options
 #define _MAKE_STR(x)	#x
 #define MAKE_STR(x)	_MAKE_STR(x)
 #define TEP_PLUGIN_LOADER_NAME MAKE_STR(TEP_PLUGIN_LOADER)
 #define TEP_PLUGIN_UNLOADER_NAME MAKE_STR(TEP_PLUGIN_UNLOADER)
-#define TEP_PLUGIN_OPTIONS_NAME MAKE_STR(TEP_PLUGIN_OPTIONS)
 
 enum tep_format_flags {
 	TEP_FIELD_IS_ARRAY	= 1,
diff --git a/lib/traceevent/event-plugin.c b/lib/traceevent/event-plugin.c
index bbe87d4..8c48ccf 100644
--- a/lib/traceevent/event-plugin.c
+++ b/lib/traceevent/event-plugin.c
@@ -427,7 +427,6 @@ load_plugin(struct tep_handle *tep, const char *path,
 	    const char *file, void *data)
 {
 	struct tep_plugin_list **plugin_list = data;
-	struct tep_plugin_option *options;
 	tep_plugin_load_func func;
 	struct tep_plugin_list *list;
 	char *plugin;
@@ -447,16 +446,6 @@ load_plugin(struct tep_handle *tep, const char *path,
 		goto out_free;
 	}
 
-	options = dlsym(handle, TEP_PLUGIN_OPTIONS_NAME);
-	if (options) {
-		while (options->name) {
-			ret = update_option(file, options);
-			if (ret < 0)
-				goto out_free;
-			options++;
-		}
-	}
-
 	func = dlsym(handle, TEP_PLUGIN_LOADER_NAME);
 	if (!func) {
 		warning("could not find func '%s' in plugin '%s'\n%s\n",
-- 
2.24.1

[PATCH 3/7] trace-cmd,libtraceevent: Check for plugin options duplication

[Tzvetomir Stoyanov (VMware)]

When loading new plugin options, check is an option with the same
name and plugin is already registered. The API
 tep_plugin_add_options()
is modified to return errors is these cases:
  - The "plugin" field in the option's description is not set,
    return -EINVAL
  - An option with the same name and plugin is already registered
    return -EBUSY

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/traceevent/event-plugin.c | 50 +++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 20 deletions(-)

diff --git a/lib/traceevent/event-plugin.c b/lib/traceevent/event-plugin.c
index 8c48ccf..e53b09c 100644
--- a/lib/traceevent/event-plugin.c
+++ b/lib/traceevent/event-plugin.c
@@ -202,6 +202,26 @@ update_option(const char *file, struct tep_plugin_option *option)
 	return ret;
 }
 
+static struct tep_plugin_option *
+find_registered_option(const char *plugin, const char *option)
+{
+	struct registered_plugin_options *reg;
+	struct tep_plugin_option *op;
+
+	for (reg = registered_options; reg; reg = reg->next) {
+		for (op = reg->options; op->name; op++) {
+			if (plugin && strcmp(plugin, op->plugin) != 0)
+				continue;
+			if (strcmp(option, op->name) != 0)
+				continue;
+
+			return op;
+		}
+	}
+
+	return NULL;
+}
+
 /**
  * tep_plugin_add_options - Add a set of options by a plugin
  * @name: The name of the plugin adding the options
@@ -213,6 +233,16 @@ int tep_plugin_add_options(const char *name,
 			   struct tep_plugin_option *options)
 {
 	struct registered_plugin_options *reg;
+	struct tep_plugin_option *option;
+
+	option = options;
+	while (option && option->name) {
+		if (!option->plugin)
+			return -EINVAL;
+		if (find_registered_option(name, option->name))
+			return -EBUSY;
+		option++;
+	}
 
 	reg = malloc(sizeof(*reg));
 	if (!reg)
@@ -262,26 +292,6 @@ static void parse_option_name(char **option, char **plugin)
 	}
 }
 
-static struct tep_plugin_option *
-find_registered_option(const char *plugin, const char *option)
-{
-	struct registered_plugin_options *reg;
-	struct tep_plugin_option *op;
-
-	for (reg = registered_options; reg; reg = reg->next) {
-		for (op = reg->options; op->name; op++) {
-			if (plugin && strcmp(plugin, op->plugin) != 0)
-				continue;
-			if (strcmp(option, op->name) != 0)
-				continue;
-
-			return op;
-		}
-	}
-
-	return NULL;
-}
-
 static int process_option(const char *plugin, const char *option, const char *val)
 {
 	struct tep_plugin_option *op;
-- 
2.24.1

[PATCH 4/7] trace-cmd,libtraceevent: Check for plugin duplication

[Tzvetomir Stoyanov (VMware)]

When loading new plugin, check if plugin with the same
file name is already registered. If there is such plugin,
unload it and then load the new one.
In case of duplication, the last one wins.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/traceevent/event-plugin.c | 50 +++++++++++++++++++++++++++++++----
 1 file changed, 45 insertions(+), 5 deletions(-)

diff --git a/lib/traceevent/event-plugin.c b/lib/traceevent/event-plugin.c
index e53b09c..a33cf09 100644
--- a/lib/traceevent/event-plugin.c
+++ b/lib/traceevent/event-plugin.c
@@ -432,16 +432,60 @@ void tep_print_plugins(struct trace_seq *s,
 	}
 }
 
+
+static void unload_plugin(void *handle, struct tep_handle *tep)
+{
+	tep_plugin_unload_func func;
+
+	if (!handle)
+		return;
+	func = dlsym(handle, TEP_PLUGIN_UNLOADER_NAME);
+	if (func)
+		func(tep);
+	dlclose(handle);
+}
+
+static struct tep_plugin_list *
+detach_plugin(const char *name, struct tep_plugin_list **plist)
+{
+	struct tep_plugin_list *plugins = *plist;
+	struct tep_plugin_list *prev = NULL;
+	char *file;
+
+	while (plugins) {
+		file = strrchr(plugins->name, '/');
+		if (file)
+			file++;
+		else
+			file = plugins->name;
+		if (!strcmp(file, name)) {
+			if (prev)
+				prev->next = plugins->next;
+			else
+				*plist = plugins->next;
+			return plugins;
+		}
+		prev = plugins;
+		plugins = plugins->next;
+	}
+
+	return NULL;
+}
+
 static void
 load_plugin(struct tep_handle *tep, const char *path,
 	    const char *file, void *data)
 {
 	struct tep_plugin_list **plugin_list = data;
+	struct tep_plugin_list *old;
 	tep_plugin_load_func func;
 	struct tep_plugin_list *list;
 	char *plugin;
 	void *handle;
 	int ret;
+	old = detach_plugin(file, plugin_list);
+	if (old)
+		unload_plugin(old->handle, tep);
 
 	ret = asprintf(&plugin, "%s/%s", path, file);
 	if (ret < 0) {
@@ -652,16 +696,12 @@ void tep_free_plugin_paths(struct tep_handle *tep)
 void
 tep_unload_plugins(struct tep_plugin_list *plugin_list, struct tep_handle *tep)
 {
-	tep_plugin_unload_func func;
 	struct tep_plugin_list *list;
 
 	while (plugin_list) {
 		list = plugin_list;
+		unload_plugin(list->handle, tep);
 		plugin_list = list->next;
-		func = dlsym(list->handle, TEP_PLUGIN_UNLOADER_NAME);
-		if (func)
-			func(tep);
-		dlclose(list->handle);
 		free(list->name);
 		free(list);
 	}
-- 
2.24.1

[PATCH 5/7] trace-cmd,libtraceevent: Remove API for plugin options print

[Tzvetomir Stoyanov (VMware)]

The functionality for printing registered plugin
options is moved from libtraceevent to the application.
A more generic walk API is introduced.

Removed APIs:
	tep_plugin_list_options()
	tep_plugin_free_options_list()
	tep_plugin_print_options()

Added API
	tep_plugin_walk_options()

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/traceevent/event-parse.h |  9 ++-
 lib/traceevent/event-plugin.c    | 98 ++++----------------------------
 tracecmd/trace-list.c            | 43 +++++++++++++-
 3 files changed, 58 insertions(+), 92 deletions(-)

diff --git a/include/traceevent/event-parse.h b/include/traceevent/event-parse.h
index cb1b46e..3b4f3a5 100644
--- a/include/traceevent/event-parse.h
+++ b/include/traceevent/event-parse.h
@@ -346,8 +346,6 @@ enum tep_errno {
 
 struct tep_plugin_list;
 
-#define INVALID_PLUGIN_LIST_OPTION	((char **)((unsigned long)-1))
-
 enum tep_plugin_load_priority {
 	TEP_PLUGIN_FIRST,
 	TEP_PLUGIN_LAST,
@@ -364,13 +362,14 @@ void tep_load_plugins_hook(struct tep_handle *tep, const char *suffix,
 					       const char *name,
 					       void *data),
 			   void *data);
-char **tep_plugin_list_options(void);
-void tep_plugin_free_options_list(char **list);
 int tep_plugin_add_options(const char *name,
 			   struct tep_plugin_option *options);
 int tep_plugin_add_option(const char *name, const char *val);
 void tep_plugin_remove_options(struct tep_plugin_option *options);
-void tep_plugin_print_options(struct trace_seq *s);
+void tep_plugin_walk_options(int (*callback)(struct tep_plugin_option *op,
+					     void *context),
+			     void *context);
+
 void tep_print_plugins(struct trace_seq *s,
 			const char *prefix, const char *suffix,
 			const struct tep_plugin_list *list);
diff --git a/lib/traceevent/event-plugin.c b/lib/traceevent/event-plugin.c
index a33cf09..191b27b 100644
--- a/lib/traceevent/event-plugin.c
+++ b/lib/traceevent/event-plugin.c
@@ -91,71 +91,6 @@ static int update_option_value(struct tep_plugin_option *op, const char *val)
 	return 0;
 }
 
-/**
- * tep_plugin_list_options - get list of plugin options
- *
- * Returns an array of char strings that list the currently registered
- * plugin options in the format of <plugin>:<option>. This list can be
- * used by toggling the option.
- *
- * Returns NULL if there's no options registered. On error it returns
- * INVALID_PLUGIN_LIST_OPTION
- *
- * Must be freed with tep_plugin_free_options_list().
- */
-char **tep_plugin_list_options(void)
-{
-	struct registered_plugin_options *reg;
-	struct tep_plugin_option *op;
-	char **list = NULL;
-	char *name;
-	int count = 0;
-
-	for (reg = registered_options; reg; reg = reg->next) {
-		for (op = reg->options; op->name; op++) {
-			char **temp = list;
-			int ret;
-
-			ret = asprintf(&name, "%s:%s", op->plugin, op->name);
-			if (ret < 0)
-				goto err;
-
-			list = realloc(list, count + 2);
-			if (!list) {
-				list = temp;
-				free(name);
-				goto err;
-			}
-			list[count++] = name;
-			list[count] = NULL;
-		}
-	}
-	return list;
-
- err:
-	while (--count >= 0)
-		free(list[count]);
-	free(list);
-
-	return INVALID_PLUGIN_LIST_OPTION;
-}
-
-void tep_plugin_free_options_list(char **list)
-{
-	int i;
-
-	if (!list)
-		return;
-
-	if (list == INVALID_PLUGIN_LIST_OPTION)
-		return;
-
-	for (i = 0; list[i]; i++)
-		free(list[i]);
-
-	free(list);
-}
-
 static int
 update_option(const char *file, struct tep_plugin_option *option)
 {
@@ -377,40 +312,31 @@ int tep_plugin_add_option(const char *name, const char *val)
 	return -ENOMEM;
 }
 
-static void print_op_data(struct trace_seq *s, const char *name,
-			  const char *op)
-{
-	if (op)
-		trace_seq_printf(s, "%8s:\t%s\n", name, op);
-}
-
 /**
- * tep_plugin_print_options - print out the registered plugin options
- * @s: The trace_seq descriptor to write the plugin options into
+ * tep_plugin_walk_options - walk through all registered plugin options
+ * @callback: a user function, called on each registered plugin option
+ * @context: user data, passed to @callback function
  *
- * Writes a list of options into trace_seq @s.
+ * If the @callback returns non zero, the iteration stops.
  */
-void tep_plugin_print_options(struct trace_seq *s)
+void tep_plugin_walk_options(int (*callback)(struct tep_plugin_option *op,
+					     void *context),
+			     void *context)
 {
 	struct registered_plugin_options *reg;
 	struct tep_plugin_option *op;
 
+	if (!callback)
+		return;
+
 	for (reg = registered_options; reg; reg = reg->next) {
-		if (reg != registered_options)
-			trace_seq_printf(s, "============\n");
 		for (op = reg->options; op->name; op++) {
-			if (op != reg->options)
-				trace_seq_printf(s, "------------\n");
-			print_op_data(s, "plugin", op->plugin);
-			print_op_data(s, "option", op->name);
-			print_op_data(s, "desc", op->description);
-			print_op_data(s, "value", op->value);
-			trace_seq_printf(s, "%8s:\t%d\n", "set", op->set);
+			if (callback(op, context))
+				break;
 		}
 	}
 }
 
-
 /**
  * tep_print_plugins - print out the list of plugins loaded
  * @s: the trace_seq descripter to write to
diff --git a/tracecmd/trace-list.c b/tracecmd/trace-list.c
index 6b59117..496a83b 100644
--- a/tracecmd/trace-list.c
+++ b/tracecmd/trace-list.c
@@ -304,6 +304,47 @@ static void show_buffers(void)
 		printf("No buffer instances defined\n");
 }
 
+struct plugin_options_context {
+	struct trace_seq *s;
+	char *plugin;
+};
+
+static void print_op_data(struct trace_seq *s, const char *name,
+			  const char *op)
+{
+	if (op)
+		trace_seq_printf(s, "%8s:\t%s\n", name, op);
+}
+
+static int print_options_walk(struct tep_plugin_option *op, void *context)
+{
+	struct plugin_options_context *data = (struct plugin_options_context *)context;
+
+	if (!data->plugin || strcmp(data->plugin, op->plugin)) {
+		trace_seq_printf(data->s, "============\n");
+		free(data->plugin);
+		data->plugin = strdup(op->plugin);
+	}
+	print_op_data(data->s, "plugin", op->plugin);
+	print_op_data(data->s, "option", op->name);
+	print_op_data(data->s, "desc", op->description);
+	print_op_data(data->s, "value", op->value);
+	trace_seq_printf(data->s, "%8s:\t%d\n", "set", op->set);
+	trace_seq_printf(data->s, "------------\n");
+
+	return 0;
+}
+
+static void plugin_options_print(struct trace_seq *s)
+{
+	struct plugin_options_context context;
+
+	memset(&context, 0, sizeof(context));
+	context.s = s;
+	tep_plugin_walk_options(print_options_walk, &context);
+	free(context.plugin);
+}
+
 
 static void show_plugin_options(void)
 {
@@ -320,7 +361,7 @@ static void show_plugin_options(void)
 	trace_seq_init(&s);
 
 	list = trace_load_plugins(pevent);
-	tep_plugin_print_options(&s);
+	plugin_options_print(&s);
 	trace_seq_do_printf(&s);
 	tep_unload_plugins(list, pevent);
 	tep_free(pevent);
-- 
2.24.1

[PATCH 6/7] trace-cmd,libtraceevent: Remove API for plugin print

[Tzvetomir Stoyanov (VMware)]

The functionality for printing registered plugins
is moved from libtraceevent to the application.
A more generic walk API is introduced.

Removed
	tep_print_plugins()

Added
	tep_walk_plugins()

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/traceevent/event-parse.h |  8 ++++----
 lib/traceevent/event-plugin.c    | 23 +++++++++++------------
 tracecmd/trace-list.c            | 12 +++++++++---
 3 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/include/traceevent/event-parse.h b/include/traceevent/event-parse.h
index 3b4f3a5..a64482d 100644
--- a/include/traceevent/event-parse.h
+++ b/include/traceevent/event-parse.h
@@ -362,6 +362,10 @@ void tep_load_plugins_hook(struct tep_handle *tep, const char *suffix,
 					       const char *name,
 					       void *data),
 			   void *data);
+void tep_walk_plugins(const struct tep_plugin_list *list,
+		      int (*callback)(char *plugin_file, void *context),
+		      void *context);
+
 int tep_plugin_add_options(const char *name,
 			   struct tep_plugin_option *options);
 int tep_plugin_add_option(const char *name, const char *val);
@@ -370,10 +374,6 @@ void tep_plugin_walk_options(int (*callback)(struct tep_plugin_option *op,
 					     void *context),
 			     void *context);
 
-void tep_print_plugins(struct trace_seq *s,
-			const char *prefix, const char *suffix,
-			const struct tep_plugin_list *list);
-
 /* tep_handle */
 typedef char *(tep_func_resolver_t)(void *priv,
 				    unsigned long long *addrp, char **modp);
diff --git a/lib/traceevent/event-plugin.c b/lib/traceevent/event-plugin.c
index 191b27b..1ea02e6 100644
--- a/lib/traceevent/event-plugin.c
+++ b/lib/traceevent/event-plugin.c
@@ -338,22 +338,21 @@ void tep_plugin_walk_options(int (*callback)(struct tep_plugin_option *op,
 }
 
 /**
- * tep_print_plugins - print out the list of plugins loaded
- * @s: the trace_seq descripter to write to
- * @prefix: The prefix string to add before listing the option name
- * @suffix: The suffix string ot append after the option name
- * @list: The list of plugins (usually returned by tep_load_plugins()
+ * tep_walk_plugins - walk through all plugins from the list
+ * @list: plugin list, returned by tep_load_plugins()
+ * @callback: a user function, called on each plugin from the list
+ * @context: user data, passed to @callback function
  *
- * Writes to the trace_seq @s the list of plugins (files) that is
- * returned by tep_load_plugins(). Use @prefix and @suffix for formating:
- * @prefix = "  ", @suffix = "\n".
+ * If the @callback returns non zero, the iteration stops.
  */
-void tep_print_plugins(struct trace_seq *s,
-		       const char *prefix, const char *suffix,
-		       const struct tep_plugin_list *list)
+void tep_walk_plugins(const struct tep_plugin_list *list,
+		      int (*callback)(char *plugin_file, void *context),
+		      void *context)
 {
+	if (!list || !callback)
+		return;
 	while (list) {
-		trace_seq_printf(s, "%s%s%s", prefix, list->name, suffix);
+		callback(list->name, context);
 		list = list->next;
 	}
 }
diff --git a/tracecmd/trace-list.c b/tracecmd/trace-list.c
index 496a83b..f7bef45 100644
--- a/tracecmd/trace-list.c
+++ b/tracecmd/trace-list.c
@@ -373,6 +373,14 @@ void trace_option(int argc, char **argv)
 	show_plugin_options();
 }
 
+static int plugins_walk(char *plugin_file, void *context)
+{
+	struct trace_seq *s = (struct trace_seq *)context;
+
+	if (plugin_file)
+		trace_seq_printf(s, "  %s\n", plugin_file);
+	return 0;
+}
 
 static void show_plugins(void)
 {
@@ -387,14 +395,12 @@ static void show_plugins(void)
 	trace_seq_init(&s);
 
 	list = trace_load_plugins(pevent);
-	tep_print_plugins(&s, "  ", "\n", list);
-
+	tep_walk_plugins(list, plugins_walk, &s);
 	trace_seq_do_printf(&s);
 	tep_unload_plugins(list, pevent);
 	tep_free(pevent);
 }
 
-
 void trace_list(int argc, char **argv)
 {
 	int events = 0;
-- 
2.24.1

[PATCH 7/7] trace-cmd, kernelshark, libtraceevent: Changed tep_plugin_add_option() API

[Tzvetomir Stoyanov (VMware)]

The API
 tep_plugin_add_option()
is renamed to
 tep_plugin_set_option()

The new name describes more closely the purpose of the API -
it lets us update the option's value. The logic of the API is
slightly changed: If no plugin is given, all options that match
the option name will be updated.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/traceevent/event-parse.h |  2 +-
 kernel-shark/src/libkshark.c     |  4 ++--
 lib/traceevent/event-plugin.c    | 30 +++++++++++++++++++++++-------
 tracecmd/trace-read.c            |  2 +-
 4 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/include/traceevent/event-parse.h b/include/traceevent/event-parse.h
index a64482d..538dd94 100644
--- a/include/traceevent/event-parse.h
+++ b/include/traceevent/event-parse.h
@@ -368,7 +368,7 @@ void tep_walk_plugins(const struct tep_plugin_list *list,
 
 int tep_plugin_add_options(const char *name,
 			   struct tep_plugin_option *options);
-int tep_plugin_add_option(const char *name, const char *val);
+int tep_plugin_set_option(const char *name, const char *val);
 void tep_plugin_remove_options(struct tep_plugin_option *options);
 void tep_plugin_walk_options(int (*callback)(struct tep_plugin_option *op,
 					     void *context),
diff --git a/kernel-shark/src/libkshark.c b/kernel-shark/src/libkshark.c
index a361578..34fa007 100644
--- a/kernel-shark/src/libkshark.c
+++ b/kernel-shark/src/libkshark.c
@@ -160,8 +160,8 @@ bool kshark_open(struct kshark_context *kshark_ctx, const char *file)
 	 * Turn off function trace indent and turn on show parent
 	 * if possible.
 	 */
-	tep_plugin_add_option("ftrace:parent", "1");
-	tep_plugin_add_option("ftrace:indent", "0");
+	tep_plugin_set_option("ftrace:parent", "1");
+	tep_plugin_set_option("ftrace:indent", "0");
 
 	return true;
 }
diff --git a/lib/traceevent/event-plugin.c b/lib/traceevent/event-plugin.c
index 1ea02e6..4af85de 100644
--- a/lib/traceevent/event-plugin.c
+++ b/lib/traceevent/event-plugin.c
@@ -219,7 +219,10 @@ static void parse_option_name(char **option, char **plugin)
 	*plugin = NULL;
 
 	if ((p = strstr(*option, ":"))) {
-		*plugin = *option;
+		if (**option == ':')
+			*plugin = NULL;
+		else
+			*plugin = *option;
 		*p = '\0';
 		*option = strdup(p + 1);
 		if (!*option)
@@ -229,25 +232,38 @@ static void parse_option_name(char **option, char **plugin)
 
 static int process_option(const char *plugin, const char *option, const char *val)
 {
+	struct registered_plugin_options *reg;
 	struct tep_plugin_option *op;
+	int ret;
 
-	op = find_registered_option(plugin, option);
-	if (!op)
-		return 0;
+	for (reg = registered_options; reg; reg = reg->next) {
+		for (op = reg->options; op->name; op++) {
+			if (plugin && strcmp(plugin, op->plugin) != 0)
+				continue;
+			if (strcmp(option, op->name) != 0)
+				continue;
 
-	return update_option_value(op, val);
+			ret = update_option_value(op, val);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
 }
 
 /**
- * tep_plugin_add_option - add an option/val pair to set plugin options
+ * tep_plugin_set_option - update the value of plugin option
  * @name: The name of the option (format: <plugin>:<option> or just <option>)
  * @val: (optiona) the value for the option
  *
  * Modify a plugin option. If @val is given than the value of the option
  * is set (note, some options just take a boolean, so @val must be either
  * "1" or "0" or "true" or "false").
+ * If there is no <plugin> in the @name, all options with name <option> will
+ * be updated.
  */
-int tep_plugin_add_option(const char *name, const char *val)
+int tep_plugin_set_option(const char *name, const char *val)
 {
 	struct trace_plugin_options *op;
 	char *option_str;
diff --git a/tracecmd/trace-read.c b/tracecmd/trace-read.c
index c0d640d..7b41d72 100644
--- a/tracecmd/trace-read.c
+++ b/tracecmd/trace-read.c
@@ -1381,7 +1381,7 @@ static void process_plugin_option(char *option)
 		*p = '\0';
 		val = p+1;
 	}
-	tep_plugin_add_option(name, val);
+	tep_plugin_set_option(name, val);
 }
 
 static void set_event_flags(struct tep_handle *pevent, struct event_str *list,
-- 
2.24.1