parse-events: Make trace_seq dynamically expand its buffers

The trace_seq code that is used to copy data into a buffer to later
be able to print was originally taken from the Linux kernel. But the
constraints on the kernel are different than what they are in userspace.
Since the kernel uses the trace_seq in critical paths, it can not
allocate memory during use, so it used a static page buffer.

But in userspace, we do not have this constraint, and are allowed to
realloc the buffer when needed. There's no reason to fill the buffer
when we hit the buffer size, we can simply allocate more memory.

The downside to this is that we must call a destructor on the trace_seq
structure. All trace_seq's must be initialized with trace_seq_init(),
so this patch matched all trace_seq_init()'s with the new function
trace_seq_destroy().

Suggested-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

7 files changed, 103 insertions, 41 deletions

diff --git a/parse-events.c b/parse-events.c
index e5fe89a..1a3fd8d 100644
--- a/parse-events.c
+++ b/parse-events.c
@@ -2887,7 +2887,9 @@ eval_num_arg(void *data, int size, struct event_format *event, struct print_arg
         case PRINT_FUNC: {
                 struct trace_seq s;
                 trace_seq_init(&s);
-                return process_defined_func(&s, data, size, event, arg);
+                val = process_defined_func(&s, data, size, event, arg);
+                trace_seq_destroy(&s);
+                return val;
         }
         case PRINT_OP:
                 if (strcmp(arg->op.op, "[") == 0) {
@@ -3220,6 +3222,7 @@ process_defined_func(struct trace_seq *s, void *data, int size,
                         string->next = strings;
                         string->str = strdup(str.buffer);
                         strings = string;
+                        trace_seq_destroy(&str);
                         break;
                 default:
                         /*
@@ -4097,6 +4100,7 @@ static void print_args(struct print_arg *args)
                 trace_seq_init(&s);
                 print_fields(&s, args->flags.flags);
                 trace_seq_do_printf(&s);
+                trace_seq_destroy(&s);
                 printf(")");
                 break;
         case PRINT_SYMBOL:
@@ -4106,6 +4110,7 @@ static void print_args(struct print_arg *args)
                 trace_seq_init(&s);
                 print_fields(&s, args->symbol.symbols);
                 trace_seq_do_printf(&s);
+                trace_seq_destroy(&s);
                 printf(")");
                 break;
         case PRINT_STRING:
diff --git a/parse-events.h b/parse-events.h
index 98eb76c..efbf8d4 100644
--- a/parse-events.h
+++ b/parse-events.h
@@ -31,8 +31,8 @@
 /* ----------------------- trace_seq ----------------------- */
 
 
-#ifndef TRACE_SEQ_SIZE
-#define TRACE_SEQ_SIZE 4096
+#ifndef TRACE_SEQ_BUF_SIZE
+#define TRACE_SEQ_BUF_SIZE 4096
 #endif
 
 struct record {
@@ -54,19 +54,14 @@ struct record {
  */
 
 struct trace_seq {
-        char                    buffer[TRACE_SEQ_SIZE];
+        char                    *buffer;
+        unsigned int            buffer_size;
         unsigned int            len;
         unsigned int            readpos;
-        int                     full;
 };
 
-static inline void
-trace_seq_init(struct trace_seq *s)
-{
-        s->len = 0;
-        s->readpos = 0;
-        s->full = 0;
-}
+void trace_seq_init(struct trace_seq *s);
+void trace_seq_destroy(struct trace_seq *s);
 
 extern int trace_seq_printf(struct trace_seq *s, const char *fmt, ...)
         __attribute__ ((format (printf, 2, 3)));
diff --git a/trace-cmd.c b/trace-cmd.c
index b4eb0fe..6cfdfdc 100644
--- a/trace-cmd.c
+++ b/trace-cmd.c
@@ -1713,6 +1713,7 @@ int main (int argc, char **argv)
                 trace_seq_printf(&s, "CPU: %d\n", cpu);
                 tracecmd_stat_cpu(&s, cpu);
                 trace_seq_do_printf(&s);
+                trace_seq_destroy(&s);
                 printf("\n");
         }
 
diff --git a/trace-graph.c b/trace-graph.c
index a53ab84..44c1273 100644
--- a/trace-graph.c
+++ b/trace-graph.c
@@ -326,6 +326,7 @@ static void update_label_time(GtkWidget *label, gint64 time)
         trace_seq_printf(&s, "%s%lu.%06lu", min, sec, usec);
 
         gtk_label_set_text(GTK_LABEL(label), s.buffer);
+        trace_seq_destroy(&s);
 }
 
 static void update_cursor(struct graph_info *ginfo)
@@ -1225,6 +1226,7 @@ static void draw_plot_info(struct graph_info *ginfo, struct graph_plot *plot,
 
         if (!trace_graph_plot_display_info(ginfo, plot, &s, time)) {
                 /* Just display the current time */
+                trace_seq_destroy(&s);
                 trace_seq_init(&s);
                 trace_seq_printf(&s, "%lu.%06lu", sec, usec);
         }
@@ -1232,6 +1234,7 @@ static void draw_plot_info(struct graph_info *ginfo, struct graph_plot *plot,
         trace_seq_putc(&s, 0);
 
         draw_info_box(ginfo, s.buffer, x, y);
+        trace_seq_destroy(&s);
 }
 
 static void draw_latency(struct graph_info *ginfo, gint x, gint y)
@@ -1261,6 +1264,7 @@ static void draw_latency(struct graph_info *ginfo, gint x, gint y)
         trace_seq_printf(&s, "Diff: %s%ld.%06lu secs", neg ? "-":"", sec, usec);
 
         draw_info_box(ginfo, s.buffer, x, y);
+        trace_seq_destroy(&s);
 }
 
 static gboolean
@@ -1662,6 +1666,7 @@ static gint draw_event_label(struct graph_info *ginfo, gint i,
          */
         ret = trace_graph_plot_display_last_event(ginfo, plot, &s,
                                                   convert_x_to_time(ginfo, p2-1));
+        trace_seq_destroy(&s);
         if (!ret)
                 return p2;
 
@@ -1935,6 +1940,7 @@ static void draw_timeline(struct graph_info *ginfo, gint width)
         gdk_draw_layout(ginfo->curr_pixmap, ginfo->draw->style->black_gc,
                         1, height+10, layout);
         g_object_unref(layout);
+        trace_seq_destroy(&s);
 
 
         /* --- draw ending time --- */
@@ -1948,6 +1954,7 @@ static void draw_timeline(struct graph_info *ginfo, gint width)
         gdk_draw_layout(ginfo->curr_pixmap, ginfo->draw->style->black_gc,
                         width - (w + 2), height+10, layout);
         g_object_unref(layout);
+        trace_seq_destroy(&s);
 
 
         /* --- draw time at intervals --- */
@@ -1960,6 +1967,7 @@ static void draw_timeline(struct graph_info *ginfo, gint width)
                 convert_nano(time, &sec, &usec);
                 trace_seq_init(&s);
                 trace_seq_printf(&s, "%lu.%06lu", sec, usec);
+                trace_seq_destroy(&s);
 
                 gdk_draw_line(ginfo->curr_pixmap, ginfo->draw->style->black_gc,
                               mid, height, mid, height + 5);
diff --git a/trace-read.c b/trace-read.c
index f09dcf8..f8455a3 100644
--- a/trace-read.c
+++ b/trace-read.c
@@ -108,6 +108,7 @@ static void show_test(struct tracecmd_input *handle)
         trace_seq_init(&s);
         pevent_print_event(pevent, &s, cpu, record->data, record->size, record->ts);
         trace_seq_do_printf(&s);
+        trace_seq_destroy(&s);
         printf("\n");
 
         free_record(record);
@@ -154,6 +155,7 @@ static void show_test(struct tracecmd_input *handle)
         trace_seq_init(&s);
         pevent_print_event(pevent, &s, cpu, record->data, record->size, record->ts);
         trace_seq_do_printf(&s);
+        trace_seq_destroy(&s);
         printf("\n");
 
         free_record(record);
@@ -194,6 +196,7 @@ static void show_test(struct tracecmd_input *handle)
         trace_seq_init(&s);
         pevent_print_event(pevent, &s, cpu, record->data, record->size, record->ts);
         trace_seq_do_printf(&s);
+        trace_seq_destroy(&s);
         printf("\n");
 
         free_record(record);
@@ -209,6 +212,7 @@ static void show_test(struct tracecmd_input *handle)
         trace_seq_init(&s);
         pevent_print_event(pevent, &s, cpu, record->data, record->size, record->ts);
         trace_seq_do_printf(&s);
+        trace_seq_destroy(&s);
         printf("\n");
 
         free_record(record);
@@ -473,6 +477,7 @@ static void show_data(struct tracecmd_input *handle,
                                  record->cpu);
         pevent_print_event(pevent, &s, record);
         trace_seq_do_printf(&s);
+        trace_seq_destroy(&s);
 
         process_wakeup(pevent, record);
 
diff --git a/trace-seq.c b/trace-seq.c
index d84a5bd..da5eae9 100644
--- a/trace-seq.c
+++ b/trace-seq.c
@@ -25,6 +25,52 @@
 
 #include "parse-events.h"
 
+/*
+ * The TRACE_SEQ_POISON is to catch the use of using
+ * a trace_seq structure after it was destroyed.
+ */
+#define TRACE_SEQ_POISON        ((void *)0xdeadbeef)
+#define TRACE_SEQ_CHECK(s)                                              \
+do {                                                                    \
+        if ((s)->buffer == TRACE_SEQ_POISON)                    \
+                die("Usage of trace_seq after it was destroyed");       \
+} while (0)
+
+/**
+ * trace_seq_init - initialize the trace_seq structure
+ * @s: a pointer to the trace_seq structure to initialize
+ */
+void trace_seq_init(struct trace_seq *s)
+{
+        s->len = 0;
+        s->readpos = 0;
+        s->buffer_size = TRACE_SEQ_BUF_SIZE;
+        s->buffer = malloc_or_die(s->buffer_size);
+}
+
+/**
+ * trace_seq_destroy - free up memory of a trace_seq
+ * @s: a pointer to the trace_seq to free the buffer
+ *
+ * Only frees the buffer, not the trace_seq struct itself.
+ */
+void trace_seq_destroy(struct trace_seq *s)
+{
+        if (!s)
+                return;
+        TRACE_SEQ_CHECK(s);
+        free(s->buffer);
+        s->buffer = TRACE_SEQ_POISON;
+}
+
+static void expand_buffer(struct trace_seq *s)
+{
+        s->buffer_size += TRACE_SEQ_BUF_SIZE;
+        s->buffer = realloc(s->buffer, s->buffer_size);
+        if (!s->buffer)
+                die("Can't allocate trace_seq buffer memory");
+}
+
 /**
  * trace_seq_printf - sequence printing of trace information
  * @s: trace sequence descriptor
@@ -42,21 +88,22 @@
 int
 trace_seq_printf(struct trace_seq *s, const char *fmt, ...)
 {
-        int len = (TRACE_SEQ_SIZE - 1) - s->len;
         va_list ap;
+        int len;
         int ret;
 
-        if (s->full || !len)
-                return 0;
+        TRACE_SEQ_CHECK(s);
+
+ try_again:
+        len = (s->buffer_size - 1) - s->len;
 
         va_start(ap, fmt);
         ret = vsnprintf(s->buffer + s->len, len, fmt, ap);
         va_end(ap);
 
-        /* If we can't write it all, don't bother writing anything */
         if (ret >= len) {
-                s->full = 1;
-                return 0;
+                expand_buffer(s);
+                goto try_again;
         }
 
         s->len += ret;
@@ -78,18 +125,19 @@ trace_seq_printf(struct trace_seq *s, const char *fmt, ...)
 int
 trace_seq_vprintf(struct trace_seq *s, const char *fmt, va_list args)
 {
-        int len = (TRACE_SEQ_SIZE - 1) - s->len;
+        int len;
         int ret;
 
-        if (s->full || !len)
-                return 0;
+        TRACE_SEQ_CHECK(s);
+
+ try_again:
+        len = (s->buffer_size - 1) - s->len;
 
         ret = vsnprintf(s->buffer + s->len, len, fmt, args);
 
-        /* If we can't write it all, don't bother writing anything */
         if (ret >= len) {
-                s->full = 1;
-                return 0;
+                expand_buffer(s);
+                goto try_again;
         }
 
         s->len += ret;
@@ -109,15 +157,14 @@ trace_seq_vprintf(struct trace_seq *s, const char *fmt, va_list args)
  */
 int trace_seq_puts(struct trace_seq *s, const char *str)
 {
-        int len = strlen(str);
+        int len;
 
-        if (s->full)
-                return 0;
+        TRACE_SEQ_CHECK(s);
 
-        if (len > ((TRACE_SEQ_SIZE - 1) - s->len)) {
-                s->full = 1;
-                return 0;
-        }
+        len = strlen(str);
+
+        while (len > ((s->buffer_size - 1) - s->len))
+                expand_buffer(s);
 
         memcpy(s->buffer + s->len, str, len);
         s->len += len;
@@ -127,13 +174,10 @@ int trace_seq_puts(struct trace_seq *s, const char *str)
 
 int trace_seq_putc(struct trace_seq *s, unsigned char c)
 {
-        if (s->full)
-                return 0;
+        TRACE_SEQ_CHECK(s);
 
-        if (s->len >= (TRACE_SEQ_SIZE - 1)) {
-                s->full = 1;
-                return 0;
-        }
+        while (s->len >= (s->buffer_size - 1))
+                expand_buffer(s);
 
         s->buffer[s->len++] = c;
 
@@ -142,12 +186,14 @@ int trace_seq_putc(struct trace_seq *s, unsigned char c)
 
 void trace_seq_terminate(struct trace_seq *s)
 {
-        if (!s->full)
-                s->buffer[s->len] = 0;
+        TRACE_SEQ_CHECK(s);
+
+        /* There's always one character left on the buffer */
+        s->buffer[s->len] = 0;
 }
 
 int trace_seq_do_printf(struct trace_seq *s)
 {
-        return printf("%.*s%s", s->len, s->buffer,
-                        s->full ? "[truncated]" : "");
+        TRACE_SEQ_CHECK(s);
+        return printf("%.*s", s->len, s->buffer);
 }
diff --git a/trace-view-store.c b/trace-view-store.c
index 6f70ca5..0834df7 100644
--- a/trace-view-store.c
+++ b/trace-view-store.c
@@ -500,6 +500,7 @@ trace_view_store_get_value (GtkTreeModel *tree_model,
                         trace_seq_init(&s);
                         pevent_data_lat_fmt(pevent, &s, data);
                         g_value_set_string(value, s.buffer);
+                        trace_seq_destroy(&s);
                         break;
 
                 case TRACE_VIEW_STORE_COL_EVENT:
@@ -515,6 +516,7 @@ trace_view_store_get_value (GtkTreeModel *tree_model,
                         trace_seq_init(&s);
                         pevent_event_info(&s, event, data);
                         g_value_set_string(value, s.buffer);
+                        trace_seq_destroy(&s);
                         break;
                 }
                 free_record(data);