* [PATCH] Introduce new libtracevent API: tep_override_comm()
@ 2018-08-30 15:09 Tzvetomir Stoyanov (VMware)
2018-08-30 16:32 ` Steven Rostedt
2018-08-31 6:27 ` [PATCH v2] " Tzvetomir Stoyanov (VMware)
0 siblings, 2 replies; 6+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2018-08-30 15:09 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] Introduce new libtracevent API: tep_override_comm()
2018-08-30 15:09 [PATCH] Introduce new libtracevent API: tep_override_comm() Tzvetomir Stoyanov (VMware)
@ 2018-08-30 16:32 ` Steven Rostedt
2018-08-31 6:27 ` [PATCH v2] " Tzvetomir Stoyanov (VMware)
1 sibling, 0 replies; 6+ messages in thread
From: Steven Rostedt @ 2018-08-30 16:32 UTC (permalink / raw)
To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel
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);
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2] Introduce new libtracevent API: tep_override_comm()
2018-08-30 15:09 [PATCH] Introduce new libtracevent API: tep_override_comm() Tzvetomir Stoyanov (VMware)
2018-08-30 16:32 ` Steven Rostedt
@ 2018-08-31 6:27 ` Tzvetomir Stoyanov (VMware)
2018-10-16 6:46 ` Ceco
1 sibling, 1 reply; 6+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2018-08-31 6:27 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] Introduce new libtracevent API: tep_override_comm()
2018-08-31 6:27 ` [PATCH v2] " Tzvetomir Stoyanov (VMware)
@ 2018-10-16 6:46 ` Ceco
2018-10-16 11:43 ` Steven Rostedt
2018-10-16 18:22 ` Steven Rostedt
0 siblings, 2 replies; 6+ messages in thread
From: Ceco @ 2018-10-16 6:46 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] Introduce new libtracevent API: tep_override_comm()
2018-10-16 6:46 ` Ceco
@ 2018-10-16 11:43 ` Steven Rostedt
2018-10-16 18:22 ` Steven Rostedt
1 sibling, 0 replies; 6+ messages in thread
From: Steven Rostedt @ 2018-10-16 11:43 UTC (permalink / raw)
To: Ceco; +Cc: linux-trace-devel
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:
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] Introduce new libtracevent API: tep_override_comm()
2018-10-16 6:46 ` Ceco
2018-10-16 11:43 ` Steven Rostedt
@ 2018-10-16 18:22 ` Steven Rostedt
1 sibling, 0 replies; 6+ messages in thread
From: Steven Rostedt @ 2018-10-16 18:22 UTC (permalink / raw)
To: Ceco; +Cc: linux-trace-devel
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
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-10-17 2:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-30 15:09 [PATCH] Introduce new libtracevent API: tep_override_comm() Tzvetomir Stoyanov (VMware)
2018-08-30 16:32 ` Steven Rostedt
2018-08-31 6:27 ` [PATCH v2] " Tzvetomir Stoyanov (VMware)
2018-10-16 6:46 ` Ceco
2018-10-16 11:43 ` Steven Rostedt
2018-10-16 18: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).