On 12/26/13, 12:38 AM, Namhyung Kim wrote:
diff --git a/tools/perf/util/log.c b/tools/perf/util/log.c
new file mode 100644
index 000000000000..3838d49f82de
--- /dev/null
+++ b/tools/perf/util/log.c
@@ -0,0 +1,105 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include "util/debug.h"
+
+#define LINEMAP_GROW 128
+
+struct perf_log perf_log = {
+ .seen_newline = true,
+};
+
+int perf_log_init(void)
Why return int if the rc is not checked? Failure here is not going to
stop the perf command right?
+{
+ FILE *fp;
+ char name[] = "/tmp/perf-log-XXXXXX";
+ int fd = mkstemp(name);
+
+ if (fd < 0)
+ return -1;
+
+ fp = fdopen(fd, "r+");
+ if (fp == NULL) {
+ close(fd);
+ return -1;
+ }
+
+ perf_log.fp = fp;
Add 'unlink(name);' here to ensure the file is removed regardless of how
perf terminates.
+
+ return 0;
+}
+
+int perf_log_exit(void)
+{
+ FILE *fp = perf_log.fp;
+ if (fp)
+ fclose(fp);
+
+ free(perf_log.linemap);
+
+ perf_log.fp = NULL;
+ perf_log.linemap = NULL;
+ return 0;
+}
+
+static int grow_linemap(struct perf_log *log)
+{
+ off_t *newmap;
+ int newsize = log->nr_alloc + LINEMAP_GROW;
+
+ newmap = realloc(log->linemap, newsize * sizeof(*log->linemap));
+ if (newmap == NULL)
+ return -1;
+
+ log->nr_alloc = newsize;
+ log->linemap = newmap;
+ return 0;
+}
What's the point of linemap?
+
+static int __add_to_linemap(struct perf_log *log, off_t idx)
+{
+ if (log->lines == log->nr_alloc)
+ if (grow_linemap(log) < 0)
+ return -1;
+
+ log->linemap[log->lines++] = idx;
+ return 0;
+}
+
+static void add_to_linemap(struct perf_log *log, const char *msg, off_t base)
+{
+ const char *pos;
+
+ if (strlen(msg) == 0)
+ return;
+
+ if (log->seen_newline) {
+ if (__add_to_linemap(log, base) < 0)
+ return;
+ }
+
+ if ((pos = strchr(msg, '\n')) != NULL) {
+ log->seen_newline = true;
+ pos++;
+ add_to_linemap(log, pos, base + (pos - msg));
+ } else {
+ log->seen_newline = false;
+ }
+}
+
+void perf_log_add(const char *msg)
+{
+ FILE *fp = perf_log.fp;
Don't assume every user of libperf calls perf_log_init() or that the
file was actually created. i.e., add 'if (fp == NULL) return;'
+ off_t offset = ftello(fp);
+
+ add_to_linemap(&perf_log, msg, offset);
+
+ fwrite(msg, 1, strlen(msg), fp);
And if write fails?
+}
+
+void perf_log_addv(const char *fmt, va_list ap)
+{
+ char buf[4096];
Add as an optimization add the fp != NULL check here too. Don't need to
do the vsnprintf only to drop it.
+
+ vsnprintf(buf, sizeof(buf), fmt, ap);
+ perf_log_add(buf);
+}
What limits the size of the file - other than the obvious out of space
in /tmp? Allow the file to grow without bounds in case a user wants the
messages seems dangerous.
What about using a circular buffer instead?
David
--
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/
