linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] Few libtraceeevnt fixes, suggested Namhyung Kim
@ 2020-07-14 10:30 Tzvetomir Stoyanov (VMware)
  2020-07-14 10:30 ` [PATCH 1/8] trace-cmd: Document tep_load_plugins_hook() Tzvetomir Stoyanov (VMware)
                   ` (8 more replies)
  0 siblings, 9 replies; 16+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-07-14 10:30 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, namhyung

Fixes in libtraceeevnt, suggested by Namhyung Kim <namhyung@kernel.org>

Tzvetomir Stoyanov (VMware) (8):
  trace-cmd: Document tep_load_plugins_hook()
  trace-cmd: Handle strdup() error in parse_option_name()
  trace-cmd: Fix typo in tep_plugin_add_option() description
  trace-cmd: Improve error handling of tep_plugin_add_option() API
  trace-cmd: Fixed broken indentation in parse_ip4_print_args()
  trace-cmd: Fixed type in PRINT_FMT_STING
  trace-cmd: Fixed description of tep_add_plugin_path() API
  trace-cmd: Handle possible strdup() error in tep_add_plugin_path() API

 lib/traceevent/event-parse-local.h |  2 +-
 lib/traceevent/event-parse.c       | 10 +++---
 lib/traceevent/event-plugin.c      | 50 ++++++++++++++++++++++--------
 3 files changed, 43 insertions(+), 19 deletions(-)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/8] trace-cmd: Document tep_load_plugins_hook()
  2020-07-14 10:30 [PATCH 0/8] Few libtraceeevnt fixes, suggested Namhyung Kim Tzvetomir Stoyanov (VMware)
@ 2020-07-14 10:30 ` Tzvetomir Stoyanov (VMware)
  2020-07-14 14:28   ` Steven Rostedt
  2020-07-14 10:30 ` [PATCH 2/8] trace-cmd: Handle strdup() error in parse_option_name() Tzvetomir Stoyanov (VMware)
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-07-14 10:30 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, namhyung

Add description of tep_load_plugins_hook() traceevent API.

Suggested-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/traceevent/event-plugin.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/lib/traceevent/event-plugin.c b/lib/traceevent/event-plugin.c
index 30c1526d..c11636ce 100644
--- a/lib/traceevent/event-plugin.c
+++ b/lib/traceevent/event-plugin.c
@@ -544,6 +544,22 @@ load_plugins_dir(struct tep_handle *tep, const char *suffix,
 	closedir(dir);
 }
 
+/**
+ * tep_load_plugins_hook - call a user specified callback to load a plugin
+ * @tep: handler to traceevent context
+ * @suffix: filter only plugin files with given suffix
+ * @load_plugin: user specified callback, called for each plugin file
+ * @data: custom context, passed to @load_plugin
+ *
+ * Searches for traceevent plugin files and calls @load_plugin for each
+ * The order of plugins search is:
+ *  - Directories, specified in @tep->plugins_dir and priority TEP_PLUGIN_FIRST
+ *  - Directory, specified at compile time with PLUGIN_TRACEEVENT_DIR
+ *  - Directory, specified by environment variable TRACEEVENT_PLUGIN_DIR
+ *  - In user's home: ~/.local/lib/traceevent/plugins/
+ *  - Directories, specified in @tep->plugins_dir and priority TEP_PLUGIN_LAST
+ *
+ */
 void tep_load_plugins_hook(struct tep_handle *tep, const char *suffix,
 			   void (*load_plugin)(struct tep_handle *tep,
 					       const char *path,
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/8] trace-cmd: Handle strdup() error in parse_option_name()
  2020-07-14 10:30 [PATCH 0/8] Few libtraceeevnt fixes, suggested Namhyung Kim Tzvetomir Stoyanov (VMware)
  2020-07-14 10:30 ` [PATCH 1/8] trace-cmd: Document tep_load_plugins_hook() Tzvetomir Stoyanov (VMware)
@ 2020-07-14 10:30 ` Tzvetomir Stoyanov (VMware)
  2020-07-14 10:30 ` [PATCH 3/8] trace-cmd: Fix typo in tep_plugin_add_option() description Tzvetomir Stoyanov (VMware)
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-07-14 10:30 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, namhyung

Modified internal function parse_option_name() to return error and handle
that error in function callers in case strdup() fails.

Suggested-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/traceevent/event-plugin.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/traceevent/event-plugin.c b/lib/traceevent/event-plugin.c
index c11636ce..b97d0c7e 100644
--- a/lib/traceevent/event-plugin.c
+++ b/lib/traceevent/event-plugin.c
@@ -254,7 +254,7 @@ void tep_plugin_remove_options(struct tep_plugin_option *options)
 	}
 }
 
-static void parse_option_name(char **option, char **plugin)
+static int parse_option_name(char **option, char **plugin)
 {
 	char *p;
 
@@ -265,8 +265,9 @@ static void parse_option_name(char **option, char **plugin)
 		*p = '\0';
 		*option = strdup(p + 1);
 		if (!*option)
-			return;
+			return -1;
 	}
+	return 0;
 }
 
 static struct tep_plugin_option *
@@ -325,7 +326,8 @@ int tep_plugin_add_option(const char *name, const char *val)
 	if (!option_str)
 		return -ENOMEM;
 
-	parse_option_name(&option_str, &plugin);
+	if (parse_option_name(&option_str, &plugin) < 0)
+		return -ENOMEM;
 
 	/* If the option exists, update the val */
 	for (op = trace_plugin_options; op; op = op->next) {
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/8] trace-cmd: Fix typo in tep_plugin_add_option() description
  2020-07-14 10:30 [PATCH 0/8] Few libtraceeevnt fixes, suggested Namhyung Kim Tzvetomir Stoyanov (VMware)
  2020-07-14 10:30 ` [PATCH 1/8] trace-cmd: Document tep_load_plugins_hook() Tzvetomir Stoyanov (VMware)
  2020-07-14 10:30 ` [PATCH 2/8] trace-cmd: Handle strdup() error in parse_option_name() Tzvetomir Stoyanov (VMware)
@ 2020-07-14 10:30 ` Tzvetomir Stoyanov (VMware)
  2020-07-14 10:30 ` [PATCH 4/8] trace-cmd: Improve error handling of tep_plugin_add_option() API Tzvetomir Stoyanov (VMware)
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-07-14 10:30 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, namhyung

A type "optiona" -> "optional" is fixed in description of
tep_plugin_add_option() API.

Suggested-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/traceevent/event-plugin.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/traceevent/event-plugin.c b/lib/traceevent/event-plugin.c
index b97d0c7e..03f26c8c 100644
--- a/lib/traceevent/event-plugin.c
+++ b/lib/traceevent/event-plugin.c
@@ -310,7 +310,7 @@ static int process_option(const char *plugin, const char *option, const char *va
 /**
  * tep_plugin_add_option - add an option/val pair to set plugin options
  * @name: The name of the option (format: <plugin>:<option> or just <option>)
- * @val: (optiona) the value for the option
+ * @val: (optional) 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
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/8] trace-cmd: Improve error handling of tep_plugin_add_option() API
  2020-07-14 10:30 [PATCH 0/8] Few libtraceeevnt fixes, suggested Namhyung Kim Tzvetomir Stoyanov (VMware)
                   ` (2 preceding siblings ...)
  2020-07-14 10:30 ` [PATCH 3/8] trace-cmd: Fix typo in tep_plugin_add_option() description Tzvetomir Stoyanov (VMware)
@ 2020-07-14 10:30 ` Tzvetomir Stoyanov (VMware)
  2020-07-14 10:30 ` [PATCH 5/8] trace-cmd: Fixed broken indentation in parse_ip4_print_args() Tzvetomir Stoyanov (VMware)
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-07-14 10:30 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, namhyung

In case of memory error, ensure all allocated resources are freed.
Do not append broken option in trace_plugin_options list.

Suggested-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/traceevent/event-plugin.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/lib/traceevent/event-plugin.c b/lib/traceevent/event-plugin.c
index 03f26c8c..aa868133 100644
--- a/lib/traceevent/event-plugin.c
+++ b/lib/traceevent/event-plugin.c
@@ -361,23 +361,25 @@ int tep_plugin_add_option(const char *name, const char *val)
 	if (!op) {
 		op = malloc(sizeof(*op));
 		if (!op)
-			return -ENOMEM;
+			goto out_free;
 		memset(op, 0, sizeof(*op));
-		op->next = trace_plugin_options;
-		trace_plugin_options = op;
-
 		op->plugin = plugin;
 		op->option = option_str;
-
 		if (val) {
 			op->value = strdup(val);
-			if (!op->value)
+			if (!op->value) {
+				free(op);
 				goto out_free;
+			}
 		}
+		op->next = trace_plugin_options;
+		trace_plugin_options = op;
 	}
 
 	return process_option(plugin, option_str, val);
- out_free:
+
+out_free:
+	free(plugin);
 	free(option_str);
 	return -ENOMEM;
 }
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/8] trace-cmd: Fixed broken indentation in parse_ip4_print_args()
  2020-07-14 10:30 [PATCH 0/8] Few libtraceeevnt fixes, suggested Namhyung Kim Tzvetomir Stoyanov (VMware)
                   ` (3 preceding siblings ...)
  2020-07-14 10:30 ` [PATCH 4/8] trace-cmd: Improve error handling of tep_plugin_add_option() API Tzvetomir Stoyanov (VMware)
@ 2020-07-14 10:30 ` Tzvetomir Stoyanov (VMware)
  2020-07-14 10:30 ` [PATCH 6/8] trace-cmd: Fixed type in PRINT_FMT_STING Tzvetomir Stoyanov (VMware)
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-07-14 10:30 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, namhyung

Fixed the "break" indentation in a switch() inside
parse_ip4_print_args() static function.

Suggested-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/traceevent/event-parse.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/traceevent/event-parse.c b/lib/traceevent/event-parse.c
index 77c32492..4de9729c 100644
--- a/lib/traceevent/event-parse.c
+++ b/lib/traceevent/event-parse.c
@@ -4680,11 +4680,11 @@ static int parse_ip4_print_args(struct tep_handle *tep,
 		else
 			*reverse = true;
 		ret++;
-	break;
+		break;
 	case 'l':
 		*reverse = true;
 		ret++;
-	break;
+		break;
 	case 'n':
 	case 'b':
 		ret++;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 6/8] trace-cmd: Fixed type in PRINT_FMT_STING
  2020-07-14 10:30 [PATCH 0/8] Few libtraceeevnt fixes, suggested Namhyung Kim Tzvetomir Stoyanov (VMware)
                   ` (4 preceding siblings ...)
  2020-07-14 10:30 ` [PATCH 5/8] trace-cmd: Fixed broken indentation in parse_ip4_print_args() Tzvetomir Stoyanov (VMware)
@ 2020-07-14 10:30 ` Tzvetomir Stoyanov (VMware)
  2020-07-14 10:30 ` [PATCH 7/8] trace-cmd: Fixed description of tep_add_plugin_path() API Tzvetomir Stoyanov (VMware)
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-07-14 10:30 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, namhyung

PRINT_FMT_STING -> PRINT_FMT_STRING

Suggested-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/traceevent/event-parse-local.h | 2 +-
 lib/traceevent/event-parse.c       | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/traceevent/event-parse-local.h b/lib/traceevent/event-parse-local.h
index e71296a6..d805a920 100644
--- a/lib/traceevent/event-parse-local.h
+++ b/lib/traceevent/event-parse-local.h
@@ -86,7 +86,7 @@ struct tep_handle {
 };
 
 enum tep_print_parse_type {
-	PRINT_FMT_STING,
+	PRINT_FMT_STRING,
 	PRINT_FMT_ARG_DIGIT,
 	PRINT_FMT_ARG_POINTER,
 	PRINT_FMT_ARG_STRING,
diff --git a/lib/traceevent/event-parse.c b/lib/traceevent/event-parse.c
index 4de9729c..186fee33 100644
--- a/lib/traceevent/event-parse.c
+++ b/lib/traceevent/event-parse.c
@@ -5659,7 +5659,7 @@ static int parse_arg_format(struct tep_print_parse **parse,
 		default:
 			snprintf(print_format, 32, ">%c<", *format);
 			parse_arg_add(parse, print_format,
-					PRINT_FMT_STING, NULL, NULL, 0);
+					PRINT_FMT_STRING, NULL, NULL, 0);
 			ret++;
 			return ret;
 		}
@@ -5711,7 +5711,7 @@ static int parse_arg_string(struct tep_print_parse **parse, const char *format)
 		ret++;
 	}
 	trace_seq_terminate(&s);
-	parse_arg_add(parse, s.buffer, PRINT_FMT_STING, NULL, NULL, 0);
+	parse_arg_add(parse, s.buffer, PRINT_FMT_STRING, NULL, NULL, 0);
 	trace_seq_destroy(&s);
 
 	return ret;
@@ -5769,7 +5769,7 @@ static void print_event_cache(struct tep_print_parse *parse, struct trace_seq *s
 					 parse->len_as_arg ? len_arg : -1,
 					 data, size, event, parse->arg);
 			break;
-		case PRINT_FMT_STING:
+		case PRINT_FMT_STRING:
 		default:
 			trace_seq_printf(s, "%s", parse->format);
 			break;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 7/8] trace-cmd: Fixed description of tep_add_plugin_path() API
  2020-07-14 10:30 [PATCH 0/8] Few libtraceeevnt fixes, suggested Namhyung Kim Tzvetomir Stoyanov (VMware)
                   ` (5 preceding siblings ...)
  2020-07-14 10:30 ` [PATCH 6/8] trace-cmd: Fixed type in PRINT_FMT_STING Tzvetomir Stoyanov (VMware)
@ 2020-07-14 10:30 ` Tzvetomir Stoyanov (VMware)
  2020-07-14 10:30 ` [PATCH 8/8] trace-cmd: Handle possible strdup() error in " Tzvetomir Stoyanov (VMware)
  2020-07-14 12:07 ` [PATCH 0/8] Few libtraceeevnt fixes, suggested Namhyung Kim Namhyung Kim
  8 siblings, 0 replies; 16+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-07-14 10:30 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, namhyung

Changed the description of tep_add_plugin_path() API to reflect the
logic of the function. The suffix of plugin files is not hardcoded
to ".so", it depends on the custom plugin loader callback.

Suggested-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/traceevent/event-plugin.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/traceevent/event-plugin.c b/lib/traceevent/event-plugin.c
index aa868133..841e41c6 100644
--- a/lib/traceevent/event-plugin.c
+++ b/lib/traceevent/event-plugin.c
@@ -647,8 +647,8 @@ tep_load_plugins(struct tep_handle *tep)
 /**
  * tep_add_plugin_path - Add a new plugin directory.
  * @tep: Trace event handler.
- * @path: Path to a directory. All files with extension .so in that
- *	  directory will be loaded as plugins.
+ * @path: Path to a directory. All plugin files in that
+ *	  directory will be loaded.
  *@prio: Load priority of the plugins in that directory.
  *
  * Returns -1 in case of an error, 0 otherwise.
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 8/8] trace-cmd: Handle possible strdup() error in tep_add_plugin_path() API
  2020-07-14 10:30 [PATCH 0/8] Few libtraceeevnt fixes, suggested Namhyung Kim Tzvetomir Stoyanov (VMware)
                   ` (6 preceding siblings ...)
  2020-07-14 10:30 ` [PATCH 7/8] trace-cmd: Fixed description of tep_add_plugin_path() API Tzvetomir Stoyanov (VMware)
@ 2020-07-14 10:30 ` Tzvetomir Stoyanov (VMware)
  2020-07-14 12:07 ` [PATCH 0/8] Few libtraceeevnt fixes, suggested Namhyung Kim Namhyung Kim
  8 siblings, 0 replies; 16+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-07-14 10:30 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, namhyung

Free allocated resources and return -1 in case strdup() fails in
tep_add_plugin_path() API.

Suggested-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 lib/traceevent/event-plugin.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/traceevent/event-plugin.c b/lib/traceevent/event-plugin.c
index 841e41c6..4a3f7d3f 100644
--- a/lib/traceevent/event-plugin.c
+++ b/lib/traceevent/event-plugin.c
@@ -666,6 +666,10 @@ int tep_add_plugin_path(struct tep_handle *tep, char *path,
 		return -1;
 
 	dir->path = strdup(path);
+	if (!dir->path) {
+		free(dir);
+		return -1;
+	}
 	dir->prio = prio;
 	dir->next = tep->plugins_dir;
 	tep->plugins_dir = dir;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/8] Few libtraceeevnt fixes, suggested Namhyung Kim
  2020-07-14 10:30 [PATCH 0/8] Few libtraceeevnt fixes, suggested Namhyung Kim Tzvetomir Stoyanov (VMware)
                   ` (7 preceding siblings ...)
  2020-07-14 10:30 ` [PATCH 8/8] trace-cmd: Handle possible strdup() error in " Tzvetomir Stoyanov (VMware)
@ 2020-07-14 12:07 ` Namhyung Kim
  2020-07-14 12:12   ` Tzvetomir Stoyanov
  2020-07-14 14:22   ` Steven Rostedt
  8 siblings, 2 replies; 16+ messages in thread
From: Namhyung Kim @ 2020-07-14 12:07 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware); +Cc: Steven Rostedt, Linux Trace Devel

Hello,

On Tue, Jul 14, 2020 at 7:30 PM Tzvetomir Stoyanov (VMware)
<tz.stoyanov@gmail.com> wrote:
>
> Fixes in libtraceeevnt, suggested by Namhyung Kim <namhyung@kernel.org>
>
> Tzvetomir Stoyanov (VMware) (8):
>   trace-cmd: Document tep_load_plugins_hook()
>   trace-cmd: Handle strdup() error in parse_option_name()
>   trace-cmd: Fix typo in tep_plugin_add_option() description
>   trace-cmd: Improve error handling of tep_plugin_add_option() API
>   trace-cmd: Fixed broken indentation in parse_ip4_print_args()
>   trace-cmd: Fixed type in PRINT_FMT_STING
>   trace-cmd: Fixed description of tep_add_plugin_path() API
>   trace-cmd: Handle possible strdup() error in tep_add_plugin_path() API

Shouldn't the prefix be libtraceevent ?  Other than that,

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks
Namhyung

>
>  lib/traceevent/event-parse-local.h |  2 +-
>  lib/traceevent/event-parse.c       | 10 +++---
>  lib/traceevent/event-plugin.c      | 50 ++++++++++++++++++++++--------
>  3 files changed, 43 insertions(+), 19 deletions(-)
>
> --
> 2.26.2
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/8] Few libtraceeevnt fixes, suggested Namhyung Kim
  2020-07-14 12:07 ` [PATCH 0/8] Few libtraceeevnt fixes, suggested Namhyung Kim Namhyung Kim
@ 2020-07-14 12:12   ` Tzvetomir Stoyanov
  2020-07-14 14:25     ` Steven Rostedt
  2020-07-14 14:22   ` Steven Rostedt
  1 sibling, 1 reply; 16+ messages in thread
From: Tzvetomir Stoyanov @ 2020-07-14 12:12 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: Steven Rostedt, Linux Trace Devel

On Tue, Jul 14, 2020 at 3:07 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hello,
>
> On Tue, Jul 14, 2020 at 7:30 PM Tzvetomir Stoyanov (VMware)
> <tz.stoyanov@gmail.com> wrote:
> >
> > Fixes in libtraceeevnt, suggested by Namhyung Kim <namhyung@kernel.org>
> >
> > Tzvetomir Stoyanov (VMware) (8):
> >   trace-cmd: Document tep_load_plugins_hook()
> >   trace-cmd: Handle strdup() error in parse_option_name()
> >   trace-cmd: Fix typo in tep_plugin_add_option() description
> >   trace-cmd: Improve error handling of tep_plugin_add_option() API
> >   trace-cmd: Fixed broken indentation in parse_ip4_print_args()
> >   trace-cmd: Fixed type in PRINT_FMT_STING
> >   trace-cmd: Fixed description of tep_add_plugin_path() API
> >   trace-cmd: Handle possible strdup() error in tep_add_plugin_path() API
>
> Shouldn't the prefix be libtraceevent ?  Other than that,

I decided to address the comments in trace-cmd repo first, and after
that to backport the patches to the kernel's libtraceevent. That's why
the prefix is trace-cmd.
Thanks for making such a detailed review, Namhyung !

>
> Acked-by: Namhyung Kim <namhyung@kernel.org>
>
> Thanks
> Namhyung
>
> >
> >  lib/traceevent/event-parse-local.h |  2 +-
> >  lib/traceevent/event-parse.c       | 10 +++---
> >  lib/traceevent/event-plugin.c      | 50 ++++++++++++++++++++++--------
> >  3 files changed, 43 insertions(+), 19 deletions(-)
> >
> > --
> > 2.26.2
> >



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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/8] Few libtraceeevnt fixes, suggested Namhyung Kim
  2020-07-14 12:07 ` [PATCH 0/8] Few libtraceeevnt fixes, suggested Namhyung Kim Namhyung Kim
  2020-07-14 12:12   ` Tzvetomir Stoyanov
@ 2020-07-14 14:22   ` Steven Rostedt
  1 sibling, 0 replies; 16+ messages in thread
From: Steven Rostedt @ 2020-07-14 14:22 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: Tzvetomir Stoyanov (VMware), Linux Trace Devel

On Tue, 14 Jul 2020 21:07:12 +0900
Namhyung Kim <namhyung@kernel.org> wrote:

> Hello,
> 
> On Tue, Jul 14, 2020 at 7:30 PM Tzvetomir Stoyanov (VMware)
> <tz.stoyanov@gmail.com> wrote:
> >
> > Fixes in libtraceeevnt, suggested by Namhyung Kim <namhyung@kernel.org>
> >
> > Tzvetomir Stoyanov (VMware) (8):
> >   trace-cmd: Document tep_load_plugins_hook()
> >   trace-cmd: Handle strdup() error in parse_option_name()
> >   trace-cmd: Fix typo in tep_plugin_add_option() description
> >   trace-cmd: Improve error handling of tep_plugin_add_option() API
> >   trace-cmd: Fixed broken indentation in parse_ip4_print_args()
> >   trace-cmd: Fixed type in PRINT_FMT_STING
> >   trace-cmd: Fixed description of tep_add_plugin_path() API
> >   trace-cmd: Handle possible strdup() error in tep_add_plugin_path() API  
> 
> Shouldn't the prefix be libtraceevent ?  Other than that,

Yes they should, because they are not trace-cmd specific.

> 
> Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks Namhyung!

-- Steve

> 
> Thanks
> Namhyung
> 
> >
> >  lib/traceevent/event-parse-local.h |  2 +-
> >  lib/traceevent/event-parse.c       | 10 +++---
> >  lib/traceevent/event-plugin.c      | 50 ++++++++++++++++++++++--------
> >  3 files changed, 43 insertions(+), 19 deletions(-)
> >
> > --
> > 2.26.2
> >  


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/8] Few libtraceeevnt fixes, suggested Namhyung Kim
  2020-07-14 12:12   ` Tzvetomir Stoyanov
@ 2020-07-14 14:25     ` Steven Rostedt
  0 siblings, 0 replies; 16+ messages in thread
From: Steven Rostedt @ 2020-07-14 14:25 UTC (permalink / raw)
  To: Tzvetomir Stoyanov; +Cc: Namhyung Kim, Linux Trace Devel

On Tue, 14 Jul 2020 15:12:10 +0300
Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:

> On Tue, Jul 14, 2020 at 3:07 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > Hello,
> >
> > On Tue, Jul 14, 2020 at 7:30 PM Tzvetomir Stoyanov (VMware)
> > <tz.stoyanov@gmail.com> wrote:  
> > >
> > > Fixes in libtraceeevnt, suggested by Namhyung Kim <namhyung@kernel.org>
> > >
> > > Tzvetomir Stoyanov (VMware) (8):
> > >   trace-cmd: Document tep_load_plugins_hook()
> > >   trace-cmd: Handle strdup() error in parse_option_name()
> > >   trace-cmd: Fix typo in tep_plugin_add_option() description
> > >   trace-cmd: Improve error handling of tep_plugin_add_option() API
> > >   trace-cmd: Fixed broken indentation in parse_ip4_print_args()
> > >   trace-cmd: Fixed type in PRINT_FMT_STING
> > >   trace-cmd: Fixed description of tep_add_plugin_path() API
> > >   trace-cmd: Handle possible strdup() error in tep_add_plugin_path() API  
> >
> > Shouldn't the prefix be libtraceevent ?  Other than that,  
> 
> I decided to address the comments in trace-cmd repo first, and after
> that to backport the patches to the kernel's libtraceevent. That's why
> the prefix is trace-cmd.

Actually, even in the trace-cmd repo, they should be labeled
libtraceveent, as they are specific for the libtraceevent portion of
trace-cmd.git. Otherwise everything in the repo would be trace-cmd: ;-)

I've tried to separate out different parts. Anything labeled trace-cmd:
should be for something unique to the trace-cmd executable. If
something is for libtracefs, it should be libtracefs: or tracefs:. If
it is for libtracevent, then that should be libtracevent: or traceevent:

-- Steve

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/8] trace-cmd: Document tep_load_plugins_hook()
  2020-07-14 10:30 ` [PATCH 1/8] trace-cmd: Document tep_load_plugins_hook() Tzvetomir Stoyanov (VMware)
@ 2020-07-14 14:28   ` Steven Rostedt
  2020-07-14 15:22     ` Tzvetomir Stoyanov
  0 siblings, 1 reply; 16+ messages in thread
From: Steven Rostedt @ 2020-07-14 14:28 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel, namhyung

On Tue, 14 Jul 2020 13:30:20 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> Add description of tep_load_plugins_hook() traceevent API.
> 

Should add:

Link: https://lore.kernel.org/r/CAM9d7cgLBWCrEHwz+Lhv5x5EXGcNWB0QQoeGh3OKh2JfR=dV9Q@mail.gmail.com

(you get that part after '/r/' from the message ID in the email.)

> Suggested-by: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  lib/traceevent/event-plugin.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/lib/traceevent/event-plugin.c b/lib/traceevent/event-plugin.c
> index 30c1526d..c11636ce 100644
> --- a/lib/traceevent/event-plugin.c
> +++ b/lib/traceevent/event-plugin.c
> @@ -544,6 +544,22 @@ load_plugins_dir(struct tep_handle *tep, const char *suffix,
>  	closedir(dir);
>  }
>  
> +/**
> + * tep_load_plugins_hook - call a user specified callback to load a plugin
> + * @tep: handler to traceevent context
> + * @suffix: filter only plugin files with given suffix
> + * @load_plugin: user specified callback, called for each plugin file
> + * @data: custom context, passed to @load_plugin
> + *
> + * Searches for traceevent plugin files and calls @load_plugin for each
> + * The order of plugins search is:
> + *  - Directories, specified in @tep->plugins_dir and priority TEP_PLUGIN_FIRST
> + *  - Directory, specified at compile time with PLUGIN_TRACEEVENT_DIR
> + *  - Directory, specified by environment variable TRACEEVENT_PLUGIN_DIR
> + *  - In user's home: ~/.local/lib/traceevent/plugins/
> + *  - Directories, specified in @tep->plugins_dir and priority TEP_PLUGIN_LAST

We should probably have a man page as well.

Thanks Tzvetomir!

-- Steve

> + *
> + */
>  void tep_load_plugins_hook(struct tep_handle *tep, const char *suffix,
>  			   void (*load_plugin)(struct tep_handle *tep,
>  					       const char *path,


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/8] trace-cmd: Document tep_load_plugins_hook()
  2020-07-14 14:28   ` Steven Rostedt
@ 2020-07-14 15:22     ` Tzvetomir Stoyanov
  2020-07-14 15:53       ` Steven Rostedt
  0 siblings, 1 reply; 16+ messages in thread
From: Tzvetomir Stoyanov @ 2020-07-14 15:22 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-trace-devel, namhyung

On Tue, Jul 14, 2020 at 5:28 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Tue, 14 Jul 2020 13:30:20 +0300
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
>
> > Add description of tep_load_plugins_hook() traceevent API.
> >
>
> Should add:
>
> Link: https://lore.kernel.org/r/CAM9d7cgLBWCrEHwz+Lhv5x5EXGcNWB0QQoeGh3OKh2JfR=dV9Q@mail.gmail.com
>
> (you get that part after '/r/' from the message ID in the email.)
>

Ok, I'll send the v2 with links and updated subjects to "trace-cmd
libtraceevent:"

> > Suggested-by: Namhyung Kim <namhyung@kernel.org>
> > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> > ---
> >  lib/traceevent/event-plugin.c | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/lib/traceevent/event-plugin.c b/lib/traceevent/event-plugin.c
> > index 30c1526d..c11636ce 100644
> > --- a/lib/traceevent/event-plugin.c
> > +++ b/lib/traceevent/event-plugin.c
> > @@ -544,6 +544,22 @@ load_plugins_dir(struct tep_handle *tep, const char *suffix,
> >       closedir(dir);
> >  }
> >
> > +/**
> > + * tep_load_plugins_hook - call a user specified callback to load a plugin
> > + * @tep: handler to traceevent context
> > + * @suffix: filter only plugin files with given suffix
> > + * @load_plugin: user specified callback, called for each plugin file
> > + * @data: custom context, passed to @load_plugin
> > + *
> > + * Searches for traceevent plugin files and calls @load_plugin for each
> > + * The order of plugins search is:
> > + *  - Directories, specified in @tep->plugins_dir and priority TEP_PLUGIN_FIRST
> > + *  - Directory, specified at compile time with PLUGIN_TRACEEVENT_DIR
> > + *  - Directory, specified by environment variable TRACEEVENT_PLUGIN_DIR
> > + *  - In user's home: ~/.local/lib/traceevent/plugins/
> > + *  - Directories, specified in @tep->plugins_dir and priority TEP_PLUGIN_LAST
>
> We should probably have a man page as well.
I'm planning to add a man page when backporting these patches from
trace-cmd to kernel repo, as there are no libtraceevent man pages in
trace-cmd, they exist only in the kernel tree.

>
> Thanks Tzvetomir!
>
> -- Steve
>
> > + *
> > + */
> >  void tep_load_plugins_hook(struct tep_handle *tep, const char *suffix,
> >                          void (*load_plugin)(struct tep_handle *tep,
> >                                              const char *path,
>


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/8] trace-cmd: Document tep_load_plugins_hook()
  2020-07-14 15:22     ` Tzvetomir Stoyanov
@ 2020-07-14 15:53       ` Steven Rostedt
  0 siblings, 0 replies; 16+ messages in thread
From: Steven Rostedt @ 2020-07-14 15:53 UTC (permalink / raw)
  To: Tzvetomir Stoyanov; +Cc: linux-trace-devel, namhyung

On Tue, 14 Jul 2020 18:22:32 +0300
Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:

> > We should probably have a man page as well.  
> I'm planning to add a man page when backporting these patches from
> trace-cmd to kernel repo, as there are no libtraceevent man pages in
> trace-cmd, they exist only in the kernel tree.

Good point!

Yeah, we'll add a man page when these patches go to the kernel.

Thanks Tzvetomir!

-- Steve

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2020-07-14 15:53 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-14 10:30 [PATCH 0/8] Few libtraceeevnt fixes, suggested Namhyung Kim Tzvetomir Stoyanov (VMware)
2020-07-14 10:30 ` [PATCH 1/8] trace-cmd: Document tep_load_plugins_hook() Tzvetomir Stoyanov (VMware)
2020-07-14 14:28   ` Steven Rostedt
2020-07-14 15:22     ` Tzvetomir Stoyanov
2020-07-14 15:53       ` Steven Rostedt
2020-07-14 10:30 ` [PATCH 2/8] trace-cmd: Handle strdup() error in parse_option_name() Tzvetomir Stoyanov (VMware)
2020-07-14 10:30 ` [PATCH 3/8] trace-cmd: Fix typo in tep_plugin_add_option() description Tzvetomir Stoyanov (VMware)
2020-07-14 10:30 ` [PATCH 4/8] trace-cmd: Improve error handling of tep_plugin_add_option() API Tzvetomir Stoyanov (VMware)
2020-07-14 10:30 ` [PATCH 5/8] trace-cmd: Fixed broken indentation in parse_ip4_print_args() Tzvetomir Stoyanov (VMware)
2020-07-14 10:30 ` [PATCH 6/8] trace-cmd: Fixed type in PRINT_FMT_STING Tzvetomir Stoyanov (VMware)
2020-07-14 10:30 ` [PATCH 7/8] trace-cmd: Fixed description of tep_add_plugin_path() API Tzvetomir Stoyanov (VMware)
2020-07-14 10:30 ` [PATCH 8/8] trace-cmd: Handle possible strdup() error in " Tzvetomir Stoyanov (VMware)
2020-07-14 12:07 ` [PATCH 0/8] Few libtraceeevnt fixes, suggested Namhyung Kim Namhyung Kim
2020-07-14 12:12   ` Tzvetomir Stoyanov
2020-07-14 14:25     ` Steven Rostedt
2020-07-14 14:22   ` Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).