Simplify vdso reading in binfile.c
authorSøren Sandmann Pedersen <ssp@redhat.com>
Tue, 8 Sep 2009 04:00:06 +0000 (00:00 -0400)
committerSøren Sandmann Pedersen <ssp@redhat.com>
Tue, 8 Sep 2009 07:03:17 +0000 (03:03 -0400)
TODO
binfile.c

diff --git a/TODO b/TODO
index 8a5c6be..b5da1c9 100644 (file)
--- 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.
index 510e216..ebfba03 100644 (file)
--- 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)
-    {
-       g_free (name);
-       return NULL;
-    }
-
-    result = g_array_new (FALSE, FALSE, sizeof (Map));
-    
-    while (fgets (buffer, sizeof (buffer) - 1, in))
+    if (g_file_get_contents (filename, &contents, NULL, NULL))
     {
-       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);
-}
-
-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;
+    
+    g_free (filename);
+    
+    return result;
 }
 
-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;
-
-       maps = read_maps (getpid(), &n_maps);
+       char **lines = get_lines ("/proc/%d/maps", getpid());
+       int i;
 
-       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;
 }