Re: [PATCH 27/30] tools lib traceevent: Carve out events format parsing routine

2012-09-25 Thread Arnaldo Carvalho de Melo 
Em Tue, Sep 25, 2012 at 01:15:05PM +0900, Namhyung Kim escreveu:
> On Mon, 24 Sep 2012 12:59:41 -0300, Arnaldo Carvalho de Melo wrote:
> > +   if (add_event(pevent, event))
> > +   goto event_add_failed;
 
> It seems we should set the 'ret' to a proper pevent_errno -
> PEVENT_ERRNO__MEM_ALLOC_FAILED.
 
> > +event_add_failed:
> > +   free(event->system);
> > +   free(event->name);
> > +   free(event);
> 
> At this point, the 'event' also has fields and format information and
> they all need to be freed.  Looks like calling pevent_free_format()
> would be the right thing IMHO.

Right, care to send a patch?

This patch even exported pevent_free_format, used in the next patch in
this series, should've used it here :-\

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 27/30] tools lib traceevent: Carve out events format parsing routine

2012-09-24 Thread Namhyung Kim 
On Mon, 24 Sep 2012 12:59:41 -0300, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo 
>
> The pevent_parse_event() routine will parse a events/sys/tp/format file
> and add an event_format instance to the pevent struct.
>
> This patch introduces a pevent_parse_format() routine with just the bits
> needed to parse the event/sys/tp/format file and just return the
> event_format instance, useful for when all we want is to parse the
> format file, without requiring the pevent struct.
[snip]
> +enum pevent_errno pevent_parse_event(struct pevent *pevent, const char *buf,
> +  unsigned long size, const char *sys)
> +{
> + struct event_format *event = NULL;
> + int ret = __pevent_parse_format(&event, pevent, buf, size, sys);
> +
> + if (event == NULL)
> + return ret;
> +
> + /* Add pevent to event so that it can be referenced */
> + event->pevent = pevent;
> +
> + if (add_event(pevent, event))
> + goto event_add_failed;

It seems we should set the 'ret' to a proper pevent_errno -
PEVENT_ERRNO__MEM_ALLOC_FAILED.


> +
> +#define PRINT_ARGS 0
> + if (PRINT_ARGS && event->print_fmt.args)
> + print_args(event->print_fmt.args);
> +
> + return 0;
> +
> +event_add_failed:
> + free(event->system);
> + free(event->name);
> + free(event);

At this point, the 'event' also has fields and format information and
they all need to be freed.  Looks like calling pevent_free_format()
would be the right thing IMHO.

Thanks,
Namhyung

>   return ret;
>  }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 27/30] tools lib traceevent: Carve out events format parsing routine

2012-09-24 Thread Arnaldo Carvalho de Melo 
From: Arnaldo Carvalho de Melo 

The pevent_parse_event() routine will parse a events/sys/tp/format file
and add an event_format instance to the pevent struct.

This patch introduces a pevent_parse_format() routine with just the bits
needed to parse the event/sys/tp/format file and just return the
event_format instance, useful for when all we want is to parse the
format file, without requiring the pevent struct.

Acked-by: Steven Rostedt 
Cc: David Ahern 
Cc: Frederic Weisbecker 
Cc: Jiri Olsa 
Cc: Mike Galbraith 
Cc: Namhyung Kim 
Cc: Paul Mackerras 
Cc: Peter Zijlstra 
Cc: Stephane Eranian 
Cc: Steven Rostedt 
Link: http://lkml.kernel.org/n/tip-lge0afl47arh86om0m6a5...@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/lib/traceevent/event-parse.c |   96 +++-
 tools/lib/traceevent/event-parse.h |3 +
 2 files changed, 75 insertions(+), 24 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c 
b/tools/lib/traceevent/event-parse.c
index b3bc130..1fa71ca 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -4794,8 +4794,7 @@ static int find_event_handle(struct pevent *pevent, 
struct event_format *event)
 }
 
 /**
- * pevent_parse_event - parse the event format
- * @pevent: the handle to the pevent
+ * __pevent_parse_format - parse the event format
  * @buf: the buffer storing the event format string
  * @size: the size of @buf
  * @sys: the system the event belongs to
@@ -4807,15 +4806,16 @@ static int find_event_handle(struct pevent *pevent, 
struct event_format *event)
  *
  * /sys/kernel/debug/tracing/events/.../.../format
  */
-enum pevent_errno pevent_parse_event(struct pevent *pevent, const char *buf,
-unsigned long size, const char *sys)
+enum pevent_errno __pevent_parse_format(struct event_format **eventp,
+   struct pevent *pevent, const char *buf,
+   unsigned long size, const char *sys)
 {
struct event_format *event;
int ret;
 
init_input_buf(buf, size);
 
-   event = alloc_event();
+   *eventp = event = alloc_event();
if (!event)
return PEVENT_ERRNO__MEM_ALLOC_FAILED;
 
@@ -4849,9 +4849,6 @@ enum pevent_errno pevent_parse_event(struct pevent 
*pevent, const char *buf,
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) {
ret = PEVENT_ERRNO__READ_FORMAT_FAILED;
@@ -4862,19 +4859,16 @@ enum pevent_errno pevent_parse_event(struct pevent 
*pevent, const char *buf,
 * If the event has an override, don't print warnings if the event
 * print format fails to parse.
 */
-   if (find_event_handle(pevent, event))
+   if (pevent && find_event_handle(pevent, event))
show_warning = 0;
 
ret = event_read_print(event);
+   show_warning = 1;
+
if (ret < 0) {
-   show_warning = 1;
ret = PEVENT_ERRNO__READ_PRINT_FAILED;
goto event_parse_failed;
}
-   show_warning = 1;
-
-   if (add_event(pevent, event))
-   goto event_alloc_failed;
 
if (!ret && (event->flags & EVENT_FL_ISFTRACE)) {
struct format_field *field;
@@ -4898,24 +4892,78 @@ enum pevent_errno pevent_parse_event(struct pevent 
*pevent, const char *buf,
return 0;
}
 
-#define PRINT_ARGS 0
-   if (PRINT_ARGS && event->print_fmt.args)
-   print_args(event->print_fmt.args);
-
return 0;
 
  event_parse_failed:
event->flags |= EVENT_FL_FAILED;
-   /* still add it even if it failed */
-   if (add_event(pevent, event))
-   goto event_alloc_failed;
-
return ret;
 
  event_alloc_failed:
free(event->system);
free(event->name);
free(event);
+   *eventp = NULL;
+   return ret;
+}
+
+/**
+ * pevent_parse_format - parse the event format
+ * @buf: the buffer storing the event format string
+ * @size: the size of @buf
+ * @sys: the system the event belongs to
+ *
+ * This parses the event format and creates an event structure
+ * to quickly parse raw data for a given event.
+ *
+ * These files currently come from:
+ *
+ * /sys/kernel/debug/tracing/events/.../.../format
+ */
+enum pevent_errno pevent_parse_format(struct event_format **eventp, const char 
*buf,
+ unsigned long size, const char *sys)
+{
+   return __pevent_parse_format(eventp, NULL, buf, size, sys);
+}
+
+/**
+ * pevent_parse_event - parse the event format
+ * @pevent: the handle to the pevent
+ * @buf: the buffer storing the event format string
+ * @size: the size of @buf
+ * @sys: the system the event belongs to
+ *
+ * This parses the event format and creates an