[PATCH] Introduce new libtracevent API: tep_override_comm()

[PATCH] Introduce new libtracevent API: tep_override_comm()

[Tzvetomir Stoyanov (VMware)]

Added a new API of tracevent library: tep_override_comm()
It registers a pid / comm mapping. If a mapping with the same
pid already exists, the entry is updated with the new comm.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/traceevent/event-parse.h |  1 +
 lib/traceevent/event-parse.c     | 67 +++++++++++++++++++++++++-------
 2 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/include/traceevent/event-parse.h b/include/traceevent/event-parse.h
index 713a4e4..0dcc024 100644
--- a/include/traceevent/event-parse.h
+++ b/include/traceevent/event-parse.h
@@ -607,6 +607,7 @@ int tep_set_function_resolver(struct tep_handle *pevent,
 			      tep_func_resolver_t *func, void *priv);
 void tep_reset_function_resolver(struct tep_handle *pevent);
 int tep_register_comm(struct tep_handle *pevent, const char *comm, int pid);
+int tep_override_comm(struct tep_handle *pevent, const char *comm, int pid);
 int tep_register_trace_clock(struct tep_handle *pevent, const char *trace_clock);
 int tep_register_function(struct tep_handle *pevent, char *name,
 			  unsigned long long addr, char *mod);
diff --git a/lib/traceevent/event-parse.c b/lib/traceevent/event-parse.c
index ce1e202..e40f63c 100644
--- a/lib/traceevent/event-parse.c
+++ b/lib/traceevent/event-parse.c
@@ -230,10 +230,11 @@ int tep_pid_is_registered(struct tep_handle *pevent, int pid)
  * we must add this pid. This is much slower than when cmdlines
  * are added before the array is initialized.
  */
-static int add_new_comm(struct tep_handle *pevent, const char *comm, int pid)
+static int add_new_comm(struct tep_handle *pevent,
+			const char *comm, int pid, int overrite)
 {
 	struct cmdline *cmdlines = pevent->cmdlines;
-	const struct cmdline *cmdline;
+	struct cmdline *cmdline;
 	struct cmdline key;
 
 	if (!pid)
@@ -245,8 +246,18 @@ static int add_new_comm(struct tep_handle *pevent, const char *comm, int pid)
 	cmdline = bsearch(&key, pevent->cmdlines, pevent->cmdline_count,
 		       sizeof(*pevent->cmdlines), cmdline_cmp);
 	if (cmdline) {
-		errno = EEXIST;
-		return -1;
+		if (!overrite) {
+			errno = EEXIST;
+			return -1;
+		}
+		if (cmdline->comm)
+			free(cmdline->comm);
+		cmdline->comm = strdup(comm);
+		if(!cmdline->comm) {
+			errno = ENOMEM;
+			return -1;
+		}
+		return 0;
 	}
 
 	cmdlines = realloc(cmdlines, sizeof(*cmdlines) * (pevent->cmdline_count + 1));
@@ -273,21 +284,13 @@ static int add_new_comm(struct tep_handle *pevent, const char *comm, int pid)
 	return 0;
 }
 
-/**
- * tep_register_comm - register a pid / comm mapping
- * @pevent: handle for the pevent
- * @comm: the command line to register
- * @pid: the pid to map the command line to
- *
- * This adds a mapping to search for command line names with
- * a given pid. The comm is duplicated.
- */
-int tep_register_comm(struct tep_handle *pevent, const char *comm, int pid)
+static int _tep_register_comm(struct tep_handle *pevent,
+			      const char *comm, int pid, int overrite)
 {
 	struct cmdline_list *item;
 
 	if (pevent->cmdlines)
-		return add_new_comm(pevent, comm, pid);
+		return add_new_comm(pevent, comm, pid, overrite);
 
 	item = malloc(sizeof(*item));
 	if (!item)
@@ -310,6 +313,40 @@ int tep_register_comm(struct tep_handle *pevent, const char *comm, int pid)
 	return 0;
 }
 
+/**
+ * tep_register_comm - register a pid / comm mapping
+ * @pevent: handle for the pevent
+ * @comm: the command line to register
+ * @pid: the pid to map the command line to
+ *
+ * This adds a mapping to search for command line names with
+ * a given pid. The comm is duplicated. If a command with the same pid
+ * already exist, -1 is returned and errno is set to EEXIST
+ */
+int tep_register_comm(struct tep_handle *pevent, const char *comm, int pid)
+{
+	return _tep_register_comm(pevent, comm, pid, 0);
+}
+
+/**
+ * tep_override_comm - register a pid / comm mapping
+ * @pevent: handle for the pevent
+ * @comm: the command line to register
+ * @pid: the pid to map the command line to
+ *
+ * This adds a mapping to search for command line names with
+ * a given pid. The comm is duplicated. If a command with the same pid
+ * already exist, the command string is udapted with the new one
+ */
+int tep_override_comm(struct tep_handle *pevent, const char *comm, int pid)
+{
+	if (!pevent->cmdlines && cmdline_init(pevent)) {
+		errno = ENOMEM;
+		return -1;
+	}
+	return _tep_register_comm(pevent, comm, pid, 1);
+}
+
 int tep_register_trace_clock(struct tep_handle *pevent, const char *trace_clock)
 {
 	pevent->trace_clock = strdup(trace_clock);
-- 
2.17.1

Re: [PATCH] Introduce new libtracevent API: tep_override_comm()

[Steven Rostedt]

On Thu, 30 Aug 2018 18:09:00 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> Added a new API of tracevent library: tep_override_comm()
> It registers a pid / comm mapping. If a mapping with the same
> pid already exists, the entry is updated with the new comm.
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  include/traceevent/event-parse.h |  1 +
>  lib/traceevent/event-parse.c     | 67 +++++++++++++++++++++++++-------
>  2 files changed, 53 insertions(+), 15 deletions(-)
> 
> diff --git a/include/traceevent/event-parse.h b/include/traceevent/event-parse.h
> index 713a4e4..0dcc024 100644
> --- a/include/traceevent/event-parse.h
> +++ b/include/traceevent/event-parse.h
> @@ -607,6 +607,7 @@ int tep_set_function_resolver(struct tep_handle *pevent,
>  			      tep_func_resolver_t *func, void *priv);
>  void tep_reset_function_resolver(struct tep_handle *pevent);
>  int tep_register_comm(struct tep_handle *pevent, const char *comm, int pid);
> +int tep_override_comm(struct tep_handle *pevent, const char *comm, int pid);
>  int tep_register_trace_clock(struct tep_handle *pevent, const char *trace_clock);
>  int tep_register_function(struct tep_handle *pevent, char *name,
>  			  unsigned long long addr, char *mod);
> diff --git a/lib/traceevent/event-parse.c b/lib/traceevent/event-parse.c
> index ce1e202..e40f63c 100644
> --- a/lib/traceevent/event-parse.c
> +++ b/lib/traceevent/event-parse.c
> @@ -230,10 +230,11 @@ int tep_pid_is_registered(struct tep_handle *pevent, int pid)
>   * we must add this pid. This is much slower than when cmdlines
>   * are added before the array is initialized.
>   */
> -static int add_new_comm(struct tep_handle *pevent, const char *comm, int pid)
> +static int add_new_comm(struct tep_handle *pevent,
> +			const char *comm, int pid, int overrite)

Typo, should be "overwrite" or "override". Also make it a "bool" type.

>  {
>  	struct cmdline *cmdlines = pevent->cmdlines;
> -	const struct cmdline *cmdline;
> +	struct cmdline *cmdline;
>  	struct cmdline key;
>  
>  	if (!pid)
> @@ -245,8 +246,18 @@ static int add_new_comm(struct tep_handle *pevent, const char *comm, int pid)
>  	cmdline = bsearch(&key, pevent->cmdlines, pevent->cmdline_count,
>  		       sizeof(*pevent->cmdlines), cmdline_cmp);
>  	if (cmdline) {
> -		errno = EEXIST;
> -		return -1;
> +		if (!overrite) {
> +			errno = EEXIST;
> +			return -1;
> +		}
> +		if (cmdline->comm)
> +			free(cmdline->comm);
> +		cmdline->comm = strdup(comm);

We don't want to lose the old comm if the new comm fails to be
allocated. Thus, use a temp variable to save the strdup().

		char *new_comm;

		new_comm = strdup(comm);
		if (!new_comm) {
			errno = ENOMEM;
			return -1;
		}

		free(cmdline->comm);
		cmdline->comm = new_comm;

> +		if(!cmdline->comm) {
> +			errno = ENOMEM;
> +			return -1;
> +		}
> +		return 0;
>  	}
>  
>  	cmdlines = realloc(cmdlines, sizeof(*cmdlines) * (pevent->cmdline_count + 1));
> @@ -273,21 +284,13 @@ static int add_new_comm(struct tep_handle *pevent, const char *comm, int pid)
>  	return 0;
>  }
>  
> -/**
> - * tep_register_comm - register a pid / comm mapping
> - * @pevent: handle for the pevent
> - * @comm: the command line to register
> - * @pid: the pid to map the command line to
> - *
> - * This adds a mapping to search for command line names with
> - * a given pid. The comm is duplicated.
> - */
> -int tep_register_comm(struct tep_handle *pevent, const char *comm, int pid)
> +static int _tep_register_comm(struct tep_handle *pevent,
> +			      const char *comm, int pid, int overrite)

Same comment here for "overrite".

>  {
>  	struct cmdline_list *item;
>  
>  	if (pevent->cmdlines)
> -		return add_new_comm(pevent, comm, pid);
> +		return add_new_comm(pevent, comm, pid, overrite);
>  
>  	item = malloc(sizeof(*item));
>  	if (!item)
> @@ -310,6 +313,40 @@ int tep_register_comm(struct tep_handle *pevent, const char *comm, int pid)
>  	return 0;
>  }
>  
> +/**
> + * tep_register_comm - register a pid / comm mapping
> + * @pevent: handle for the pevent
> + * @comm: the command line to register
> + * @pid: the pid to map the command line to
> + *
> + * This adds a mapping to search for command line names with
> + * a given pid. The comm is duplicated. If a command with the same pid
> + * already exist, -1 is returned and errno is set to EEXIST
> + */
> +int tep_register_comm(struct tep_handle *pevent, const char *comm, int pid)
> +{
> +	return _tep_register_comm(pevent, comm, pid, 0);

Pass in "false" instead of 0.

> +}
> +
> +/**
> + * tep_override_comm - register a pid / comm mapping
> + * @pevent: handle for the pevent
> + * @comm: the command line to register
> + * @pid: the pid to map the command line to
> + *
> + * This adds a mapping to search for command line names with
> + * a given pid. The comm is duplicated. If a command with the same pid
> + * already exist, the command string is udapted with the new one
> + */
> +int tep_override_comm(struct tep_handle *pevent, const char *comm, int pid)
> +{
> +	if (!pevent->cmdlines && cmdline_init(pevent)) {
> +		errno = ENOMEM;
> +		return -1;
> +	}
> +	return _tep_register_comm(pevent, comm, pid, 1);

Pass in "true" instead of 1.

> +}
> +

Rest looks good.

Thanks Tzvetomir!

-- Steve

>  int tep_register_trace_clock(struct tep_handle *pevent, const char *trace_clock)
>  {
>  	pevent->trace_clock = strdup(trace_clock);

[PATCH v2] Introduce new libtracevent API: tep_override_comm()

[Tzvetomir Stoyanov (VMware)]

Added a new API of tracevent library: tep_override_comm()
It registers a pid / comm mapping. If a mapping with the same
pid already exists, the entry is updated with the new comm.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/traceevent/event-parse.h |  1 +
 lib/traceevent/event-parse.c     | 70 +++++++++++++++++++++++++-------
 2 files changed, 56 insertions(+), 15 deletions(-)

diff --git a/include/traceevent/event-parse.h b/include/traceevent/event-parse.h
index 713a4e4..0dcc024 100644
--- a/include/traceevent/event-parse.h
+++ b/include/traceevent/event-parse.h
@@ -607,6 +607,7 @@ int tep_set_function_resolver(struct tep_handle *pevent,
 			      tep_func_resolver_t *func, void *priv);
 void tep_reset_function_resolver(struct tep_handle *pevent);
 int tep_register_comm(struct tep_handle *pevent, const char *comm, int pid);
+int tep_override_comm(struct tep_handle *pevent, const char *comm, int pid);
 int tep_register_trace_clock(struct tep_handle *pevent, const char *trace_clock);
 int tep_register_function(struct tep_handle *pevent, char *name,
 			  unsigned long long addr, char *mod);
diff --git a/lib/traceevent/event-parse.c b/lib/traceevent/event-parse.c
index ce1e202..1954908 100644
--- a/lib/traceevent/event-parse.c
+++ b/lib/traceevent/event-parse.c
@@ -230,11 +230,13 @@ int tep_pid_is_registered(struct tep_handle *pevent, int pid)
  * we must add this pid. This is much slower than when cmdlines
  * are added before the array is initialized.
  */
-static int add_new_comm(struct tep_handle *pevent, const char *comm, int pid)
+static int add_new_comm(struct tep_handle *pevent,
+			const char *comm, int pid, bool override)
 {
 	struct cmdline *cmdlines = pevent->cmdlines;
-	const struct cmdline *cmdline;
+	struct cmdline *cmdline;
 	struct cmdline key;
+	char *new_comm;
 
 	if (!pid)
 		return 0;
@@ -245,8 +247,20 @@ static int add_new_comm(struct tep_handle *pevent, const char *comm, int pid)
 	cmdline = bsearch(&key, pevent->cmdlines, pevent->cmdline_count,
 		       sizeof(*pevent->cmdlines), cmdline_cmp);
 	if (cmdline) {
-		errno = EEXIST;
-		return -1;
+		if (!override) {
+			errno = EEXIST;
+			return -1;
+		}
+		new_comm = strdup(comm);
+		if(!new_comm) {
+			errno = ENOMEM;
+			return -1;
+		}
+		if (cmdline->comm)
+			free(cmdline->comm);
+		cmdline->comm = new_comm;
+
+		return 0;
 	}
 
 	cmdlines = realloc(cmdlines, sizeof(*cmdlines) * (pevent->cmdline_count + 1));
@@ -273,21 +287,13 @@ static int add_new_comm(struct tep_handle *pevent, const char *comm, int pid)
 	return 0;
 }
 
-/**
- * tep_register_comm - register a pid / comm mapping
- * @pevent: handle for the pevent
- * @comm: the command line to register
- * @pid: the pid to map the command line to
- *
- * This adds a mapping to search for command line names with
- * a given pid. The comm is duplicated.
- */
-int tep_register_comm(struct tep_handle *pevent, const char *comm, int pid)
+static int _tep_register_comm(struct tep_handle *pevent,
+			      const char *comm, int pid, bool override)
 {
 	struct cmdline_list *item;
 
 	if (pevent->cmdlines)
-		return add_new_comm(pevent, comm, pid);
+		return add_new_comm(pevent, comm, pid, override);
 
 	item = malloc(sizeof(*item));
 	if (!item)
@@ -310,6 +316,40 @@ int tep_register_comm(struct tep_handle *pevent, const char *comm, int pid)
 	return 0;
 }
 
+/**
+ * tep_register_comm - register a pid / comm mapping
+ * @pevent: handle for the pevent
+ * @comm: the command line to register
+ * @pid: the pid to map the command line to
+ *
+ * This adds a mapping to search for command line names with
+ * a given pid. The comm is duplicated. If a command with the same pid
+ * already exist, -1 is returned and errno is set to EEXIST
+ */
+int tep_register_comm(struct tep_handle *pevent, const char *comm, int pid)
+{
+	return _tep_register_comm(pevent, comm, pid, false);
+}
+
+/**
+ * tep_override_comm - register a pid / comm mapping
+ * @pevent: handle for the pevent
+ * @comm: the command line to register
+ * @pid: the pid to map the command line to
+ *
+ * This adds a mapping to search for command line names with
+ * a given pid. The comm is duplicated. If a command with the same pid
+ * already exist, the command string is udapted with the new one
+ */
+int tep_override_comm(struct tep_handle *pevent, const char *comm, int pid)
+{
+	if (!pevent->cmdlines && cmdline_init(pevent)) {
+		errno = ENOMEM;
+		return -1;
+	}
+	return _tep_register_comm(pevent, comm, pid, true);
+}
+
 int tep_register_trace_clock(struct tep_handle *pevent, const char *trace_clock)
 {
 	pevent->trace_clock = strdup(trace_clock);
-- 
2.17.1

Re: [PATCH v2] Introduce new libtracevent API: tep_override_comm()

[Ceco]

ping
Hi Steven, is there a problem with this patch ? Yordan needs these
changes, he wants to use the new API in KernelShark.

On Fri, Aug 31, 2018 at 9:27 AM Tzvetomir Stoyanov (VMware)
<tz.stoyanov@gmail.com> wrote:
>
> Added a new API of tracevent library: tep_override_comm()
> It registers a pid / comm mapping. If a mapping with the same
> pid already exists, the entry is updated with the new comm.
>
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  include/traceevent/event-parse.h |  1 +
>  lib/traceevent/event-parse.c     | 70 +++++++++++++++++++++++++-------
>  2 files changed, 56 insertions(+), 15 deletions(-)
>
> diff --git a/include/traceevent/event-parse.h b/include/traceevent/event-parse.h
> index 713a4e4..0dcc024 100644
> --- a/include/traceevent/event-parse.h
> +++ b/include/traceevent/event-parse.h
> @@ -607,6 +607,7 @@ int tep_set_function_resolver(struct tep_handle *pevent,
>                               tep_func_resolver_t *func, void *priv);
>  void tep_reset_function_resolver(struct tep_handle *pevent);
>  int tep_register_comm(struct tep_handle *pevent, const char *comm, int pid);
> +int tep_override_comm(struct tep_handle *pevent, const char *comm, int pid);
>  int tep_register_trace_clock(struct tep_handle *pevent, const char *trace_clock);
>  int tep_register_function(struct tep_handle *pevent, char *name,
>                           unsigned long long addr, char *mod);
> diff --git a/lib/traceevent/event-parse.c b/lib/traceevent/event-parse.c
> index ce1e202..1954908 100644
> --- a/lib/traceevent/event-parse.c
> +++ b/lib/traceevent/event-parse.c
> @@ -230,11 +230,13 @@ int tep_pid_is_registered(struct tep_handle *pevent, int pid)
>   * we must add this pid. This is much slower than when cmdlines
>   * are added before the array is initialized.
>   */
> -static int add_new_comm(struct tep_handle *pevent, const char *comm, int pid)
> +static int add_new_comm(struct tep_handle *pevent,
> +                       const char *comm, int pid, bool override)
>  {
>         struct cmdline *cmdlines = pevent->cmdlines;
> -       const struct cmdline *cmdline;
> +       struct cmdline *cmdline;
>         struct cmdline key;
> +       char *new_comm;
>
>         if (!pid)
>                 return 0;
> @@ -245,8 +247,20 @@ static int add_new_comm(struct tep_handle *pevent, const char *comm, int pid)
>         cmdline = bsearch(&key, pevent->cmdlines, pevent->cmdline_count,
>                        sizeof(*pevent->cmdlines), cmdline_cmp);
>         if (cmdline) {
> -               errno = EEXIST;
> -               return -1;
> +               if (!override) {
> +                       errno = EEXIST;
> +                       return -1;
> +               }
> +               new_comm = strdup(comm);
> +               if(!new_comm) {
> +                       errno = ENOMEM;
> +                       return -1;
> +               }
> +               if (cmdline->comm)
> +                       free(cmdline->comm);
> +               cmdline->comm = new_comm;
> +
> +               return 0;
>         }
>
>         cmdlines = realloc(cmdlines, sizeof(*cmdlines) * (pevent->cmdline_count + 1));
> @@ -273,21 +287,13 @@ static int add_new_comm(struct tep_handle *pevent, const char *comm, int pid)
>         return 0;
>  }
>
> -/**
> - * tep_register_comm - register a pid / comm mapping
> - * @pevent: handle for the pevent
> - * @comm: the command line to register
> - * @pid: the pid to map the command line to
> - *
> - * This adds a mapping to search for command line names with
> - * a given pid. The comm is duplicated.
> - */
> -int tep_register_comm(struct tep_handle *pevent, const char *comm, int pid)
> +static int _tep_register_comm(struct tep_handle *pevent,
> +                             const char *comm, int pid, bool override)
>  {
>         struct cmdline_list *item;
>
>         if (pevent->cmdlines)
> -               return add_new_comm(pevent, comm, pid);
> +               return add_new_comm(pevent, comm, pid, override);
>
>         item = malloc(sizeof(*item));
>         if (!item)
> @@ -310,6 +316,40 @@ int tep_register_comm(struct tep_handle *pevent, const char *comm, int pid)
>         return 0;
>  }
>
> +/**
> + * tep_register_comm - register a pid / comm mapping
> + * @pevent: handle for the pevent
> + * @comm: the command line to register
> + * @pid: the pid to map the command line to
> + *
> + * This adds a mapping to search for command line names with
> + * a given pid. The comm is duplicated. If a command with the same pid
> + * already exist, -1 is returned and errno is set to EEXIST
> + */
> +int tep_register_comm(struct tep_handle *pevent, const char *comm, int pid)
> +{
> +       return _tep_register_comm(pevent, comm, pid, false);
> +}
> +
> +/**
> + * tep_override_comm - register a pid / comm mapping
> + * @pevent: handle for the pevent
> + * @comm: the command line to register
> + * @pid: the pid to map the command line to
> + *
> + * This adds a mapping to search for command line names with
> + * a given pid. The comm is duplicated. If a command with the same pid
> + * already exist, the command string is udapted with the new one
> + */
> +int tep_override_comm(struct tep_handle *pevent, const char *comm, int pid)
> +{
> +       if (!pevent->cmdlines && cmdline_init(pevent)) {
> +               errno = ENOMEM;
> +               return -1;
> +       }
> +       return _tep_register_comm(pevent, comm, pid, true);
> +}
> +
>  int tep_register_trace_clock(struct tep_handle *pevent, const char *trace_clock)
>  {
>         pevent->trace_clock = strdup(trace_clock);
> --
> 2.17.1
>

Re: [PATCH v2] Introduce new libtracevent API: tep_override_comm()

[Steven Rostedt]

On Tue, 16 Oct 2018 09:46:07 +0300
Ceco <tz.stoyanov@gmail.com> wrote:

> ping
> Hi Steven, is there a problem with this patch ? Yordan needs these
> changes, he wants to use the new API in KernelShark.

Just woke up, so I haven't had a chance to look at the patch. But the
one thing that is wrong with it, is that a v2 shouldn't be a reply to a
v1. That is, it should start a new thread. With thousands of emails in
my Inbox, I look at email threads (as do many kernel developers), and
if there's a patch within the thread, it is commonly missed.

I will say that I did mark it to look at it, so I did see it. But other
patches came in during that time, and I sorted my inbox by thread, which
did hide the patch again.

I'll take a look at it later today.

Also, don't be afraid to send a ping if there's no response for a week,
as that usually means it was lost in the flood.

Thanks!

-- Steve


> 
> On Fri, Aug 31, 2018 at 9:27 AM Tzvetomir Stoyanov (VMware)
> <tz.stoyanov@gmail.com> wrote:
> >

Re: [PATCH v2] Introduce new libtracevent API: tep_override_comm()

[Steven Rostedt]

On Tue, 16 Oct 2018 09:46:07 +0300
Ceco <tz.stoyanov@gmail.com> wrote:

> ping
> Hi Steven, is there a problem with this patch ? Yordan needs these
> changes, he wants to use the new API in KernelShark.
> 
> On Fri, Aug 31, 2018 at 9:27 AM Tzvetomir Stoyanov (VMware)
> <tz.stoyanov@gmail.com> wrote:
> >
> > Added a new API of tracevent library: tep_override_comm()
> > It registers a pid / comm mapping. If a mapping with the same
> > pid already exists, the entry is updated with the new comm.
> >
> > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> > ---
> >  include/traceevent/event-parse.h |  1 +
> >  lib/traceevent/event-parse.c     | 70 +++++++++++++++++++++++++-------
> >  2 files changed, 56 insertions(+), 15 deletions(-)
> >
> > diff --git a/include/traceevent/event-parse.h b/include/traceevent/event-parse.h
> > index 713a4e4..0dcc024 100644
> > --- a/include/traceevent/event-parse.h
> > +++ b/include/traceevent/event-parse.h
> > @@ -607,6 +607,7 @@ int tep_set_function_resolver(struct tep_handle *pevent,
> >                               tep_func_resolver_t *func, void *priv);
> >  void tep_reset_function_resolver(struct tep_handle *pevent);
> >  int tep_register_comm(struct tep_handle *pevent, const char *comm, int pid);
> > +int tep_override_comm(struct tep_handle *pevent, const char *comm, int pid);
> >  int tep_register_trace_clock(struct tep_handle *pevent, const char *trace_clock);
> >  int tep_register_function(struct tep_handle *pevent, char *name,
> >                           unsigned long long addr, char *mod);
> > diff --git a/lib/traceevent/event-parse.c b/lib/traceevent/event-parse.c
> > index ce1e202..1954908 100644
> > --- a/lib/traceevent/event-parse.c
> > +++ b/lib/traceevent/event-parse.c
> > @@ -230,11 +230,13 @@ int tep_pid_is_registered(struct tep_handle *pevent, int pid)
> >   * we must add this pid. This is much slower than when cmdlines
> >   * are added before the array is initialized.
> >   */
> > -static int add_new_comm(struct tep_handle *pevent, const char *comm, int pid)
> > +static int add_new_comm(struct tep_handle *pevent,
> > +                       const char *comm, int pid, bool override)
> >  {
> >         struct cmdline *cmdlines = pevent->cmdlines;
> > -       const struct cmdline *cmdline;
> > +       struct cmdline *cmdline;
> >         struct cmdline key;
> > +       char *new_comm;
> >
> >         if (!pid)
> >                 return 0;
> > @@ -245,8 +247,20 @@ static int add_new_comm(struct tep_handle *pevent, const char *comm, int pid)
> >         cmdline = bsearch(&key, pevent->cmdlines, pevent->cmdline_count,
> >                        sizeof(*pevent->cmdlines), cmdline_cmp);
> >         if (cmdline) {
> > -               errno = EEXIST;
> > -               return -1;
> > +               if (!override) {
> > +                       errno = EEXIST;
> > +                       return -1;
> > +               }
> > +               new_comm = strdup(comm);
> > +               if(!new_comm) {
> > +                       errno = ENOMEM;
> > +                       return -1;
> > +               }
> > +               if (cmdline->comm)
> > +                       free(cmdline->comm);

One nit here. free() can take a NULL parameter without issue, so the if
isn't necessary.

		free(cmdline->comm);
		cmdline->comm = new_comm;

Other than that, the rest looks good.

Want to send a v3 (new thread).

Thanks!

-- Steve

> > +               cmdline->comm = new_comm;
> > +
> > +               return 0;
> >         }
> >
> >         cmdlines = realloc(cmdlines, sizeof(*cmdlines) * (pevent->cmdline_count + 1));
> > @@ -273,21 +287,13 @@ static int add_new_comm(struct tep_handle *pevent, const char *comm, int pid)
> >         return 0;
> >  }
> >
> > -/**
> > - * tep_register_comm - register a pid / comm mapping
> > - * @pevent: handle for the pevent
> > - * @comm: the command line to register
> > - * @pid: the pid to map the command line to
> > - *
> > - * This adds a mapping to search for command line names with
> > - * a given pid. The comm is duplicated.
> > - */
> > -int tep_register_comm(struct tep_handle *pevent, const char *comm, int pid)
> > +static int _tep_register_comm(struct tep_handle *pevent,
> > +                             const char *comm, int pid, bool override)
> >  {
> >         struct cmdline_list *item;
> >
> >         if (pevent->cmdlines)
> > -               return add_new_comm(pevent, comm, pid);
> > +               return add_new_comm(pevent, comm, pid, override);
> >
> >         item = malloc(sizeof(*item));
> >         if (!item)
> > @@ -310,6 +316,40 @@ int tep_register_comm(struct tep_handle *pevent, const char *comm, int pid)
> >         return 0;
> >  }
> >
> > +/**
> > + * tep_register_comm - register a pid / comm mapping
> > + * @pevent: handle for the pevent
> > + * @comm: the command line to register
> > + * @pid: the pid to map the command line to
> > + *
> > + * This adds a mapping to search for command line names with
> > + * a given pid. The comm is duplicated. If a command with the same pid
> > + * already exist, -1 is returned and errno is set to EEXIST
> > + */
> > +int tep_register_comm(struct tep_handle *pevent, const char *comm, int pid)
> > +{
> > +       return _tep_register_comm(pevent, comm, pid, false);
> > +}
> > +
> > +/**
> > + * tep_override_comm - register a pid / comm mapping
> > + * @pevent: handle for the pevent
> > + * @comm: the command line to register
> > + * @pid: the pid to map the command line to
> > + *
> > + * This adds a mapping to search for command line names with
> > + * a given pid. The comm is duplicated. If a command with the same pid
> > + * already exist, the command string is udapted with the new one
> > + */
> > +int tep_override_comm(struct tep_handle *pevent, const char *comm, int pid)
> > +{
> > +       if (!pevent->cmdlines && cmdline_init(pevent)) {
> > +               errno = ENOMEM;
> > +               return -1;
> > +       }
> > +       return _tep_register_comm(pevent, comm, pid, true);
> > +}
> > +
> >  int tep_register_trace_clock(struct tep_handle *pevent, const char *trace_clock)
> >  {
> >         pevent->trace_clock = strdup(trace_clock);
> > --
> > 2.17.1
> >  

[PATCH v2] Introduce new libtracevent API: tep_override_comm()

[Tzvetomir Stoyanov]

From: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>

Added a new API of tracevent library: tep_override_comm()
It registers a pid / comm mapping. If a mapping with the same
pid already exists, the entry is updated with the new comm.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 include/traceevent/event-parse.h |  1 +
 lib/traceevent/event-parse.c     | 69 +++++++++++++++++++++++++-------
 2 files changed, 55 insertions(+), 15 deletions(-)

diff --git a/include/traceevent/event-parse.h b/include/traceevent/event-parse.h
index 18652c8..147efbc 100644
--- a/include/traceevent/event-parse.h
+++ b/include/traceevent/event-parse.h
@@ -435,6 +435,7 @@ int tep_set_function_resolver(struct tep_handle *pevent,
 			      tep_func_resolver_t *func, void *priv);
 void tep_reset_function_resolver(struct tep_handle *pevent);
 int tep_register_comm(struct tep_handle *pevent, const char *comm, int pid);
+int tep_override_comm(struct tep_handle *pevent, const char *comm, int pid);
 int tep_register_trace_clock(struct tep_handle *pevent, const char *trace_clock);
 int tep_register_function(struct tep_handle *pevent, char *name,
 			  unsigned long long addr, char *mod);
diff --git a/lib/traceevent/event-parse.c b/lib/traceevent/event-parse.c
index d889a62..73662d0 100644
--- a/lib/traceevent/event-parse.c
+++ b/lib/traceevent/event-parse.c
@@ -233,11 +233,13 @@ int tep_pid_is_registered(struct tep_handle *pevent, int pid)
  * we must add this pid. This is much slower than when cmdlines
  * are added before the array is initialized.
  */
-static int add_new_comm(struct tep_handle *pevent, const char *comm, int pid)
+static int add_new_comm(struct tep_handle *pevent,
+			const char *comm, int pid, bool override)
 {
 	struct cmdline *cmdlines = pevent->cmdlines;
-	const struct cmdline *cmdline;
+	struct cmdline *cmdline;
 	struct cmdline key;
+	char *new_comm;
 
 	if (!pid)
 		return 0;
@@ -248,8 +250,19 @@ static int add_new_comm(struct tep_handle *pevent, const char *comm, int pid)
 	cmdline = bsearch(&key, pevent->cmdlines, pevent->cmdline_count,
 		       sizeof(*pevent->cmdlines), cmdline_cmp);
 	if (cmdline) {
-		errno = EEXIST;
-		return -1;
+		if (!override) {
+			errno = EEXIST;
+			return -1;
+		}
+		new_comm = strdup(comm);
+		if(!new_comm) {
+			errno = ENOMEM;
+			return -1;
+		}
+		free(cmdline->comm);
+		cmdline->comm = new_comm;
+
+		return 0;
 	}
 
 	cmdlines = realloc(cmdlines, sizeof(*cmdlines) * (pevent->cmdline_count + 1));
@@ -276,21 +289,13 @@ static int add_new_comm(struct tep_handle *pevent, const char *comm, int pid)
 	return 0;
 }
 
-/**
- * tep_register_comm - register a pid / comm mapping
- * @pevent: handle for the pevent
- * @comm: the command line to register
- * @pid: the pid to map the command line to
- *
- * This adds a mapping to search for command line names with
- * a given pid. The comm is duplicated.
- */
-int tep_register_comm(struct tep_handle *pevent, const char *comm, int pid)
+static int _tep_register_comm(struct tep_handle *pevent,
+			      const char *comm, int pid, bool override)
 {
 	struct cmdline_list *item;
 
 	if (pevent->cmdlines)
-		return add_new_comm(pevent, comm, pid);
+		return add_new_comm(pevent, comm, pid, override);
 
 	item = malloc(sizeof(*item));
 	if (!item)
@@ -313,6 +318,40 @@ int tep_register_comm(struct tep_handle *pevent, const char *comm, int pid)
 	return 0;
 }
 
+/**
+ * tep_register_comm - register a pid / comm mapping
+ * @pevent: handle for the pevent
+ * @comm: the command line to register
+ * @pid: the pid to map the command line to
+ *
+ * This adds a mapping to search for command line names with
+ * a given pid. The comm is duplicated. If a command with the same pid
+ * already exist, -1 is returned and errno is set to EEXIST
+ */
+int tep_register_comm(struct tep_handle *pevent, const char *comm, int pid)
+{
+	return _tep_register_comm(pevent, comm, pid, false);
+}
+
+/**
+ * tep_override_comm - register a pid / comm mapping
+ * @pevent: handle for the pevent
+ * @comm: the command line to register
+ * @pid: the pid to map the command line to
+ *
+ * This adds a mapping to search for command line names with
+ * a given pid. The comm is duplicated. If a command with the same pid
+ * already exist, the command string is udapted with the new one
+ */
+int tep_override_comm(struct tep_handle *pevent, const char *comm, int pid)
+{
+	if (!pevent->cmdlines && cmdline_init(pevent)) {
+		errno = ENOMEM;
+		return -1;
+	}
+	return _tep_register_comm(pevent, comm, pid, true);
+}
+
 int tep_register_trace_clock(struct tep_handle *pevent, const char *trace_clock)
 {
 	pevent->trace_clock = strdup(trace_clock);
-- 
2.17.2