2012-08-22 07:07:48

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 0/4] tools lib traceevent: Basic error handling

Hi,

I almost forgot that I have some libtraceevent patch left. ;) The
first three of them are acked by Steven and the last is a port of a
recent perf patch from Kirill that handles a strerror_r build warning.
Please consider applying.

Thanks,
Namhyung


Namhyung Kim (4):
tools lib traceevent: Do not link broken field arg for an old ftrace event
tools lib traceevent: Introduce pevent_errno
tools lib traceevent: Introduce pevent_strerror
tools lib traceevent: Fix strerror_r() use in pevent_strerror

tools/lib/traceevent/event-parse.c | 104 +++++++++++++++++++++++++++++--------
tools/lib/traceevent/event-parse.h | 34 +++++++++++-
tools/lib/traceevent/event-utils.h | 6 +++
3 files changed, 119 insertions(+), 25 deletions(-)

--
1.7.11.4


2012-08-22 07:07:55

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 2/4] tools lib traceevent: Introduce pevent_errno

From: Namhyung Kim <[email protected]>

Define and use error numbers for pevent_parse_event()
and get rid of die() and do_warning() calls. If the
function returns non-zero value, the caller can check
the return code and do appropriate things.

I chose the error numbers to be negative not to clash
with standard errno, and as usual, 0 for success.

Cc: Frederic Weisbecker <[email protected]>
Acked-by: Steven Rostedt <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/lib/traceevent/event-parse.c | 50 +++++++++++++++++++++++---------------
tools/lib/traceevent/event-parse.h | 26 ++++++++++++++++++--
2 files changed, 54 insertions(+), 22 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 33fcd943f096..a46cae701db7 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -4686,9 +4686,8 @@ static int find_event_handle(struct pevent *pevent, struct event_format *event)
*
* /sys/kernel/debug/tracing/events/.../.../format
*/
-int pevent_parse_event(struct pevent *pevent,
- const char *buf, unsigned long size,
- const char *sys)
+enum pevent_errno pevent_parse_event(struct pevent *pevent, const char *buf,
+ unsigned long size, const char *sys)
{
struct event_format *event;
int ret;
@@ -4697,17 +4696,16 @@ int pevent_parse_event(struct pevent *pevent,

event = alloc_event();
if (!event)
- return -ENOMEM;
+ return PEVENT_ERRNO__MEM_ALLOC_FAILED;

event->name = event_read_name();
if (!event->name) {
/* Bad event? */
- free(event);
- return -1;
+ ret = PEVENT_ERRNO__MEM_ALLOC_FAILED;
+ goto event_alloc_failed;
}

if (strcmp(sys, "ftrace") == 0) {
-
event->flags |= EVENT_FL_ISFTRACE;

if (strcmp(event->name, "bprint") == 0)
@@ -4715,20 +4713,28 @@ int pevent_parse_event(struct pevent *pevent,
}

event->id = event_read_id();
- if (event->id < 0)
- die("failed to read event id");
+ if (event->id < 0) {
+ ret = PEVENT_ERRNO__READ_ID_FAILED;
+ /*
+ * This isn't an allocation error actually.
+ * But as the ID is critical, just bail out.
+ */
+ goto event_alloc_failed;
+ }

event->system = strdup(sys);
- if (!event->system)
- die("failed to allocate system");
+ if (!event->system) {
+ ret = PEVENT_ERRNO__MEM_ALLOC_FAILED;
+ goto event_alloc_failed;
+ }

/* Add pevent to event so that it can be referenced */
event->pevent = pevent;

ret = event_read_format(event);
if (ret < 0) {
- do_warning("failed to read event format for %s", event->name);
- goto event_failed;
+ ret = PEVENT_ERRNO__READ_FORMAT_FAILED;
+ goto event_parse_failed;
}

/*
@@ -4740,10 +4746,9 @@ int pevent_parse_event(struct pevent *pevent,

ret = event_read_print(event);
if (ret < 0) {
- do_warning("failed to read event print fmt for %s",
- event->name);
show_warning = 1;
- goto event_failed;
+ ret = PEVENT_ERRNO__READ_PRINT_FAILED;
+ goto event_parse_failed;
}
show_warning = 1;

@@ -4760,10 +4765,9 @@ int pevent_parse_event(struct pevent *pevent,
arg->type = PRINT_FIELD;
arg->field.name = strdup(field->name);
if (!arg->field.name) {
- do_warning("failed to allocate field name");
event->flags |= EVENT_FL_FAILED;
free_arg(arg);
- return -1;
+ return PEVENT_ERRNO__OLD_FTRACE_ARG_FAILED;
}
arg->field.field = field;
*list = arg;
@@ -4778,11 +4782,17 @@ int pevent_parse_event(struct pevent *pevent,

return 0;

- event_failed:
+ event_parse_failed:
event->flags |= EVENT_FL_FAILED;
/* still add it even if it failed */
add_event(pevent, event);
- return -1;
+ return ret;
+
+ event_alloc_failed:
+ free(event->system);
+ free(event->name);
+ free(event);
+ return ret;
}

int get_field_val(struct trace_seq *s, struct format_field *field,
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index 5772ad8cb386..3c48229a7bce 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -345,6 +345,28 @@ enum pevent_flag {
PEVENT_NSEC_OUTPUT = 1, /* output in NSECS */
};

+enum pevent_errno {
+ PEVENT_ERRNO__SUCCESS = 0,
+
+ /*
+ * Choose an arbitrary negative big number not to clash with standard
+ * errno since SUS requires the errno has distinct positive values.
+ * See 'Issue 6' in the link below.
+ *
+ * http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html
+ */
+ __PEVENT_ERRNO__START = -100000,
+
+ PEVENT_ERRNO__MEM_ALLOC_FAILED = __PEVENT_ERRNO__START,
+ PEVENT_ERRNO__PARSE_EVENT_FAILED,
+ PEVENT_ERRNO__READ_ID_FAILED,
+ PEVENT_ERRNO__READ_FORMAT_FAILED,
+ PEVENT_ERRNO__READ_PRINT_FAILED,
+ PEVENT_ERRNO__OLD_FTRACE_ARG_FAILED,
+
+ __PEVENT_ERRNO__END,
+};
+
struct cmdline;
struct cmdline_list;
struct func_map;
@@ -509,8 +531,8 @@ void pevent_print_event(struct pevent *pevent, struct trace_seq *s,
int pevent_parse_header_page(struct pevent *pevent, char *buf, unsigned long size,
int long_size);

-int pevent_parse_event(struct pevent *pevent, const char *buf,
- unsigned long size, const char *sys);
+enum pevent_errno pevent_parse_event(struct pevent *pevent, const char *buf,
+ unsigned long size, const char *sys);

void *pevent_get_field_raw(struct trace_seq *s, struct event_format *event,
const char *name, struct pevent_record *record,
--
1.7.11.4

2012-08-22 07:08:01

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 3/4] tools lib traceevent: Introduce pevent_strerror

From: Namhyung Kim <[email protected]>

The pevent_strerror() sets @buf to a string that describes the
(libtraceevent-specific) error condition that is passed via @errnum.

This is similar to strerror_r() and does same thing if @errnum has a
standard errno value.

To sync error string with its code, define PEVENT_ERRORS with _PE()
macro and use it as suggested by Steven.

Cc: Frederic Weisbecker <[email protected]>
Acked-by: Steven Rostedt <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/lib/traceevent/event-parse.c | 43 ++++++++++++++++++++++++++++++++++++++
tools/lib/traceevent/event-parse.h | 20 ++++++++++++------
2 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index a46cae701db7..1373e4cf109e 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -4795,6 +4795,49 @@ enum pevent_errno pevent_parse_event(struct pevent *pevent, const char *buf,
return ret;
}

+#undef _PE
+#define _PE(code, str) str
+static const char * const pevent_error_str[] = {
+ PEVENT_ERRORS
+};
+#undef _PE
+
+int pevent_strerror(struct pevent *pevent, enum pevent_errno errnum,
+ char *buf, size_t buflen)
+{
+ int idx;
+ const char *msg;
+
+ if (errnum >= 0) {
+ strerror_r(errnum, buf, buflen);
+ return 0;
+ }
+
+ if (errnum <= __PEVENT_ERRNO__START ||
+ errnum >= __PEVENT_ERRNO__END)
+ return -1;
+
+ idx = errnum - __PEVENT_ERRNO__START;
+ msg = pevent_error_str[idx];
+
+ switch (errnum) {
+ case PEVENT_ERRNO__MEM_ALLOC_FAILED:
+ case PEVENT_ERRNO__PARSE_EVENT_FAILED:
+ case PEVENT_ERRNO__READ_ID_FAILED:
+ case PEVENT_ERRNO__READ_FORMAT_FAILED:
+ case PEVENT_ERRNO__READ_PRINT_FAILED:
+ case PEVENT_ERRNO__OLD_FTRACE_ARG_FAILED:
+ snprintf(buf, buflen, "%s", msg);
+ break;
+
+ default:
+ /* cannot reach here */
+ break;
+ }
+
+ return 0;
+}
+
int get_field_val(struct trace_seq *s, struct format_field *field,
const char *name, struct pevent_record *record,
unsigned long long *val, int err)
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index 3c48229a7bce..527df038a25f 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -345,6 +345,16 @@ enum pevent_flag {
PEVENT_NSEC_OUTPUT = 1, /* output in NSECS */
};

+#define PEVENT_ERRORS \
+ _PE(MEM_ALLOC_FAILED, "failed to allocate memory"), \
+ _PE(PARSE_EVENT_FAILED, "failed to parse event"), \
+ _PE(READ_ID_FAILED, "failed to read event id"), \
+ _PE(READ_FORMAT_FAILED, "failed to read event format"), \
+ _PE(READ_PRINT_FAILED, "failed to read event print fmt"), \
+ _PE(OLD_FTRACE_ARG_FAILED,"failed to allocate field name for ftrace")
+
+#undef _PE
+#define _PE(__code, __str) PEVENT_ERRNO__ ## __code
enum pevent_errno {
PEVENT_ERRNO__SUCCESS = 0,

@@ -357,15 +367,11 @@ enum pevent_errno {
*/
__PEVENT_ERRNO__START = -100000,

- PEVENT_ERRNO__MEM_ALLOC_FAILED = __PEVENT_ERRNO__START,
- PEVENT_ERRNO__PARSE_EVENT_FAILED,
- PEVENT_ERRNO__READ_ID_FAILED,
- PEVENT_ERRNO__READ_FORMAT_FAILED,
- PEVENT_ERRNO__READ_PRINT_FAILED,
- PEVENT_ERRNO__OLD_FTRACE_ARG_FAILED,
+ PEVENT_ERRORS,

__PEVENT_ERRNO__END,
};
+#undef _PE

struct cmdline;
struct cmdline_list;
@@ -583,6 +589,8 @@ int pevent_data_pid(struct pevent *pevent, struct pevent_record *rec);
const char *pevent_data_comm_from_pid(struct pevent *pevent, int pid);
void pevent_event_info(struct trace_seq *s, struct event_format *event,
struct pevent_record *record);
+int pevent_strerror(struct pevent *pevent, enum pevent_errno errnum,
+ char *buf, size_t buflen);

struct event_format **pevent_list_events(struct pevent *pevent, enum event_sort_type);
struct format_field **pevent_event_common_fields(struct event_format *event);
--
1.7.11.4

2012-08-22 07:07:58

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 4/4] tools lib traceevent: Fix strerror_r() use in pevent_strerror

From: Namhyung Kim <[email protected]>

glibc-2.16 starts to mark the function with attribute
warn_unused_result so that it can cause a build warning.

Since GNU version of strerror_r() can return a pointer to a string
without setting @buf, check the return value and copy/truncate it to
our buffer if needed.

Cc: Fredereic Weisbecker <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Kirill A. Shutemov <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/lib/traceevent/event-parse.c | 7 ++++++-
tools/lib/traceevent/event-utils.h | 6 ++++++
2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 1373e4cf109e..f978c59f67bf 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -4809,7 +4809,12 @@ int pevent_strerror(struct pevent *pevent, enum pevent_errno errnum,
const char *msg;

if (errnum >= 0) {
- strerror_r(errnum, buf, buflen);
+ msg = strerror_r(errnum, buf, buflen);
+ if (msg != buf) {
+ size_t len = strlen(msg);
+ char *c = mempcpy(buf, msg, min(buflen-1, len));
+ *c = '\0';
+ }
return 0;
}

diff --git a/tools/lib/traceevent/event-utils.h b/tools/lib/traceevent/event-utils.h
index 08296383d1e6..bc075006966e 100644
--- a/tools/lib/traceevent/event-utils.h
+++ b/tools/lib/traceevent/event-utils.h
@@ -39,6 +39,12 @@ void __vdie(const char *fmt, ...);
void __vwarning(const char *fmt, ...);
void __vpr_stat(const char *fmt, ...);

+#define min(x, y) ({ \
+ typeof(x) _min1 = (x); \
+ typeof(y) _min2 = (y); \
+ (void) (&_min1 == &_min2); \
+ _min1 < _min2 ? _min1 : _min2; })
+
static inline char *strim(char *string)
{
char *ret;
--
1.7.11.4

2012-08-22 07:07:51

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 1/4] tools lib traceevent: Do not link broken field arg for an old ftrace event

From: Namhyung Kim <[email protected]>

Defer linking a newly allocated arg to print_fmt.args until
all of its field is setup so that later access to ->field.name
cannot be NULL.

Cc: Frederic Weisbecker <[email protected]>
Acked-by: Steven Rostedt <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/lib/traceevent/event-parse.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index b7c2c491f61e..33fcd943f096 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -4754,20 +4754,20 @@ int pevent_parse_event(struct pevent *pevent,
struct print_arg *arg, **list;

/* old ftrace had no args */
-
list = &event->print_fmt.args;
for (field = event->format.fields; field; field = field->next) {
arg = alloc_arg();
- *list = arg;
- list = &arg->next;
arg->type = PRINT_FIELD;
arg->field.name = strdup(field->name);
if (!arg->field.name) {
do_warning("failed to allocate field name");
event->flags |= EVENT_FL_FAILED;
+ free_arg(arg);
return -1;
}
arg->field.field = field;
+ *list = arg;
+ list = &arg->next;
}
return 0;
}
--
1.7.11.4

2012-08-22 07:31:37

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH 0/4] tools lib traceevent: Basic error handling

Hi Arnaldo,

On Wed, 22 Aug 2012 16:00:27 +0900, Namhyung Kim wrote:
> I almost forgot that I have some libtraceevent patch left. ;) The
> first three of them are acked by Steven and the last is a port of a
> recent perf patch from Kirill that handles a strerror_r build warning.
> Please consider applying.

I also forgot that I didn't remove the Link: line from the changelogs
before sending them, sorry. Anyway, you can pull this series from my
tree which contains libtraceevent/core branch, if you like (I added a
Link to patch 4 there for your convenience).

Thanks,
Namhyung

2012-08-22 08:20:21

by Kirill A. Shutemov

[permalink] [raw]
Subject: Re: [PATCH 4/4] tools lib traceevent: Fix strerror_r() use in pevent_strerror

On Wed, Aug 22, 2012 at 04:00:31PM +0900, Namhyung Kim wrote:
> From: Namhyung Kim <[email protected]>
>
> glibc-2.16 starts to mark the function with attribute
> warn_unused_result so that it can cause a build warning.
>
> Since GNU version of strerror_r() can return a pointer to a string
> without setting @buf, check the return value and copy/truncate it to
> our buffer if needed.
>
> Cc: Fredereic Weisbecker <[email protected]>
> Cc: Steven Rostedt <[email protected]>
> Cc: Kirill A. Shutemov <[email protected]>
> Signed-off-by: Namhyung Kim <[email protected]>

Acked-by: Kirill A. Shutemov <[email protected]>

> ---
> tools/lib/traceevent/event-parse.c | 7 ++++++-
> tools/lib/traceevent/event-utils.h | 6 ++++++
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
> index 1373e4cf109e..f978c59f67bf 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -4809,7 +4809,12 @@ int pevent_strerror(struct pevent *pevent, enum pevent_errno errnum,
> const char *msg;
>
> if (errnum >= 0) {
> - strerror_r(errnum, buf, buflen);
> + msg = strerror_r(errnum, buf, buflen);
> + if (msg != buf) {
> + size_t len = strlen(msg);
> + char *c = mempcpy(buf, msg, min(buflen-1, len));
> + *c = '\0';
> + }
> return 0;
> }
>
> diff --git a/tools/lib/traceevent/event-utils.h b/tools/lib/traceevent/event-utils.h
> index 08296383d1e6..bc075006966e 100644
> --- a/tools/lib/traceevent/event-utils.h
> +++ b/tools/lib/traceevent/event-utils.h
> @@ -39,6 +39,12 @@ void __vdie(const char *fmt, ...);
> void __vwarning(const char *fmt, ...);
> void __vpr_stat(const char *fmt, ...);
>
> +#define min(x, y) ({ \
> + typeof(x) _min1 = (x); \
> + typeof(y) _min2 = (y); \
> + (void) (&_min1 == &_min2); \
> + _min1 < _min2 ? _min1 : _min2; })
> +
> static inline char *strim(char *string)
> {
> char *ret;
> --
> 1.7.11.4
>

--
Kirill A. Shutemov

2012-08-22 18:54:06

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 0/4] tools lib traceevent: Basic error handling

Em Wed, Aug 22, 2012 at 04:24:43PM +0900, Namhyung Kim escreveu:
> Hi Arnaldo,
>
> On Wed, 22 Aug 2012 16:00:27 +0900, Namhyung Kim wrote:
> > I almost forgot that I have some libtraceevent patch left. ;) The
> > first three of them are acked by Steven and the last is a port of a
> > recent perf patch from Kirill that handles a strerror_r build warning.
> > Please consider applying.
>
> I also forgot that I didn't remove the Link: line from the changelogs
> before sending them, sorry. Anyway, you can pull this series from my
> tree which contains libtraceevent/core branch, if you like (I added a
> Link to patch 4 there for your convenience).

This time around I'll get it manually since its a short patchset so as
to get the Acked-by Kirill

- Arnaldo

2012-08-27 16:59:21

by Namhyung Kim

[permalink] [raw]
Subject: [tip:perf/core] tools lib traceevent: Do not link broken field arg for an old ftrace event

Commit-ID: fd34f0b26c9d0f3c3c5c5f83207efa6324cd19f7
Gitweb: http://git.kernel.org/tip/fd34f0b26c9d0f3c3c5c5f83207efa6324cd19f7
Author: Namhyung Kim <[email protected]>
AuthorDate: Wed, 22 Aug 2012 16:00:28 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 22 Aug 2012 16:02:36 -0300

tools lib traceevent: Do not link broken field arg for an old ftrace event

Defer linking a newly allocated arg to print_fmt.args until all of its
field is setup so that later access to ->field.name cannot be NULL.

Signed-off-by: Namhyung Kim <[email protected]>
Acked-by: Steven Rostedt <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/lib/traceevent/event-parse.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index b7c2c49..33fcd94 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -4754,20 +4754,20 @@ int pevent_parse_event(struct pevent *pevent,
struct print_arg *arg, **list;

/* old ftrace had no args */
-
list = &event->print_fmt.args;
for (field = event->format.fields; field; field = field->next) {
arg = alloc_arg();
- *list = arg;
- list = &arg->next;
arg->type = PRINT_FIELD;
arg->field.name = strdup(field->name);
if (!arg->field.name) {
do_warning("failed to allocate field name");
event->flags |= EVENT_FL_FAILED;
+ free_arg(arg);
return -1;
}
arg->field.field = field;
+ *list = arg;
+ list = &arg->next;
}
return 0;
}

2012-08-27 17:00:15

by Namhyung Kim

[permalink] [raw]
Subject: [tip:perf/core] tools lib traceevent: Introduce pevent_errno

Commit-ID: bffddffde7f9bd093909235a25dbb806fe639dde
Gitweb: http://git.kernel.org/tip/bffddffde7f9bd093909235a25dbb806fe639dde
Author: Namhyung Kim <[email protected]>
AuthorDate: Wed, 22 Aug 2012 16:00:29 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 22 Aug 2012 16:02:59 -0300

tools lib traceevent: Introduce pevent_errno

Define and use error numbers for pevent_parse_event() and get rid of
die() and do_warning() calls. If the function returns non-zero value,
the caller can check the return code and do appropriate things.

I chose the error numbers to be negative not to clash with standard
errno, and as usual, 0 for success.

Signed-off-by: Namhyung Kim <[email protected]>
Acked-by: Steven Rostedt <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/lib/traceevent/event-parse.c | 50 +++++++++++++++++++++--------------
tools/lib/traceevent/event-parse.h | 26 +++++++++++++++++-
2 files changed, 54 insertions(+), 22 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 33fcd94..a46cae7 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -4686,9 +4686,8 @@ static int find_event_handle(struct pevent *pevent, struct event_format *event)
*
* /sys/kernel/debug/tracing/events/.../.../format
*/
-int pevent_parse_event(struct pevent *pevent,
- const char *buf, unsigned long size,
- const char *sys)
+enum pevent_errno pevent_parse_event(struct pevent *pevent, const char *buf,
+ unsigned long size, const char *sys)
{
struct event_format *event;
int ret;
@@ -4697,17 +4696,16 @@ int pevent_parse_event(struct pevent *pevent,

event = alloc_event();
if (!event)
- return -ENOMEM;
+ return PEVENT_ERRNO__MEM_ALLOC_FAILED;

event->name = event_read_name();
if (!event->name) {
/* Bad event? */
- free(event);
- return -1;
+ ret = PEVENT_ERRNO__MEM_ALLOC_FAILED;
+ goto event_alloc_failed;
}

if (strcmp(sys, "ftrace") == 0) {
-
event->flags |= EVENT_FL_ISFTRACE;

if (strcmp(event->name, "bprint") == 0)
@@ -4715,20 +4713,28 @@ int pevent_parse_event(struct pevent *pevent,
}

event->id = event_read_id();
- if (event->id < 0)
- die("failed to read event id");
+ if (event->id < 0) {
+ ret = PEVENT_ERRNO__READ_ID_FAILED;
+ /*
+ * This isn't an allocation error actually.
+ * But as the ID is critical, just bail out.
+ */
+ goto event_alloc_failed;
+ }

event->system = strdup(sys);
- if (!event->system)
- die("failed to allocate system");
+ if (!event->system) {
+ ret = PEVENT_ERRNO__MEM_ALLOC_FAILED;
+ goto event_alloc_failed;
+ }

/* Add pevent to event so that it can be referenced */
event->pevent = pevent;

ret = event_read_format(event);
if (ret < 0) {
- do_warning("failed to read event format for %s", event->name);
- goto event_failed;
+ ret = PEVENT_ERRNO__READ_FORMAT_FAILED;
+ goto event_parse_failed;
}

/*
@@ -4740,10 +4746,9 @@ int pevent_parse_event(struct pevent *pevent,

ret = event_read_print(event);
if (ret < 0) {
- do_warning("failed to read event print fmt for %s",
- event->name);
show_warning = 1;
- goto event_failed;
+ ret = PEVENT_ERRNO__READ_PRINT_FAILED;
+ goto event_parse_failed;
}
show_warning = 1;

@@ -4760,10 +4765,9 @@ int pevent_parse_event(struct pevent *pevent,
arg->type = PRINT_FIELD;
arg->field.name = strdup(field->name);
if (!arg->field.name) {
- do_warning("failed to allocate field name");
event->flags |= EVENT_FL_FAILED;
free_arg(arg);
- return -1;
+ return PEVENT_ERRNO__OLD_FTRACE_ARG_FAILED;
}
arg->field.field = field;
*list = arg;
@@ -4778,11 +4782,17 @@ int pevent_parse_event(struct pevent *pevent,

return 0;

- event_failed:
+ event_parse_failed:
event->flags |= EVENT_FL_FAILED;
/* still add it even if it failed */
add_event(pevent, event);
- return -1;
+ return ret;
+
+ event_alloc_failed:
+ free(event->system);
+ free(event->name);
+ free(event);
+ return ret;
}

int get_field_val(struct trace_seq *s, struct format_field *field,
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index 5772ad8..3c48229 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -345,6 +345,28 @@ enum pevent_flag {
PEVENT_NSEC_OUTPUT = 1, /* output in NSECS */
};

+enum pevent_errno {
+ PEVENT_ERRNO__SUCCESS = 0,
+
+ /*
+ * Choose an arbitrary negative big number not to clash with standard
+ * errno since SUS requires the errno has distinct positive values.
+ * See 'Issue 6' in the link below.
+ *
+ * http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html
+ */
+ __PEVENT_ERRNO__START = -100000,
+
+ PEVENT_ERRNO__MEM_ALLOC_FAILED = __PEVENT_ERRNO__START,
+ PEVENT_ERRNO__PARSE_EVENT_FAILED,
+ PEVENT_ERRNO__READ_ID_FAILED,
+ PEVENT_ERRNO__READ_FORMAT_FAILED,
+ PEVENT_ERRNO__READ_PRINT_FAILED,
+ PEVENT_ERRNO__OLD_FTRACE_ARG_FAILED,
+
+ __PEVENT_ERRNO__END,
+};
+
struct cmdline;
struct cmdline_list;
struct func_map;
@@ -509,8 +531,8 @@ void pevent_print_event(struct pevent *pevent, struct trace_seq *s,
int pevent_parse_header_page(struct pevent *pevent, char *buf, unsigned long size,
int long_size);

-int pevent_parse_event(struct pevent *pevent, const char *buf,
- unsigned long size, const char *sys);
+enum pevent_errno pevent_parse_event(struct pevent *pevent, const char *buf,
+ unsigned long size, const char *sys);

void *pevent_get_field_raw(struct trace_seq *s, struct event_format *event,
const char *name, struct pevent_record *record,

2012-08-27 17:01:08

by Namhyung Kim

[permalink] [raw]
Subject: [tip:perf/core] tools lib traceevent: Introduce pevent_strerror

Commit-ID: 2f197b9d7eeaa723a80243610956fe4a17e7b5a4
Gitweb: http://git.kernel.org/tip/2f197b9d7eeaa723a80243610956fe4a17e7b5a4
Author: Namhyung Kim <[email protected]>
AuthorDate: Wed, 22 Aug 2012 16:00:30 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 22 Aug 2012 16:03:39 -0300

tools lib traceevent: Introduce pevent_strerror

The pevent_strerror() sets @buf to a string that describes the
(libtraceevent-specific) error condition that is passed via @errnum.

This is similar to strerror_r() and does same thing if @errnum has a
standard errno value.

To sync error string with its code, define PEVENT_ERRORS with _PE()
macro and use it as suggested by Steven.

Signed-off-by: Namhyung Kim <[email protected]>
Acked-by: Steven Rostedt <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/lib/traceevent/event-parse.c | 43 ++++++++++++++++++++++++++++++++++++
tools/lib/traceevent/event-parse.h | 20 +++++++++++-----
2 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index a46cae7..1373e4c 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -4795,6 +4795,49 @@ enum pevent_errno pevent_parse_event(struct pevent *pevent, const char *buf,
return ret;
}

+#undef _PE
+#define _PE(code, str) str
+static const char * const pevent_error_str[] = {
+ PEVENT_ERRORS
+};
+#undef _PE
+
+int pevent_strerror(struct pevent *pevent, enum pevent_errno errnum,
+ char *buf, size_t buflen)
+{
+ int idx;
+ const char *msg;
+
+ if (errnum >= 0) {
+ strerror_r(errnum, buf, buflen);
+ return 0;
+ }
+
+ if (errnum <= __PEVENT_ERRNO__START ||
+ errnum >= __PEVENT_ERRNO__END)
+ return -1;
+
+ idx = errnum - __PEVENT_ERRNO__START;
+ msg = pevent_error_str[idx];
+
+ switch (errnum) {
+ case PEVENT_ERRNO__MEM_ALLOC_FAILED:
+ case PEVENT_ERRNO__PARSE_EVENT_FAILED:
+ case PEVENT_ERRNO__READ_ID_FAILED:
+ case PEVENT_ERRNO__READ_FORMAT_FAILED:
+ case PEVENT_ERRNO__READ_PRINT_FAILED:
+ case PEVENT_ERRNO__OLD_FTRACE_ARG_FAILED:
+ snprintf(buf, buflen, "%s", msg);
+ break;
+
+ default:
+ /* cannot reach here */
+ break;
+ }
+
+ return 0;
+}
+
int get_field_val(struct trace_seq *s, struct format_field *field,
const char *name, struct pevent_record *record,
unsigned long long *val, int err)
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index 3c48229..527df03 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -345,6 +345,16 @@ enum pevent_flag {
PEVENT_NSEC_OUTPUT = 1, /* output in NSECS */
};

+#define PEVENT_ERRORS \
+ _PE(MEM_ALLOC_FAILED, "failed to allocate memory"), \
+ _PE(PARSE_EVENT_FAILED, "failed to parse event"), \
+ _PE(READ_ID_FAILED, "failed to read event id"), \
+ _PE(READ_FORMAT_FAILED, "failed to read event format"), \
+ _PE(READ_PRINT_FAILED, "failed to read event print fmt"), \
+ _PE(OLD_FTRACE_ARG_FAILED,"failed to allocate field name for ftrace")
+
+#undef _PE
+#define _PE(__code, __str) PEVENT_ERRNO__ ## __code
enum pevent_errno {
PEVENT_ERRNO__SUCCESS = 0,

@@ -357,15 +367,11 @@ enum pevent_errno {
*/
__PEVENT_ERRNO__START = -100000,

- PEVENT_ERRNO__MEM_ALLOC_FAILED = __PEVENT_ERRNO__START,
- PEVENT_ERRNO__PARSE_EVENT_FAILED,
- PEVENT_ERRNO__READ_ID_FAILED,
- PEVENT_ERRNO__READ_FORMAT_FAILED,
- PEVENT_ERRNO__READ_PRINT_FAILED,
- PEVENT_ERRNO__OLD_FTRACE_ARG_FAILED,
+ PEVENT_ERRORS,

__PEVENT_ERRNO__END,
};
+#undef _PE

struct cmdline;
struct cmdline_list;
@@ -583,6 +589,8 @@ int pevent_data_pid(struct pevent *pevent, struct pevent_record *rec);
const char *pevent_data_comm_from_pid(struct pevent *pevent, int pid);
void pevent_event_info(struct trace_seq *s, struct event_format *event,
struct pevent_record *record);
+int pevent_strerror(struct pevent *pevent, enum pevent_errno errnum,
+ char *buf, size_t buflen);

struct event_format **pevent_list_events(struct pevent *pevent, enum event_sort_type);
struct format_field **pevent_event_common_fields(struct event_format *event);

2012-08-27 17:02:09

by Namhyung Kim

[permalink] [raw]
Subject: [tip:perf/core] tools lib traceevent: Fix strerror_r() use in pevent_strerror

Commit-ID: e1aa7c30c599e99b4544f9e5b4c275c8a5325bdc
Gitweb: http://git.kernel.org/tip/e1aa7c30c599e99b4544f9e5b4c275c8a5325bdc
Author: Namhyung Kim <[email protected]>
AuthorDate: Wed, 22 Aug 2012 16:00:31 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 22 Aug 2012 16:04:05 -0300

tools lib traceevent: Fix strerror_r() use in pevent_strerror

glibc-2.16 starts to mark the function with attribute warn_unused_result
so that it can cause a build warning.

Since GNU version of strerror_r() can return a pointer to a string
without setting @buf, check the return value and copy/truncate it to our
buffer if needed.

Signed-off-by: Namhyung Kim <[email protected]>
Acked-by: Kirill A. Shutemov <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Kirill A. Shutemov <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/lib/traceevent/event-parse.c | 7 ++++++-
tools/lib/traceevent/event-utils.h | 6 ++++++
2 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 1373e4c..f978c59 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -4809,7 +4809,12 @@ int pevent_strerror(struct pevent *pevent, enum pevent_errno errnum,
const char *msg;

if (errnum >= 0) {
- strerror_r(errnum, buf, buflen);
+ msg = strerror_r(errnum, buf, buflen);
+ if (msg != buf) {
+ size_t len = strlen(msg);
+ char *c = mempcpy(buf, msg, min(buflen-1, len));
+ *c = '\0';
+ }
return 0;
}

diff --git a/tools/lib/traceevent/event-utils.h b/tools/lib/traceevent/event-utils.h
index 0829638..bc07500 100644
--- a/tools/lib/traceevent/event-utils.h
+++ b/tools/lib/traceevent/event-utils.h
@@ -39,6 +39,12 @@ void __vdie(const char *fmt, ...);
void __vwarning(const char *fmt, ...);
void __vpr_stat(const char *fmt, ...);

+#define min(x, y) ({ \
+ typeof(x) _min1 = (x); \
+ typeof(y) _min2 = (y); \
+ (void) (&_min1 == &_min2); \
+ _min1 < _min2 ? _min1 : _min2; })
+
static inline char *strim(char *string)
{
char *ret;