From 604d07600a8fd42c4f9bbf42bd6262bfb61bfcb4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=B8ren=20Sandmann=20Pedersen?= <ssp@redhat.com>
Date: Tue, 8 Sep 2009 00:00:06 -0400
Subject: [PATCH] Simplify vdso reading in binfile.c

---
 TODO      |   6 +++
 binfile.c | 147 ++++++++++++------------------------------------------
 2 files changed, 39 insertions(+), 114 deletions(-)

diff --git a/TODO b/TODO
index 8a5c6be3..b5da1c91 100644
--- a/TODO
+++ b/TODO
@@ -23,6 +23,10 @@ Before 1.0.4:
 
 Before 1.2:
 
+* Make tracker creation faster 
+
+* Share map reading code between vdso stuff in binfile.c and tracker.c
+
 * Get rid of remaining gulongs (use uint64_t instead)
 
 * Move binfile hash table to state_t.
@@ -580,6 +584,8 @@ http://www.linuxbase.org/spec/booksets/LSB-Embedded/LSB-Embedded/ehframe.html
 
 Later:
 
+- Multithreading is possible in a number of places.
+
 - If the stack trace ends in a memory access instruction, send the
   vma information to userspace. Then have user space
   produce statistics on what types of memory are accessed.
diff --git a/binfile.c b/binfile.c
index 510e216c..ebfba033 100644
--- a/binfile.c
+++ b/binfile.c
@@ -33,6 +33,7 @@
 #include <stdio.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <stdint.h>
 
 #include "binfile.h"
 #include "elfparser.h"
@@ -260,130 +261,49 @@ get_debug_binaries (GList      *files,
 
 static GHashTable *bin_files;
 
-typedef struct Map Map;
-struct Map
+static char **
+get_lines (const char *format, pid_t pid)
 {
-    char *	filename;
-    gulong	start;
-    gulong	end;
-    gulong      offset;
-    gulong	inode;
+    char *filename = g_strdup_printf (format, pid);
+    char **result = NULL;
+    char *contents;
     
-    BinFile *	bin_file;
-};
-
-static Map *
-read_maps (int pid, int *n_maps)
-{
-    char *name = g_strdup_printf ("/proc/%d/maps", pid);
-    char buffer[1024];
-    FILE *in;
-    GArray *result;
-
-    in = fopen (name, "r");
-    if (!in)
+    if (g_file_get_contents (filename, &contents, NULL, NULL))
     {
-	g_free (name);
-	return NULL;
-    }
-
-    result = g_array_new (FALSE, FALSE, sizeof (Map));
-    
-    while (fgets (buffer, sizeof (buffer) - 1, in))
-    {
-	char file[256];
-	int count;
-	gulong start;
-	gulong end;
-	gulong offset;
-	gulong inode;
+        result = g_strsplit (contents, "\n", -1);
 	
-	count = sscanf (
-	    buffer, "%lx-%lx %*15s %lx %*x:%*x %lu %255s", 
-	    &start, &end, &offset, &inode, file);
-
-	if (count == 5)
-	{
-	    Map map;
-	    
-	    map.filename = g_strdup (file);
-	    map.start = start;
-	    map.end = end;
-	    
-	    if (strcmp (map.filename, "[vdso]") == 0)
-	    {
-		/* For the vdso, the kernel reports 'offset' as the
-		 * the same as the mapping addres. This doesn't make
-		 * any sense to me, so we just zero it here. There
-		 * is code in binfile.c (read_inode) that returns 0
-		 * for [vdso].
-		 */
-		map.offset = 0;
-		map.inode = 0;
-	    }
-	    else
-	    {
-		map.offset = offset;
-		map.inode = inode;
-	    }
-
-	    map.bin_file = NULL;
-
-	    g_array_append_val (result, map);
-	}
+        g_free (contents);
     }
-
-    g_free (name);
-    fclose (in);
-
-    if (n_maps)
-	*n_maps = result->len;
-
-    return (Map *)g_array_free (result, FALSE);
+    
+    g_free (filename);
+    
+    return result;
 }
 
-static void
-free_maps (int *n_maps,
-	   Map *maps)
-{
-    int i;
-
-    for (i = 0; i < *n_maps; ++i)
-    {
-	Map *map = &(maps[i]);
-	
-	if (map->filename)
-	    g_free (map->filename);
-	
-	if (map->bin_file)
-	    bin_file_free (map->bin_file);
-    }
-
-    g_free (maps);
-    *n_maps = 0;
-}
-
-const guint8 *
-get_vdso_bytes (gsize *length)
+static const uint8_t *
+get_vdso_bytes (size_t *length)
 {
+    static const uint8_t *bytes = NULL;
+    static size_t n_bytes = 0;    
     static gboolean has_data;
-    static const guint8 *bytes = NULL;
-    static gsize n_bytes = 0;    
 
     if (!has_data)
     {
-	Map *maps;
-	int n_maps, i;
+	char **lines = get_lines ("/proc/%d/maps", getpid());
+	int i;
 
-	maps = read_maps (getpid(), &n_maps);
-
-	for (i = 0; i < n_maps; ++i)
+	for (i = 0; lines[i] != NULL; ++i)
 	{
-	    Map *map = &(maps[i]);
-
-	    if (strcmp (map->filename, "[vdso]") == 0)
+	    char file[256];
+	    gulong start;
+	    gulong end;
+	    int count = sscanf (
+		lines[i], "%lx-%lx %*15s %*x %*x:%*x %*u %255s", 
+		&start, &end, file);
+	    
+	    if (count == 3 && strcmp (file, "[vdso]") == 0)
 	    {
-		n_bytes = map->end - map->start;
+		n_bytes = end - start;
 
 		/* Dup the memory here so that valgrind will only
 		 * report one 1 byte invalid read instead of
@@ -395,17 +315,16 @@ get_vdso_bytes (gsize *length)
 		 * wrapper never returned that address. But since it
 		 * is a legal mapping, it is legal to read it.
 		 */
-		bytes = g_memdup ((guint8 *)map->start, n_bytes);
+		bytes = g_memdup ((uint8_t *)start, n_bytes);
+
+		has_data = TRUE;
 	    }
 	}
-	
-	has_data = TRUE;
-	free_maps (&n_maps, maps);
     }
 
     if (length)
 	*length = n_bytes;
-
+    
     return bytes;
 }