libsysprof-capture: Add SysprofCaptureJitmapIter to replace GHashTable

Change `sysprof_capture_reader_read_jitmap()` to return a `const
SysprofCaptureJitmap *` (like the other `read` functions), and add a new
`SysprofCaptureJitmapIter` type to allow easy iteration over the jitmap.

This allows a use of `GHashTable` to be removed from the API. It breaks
the libsysprof-capture API and ABI.

All the callers iterate over the jitmap rather than looking up elements
by key. If that functionality is needed in future, additional API can be
added to allow it on `SysprofCaptureJitmap`.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Helps: #40

src/libsysprof-capture/sysprof-capture-reader.c

@@ -758,10 +758,9 @@ sysprof_capture_reader_read_process (SysprofCaptureReader *self)
   return process;
 }
 
-GHashTable *
+const SysprofCaptureJitmap *
 sysprof_capture_reader_read_jitmap (SysprofCaptureReader *self)
 {
-  g_autoptr(GHashTable) ret = NULL;
   SysprofCaptureJitmap *jitmap;
   uint8_t *buf;
   uint8_t *endptr;
@@ -787,17 +786,15 @@ sysprof_capture_reader_read_jitmap (SysprofCaptureReader *self)
   if (!sysprof_capture_reader_ensure_space_for (self, jitmap->frame.len))
     return NULL;
 
-  jitmap = (SysprofCaptureJitmap *)(gpointer)&self->buf[self->pos];
+  jitmap = (SysprofCaptureJitmap *)(void *)&self->buf[self->pos];
-
-  ret = g_hash_table_new_full (NULL, NULL, NULL, g_free);
 
   buf = jitmap->data;
   endptr = &self->buf[self->pos + jitmap->frame.len];
 
+  /* Check the strings are all nul-terminated. */
   for (i = 0; i < jitmap->n_jitmaps; i++)
     {
       SysprofCaptureAddress addr;
-      const char *str;
 
       if (buf + sizeof addr >= endptr)
         return NULL;
@@ -805,23 +802,19 @@ sysprof_capture_reader_read_jitmap (SysprofCaptureReader *self)
       memcpy (&addr, buf, sizeof addr);
       buf += sizeof addr;
 
-      str = (char *)buf;
-
       buf = memchr (buf, '\0', (endptr - buf));
 
       if (buf == NULL)
         return NULL;
 
       buf++;
-
-      g_hash_table_insert (ret, GSIZE_TO_POINTER (addr), g_strdup (str));
     }
 
   sysprof_capture_reader_bswap_jitmap (self, jitmap);
 
   self->pos += jitmap->frame.len;
 
-  return sysprof_steal_pointer (&ret);
+  return jitmap;
 }
 
 const SysprofCaptureSample *
@@ -1512,3 +1505,53 @@ sysprof_capture_reader_read_allocation (SysprofCaptureReader *self)
 
   return ma;
 }
+
+typedef struct {
+  const SysprofCaptureJitmap *jitmap;
+  const uint8_t *buf;
+  unsigned int i;
+
+  void *padding1;
+  void *padding2;
+} RealSysprofCaptureJitmapIter;
+
+void
+sysprof_capture_jitmap_iter_init (SysprofCaptureJitmapIter   *iter,
+                                  const SysprofCaptureJitmap *jitmap)
+{
+  RealSysprofCaptureJitmapIter *real_iter = (RealSysprofCaptureJitmapIter *) iter;
+
+  assert (iter != NULL);
+  assert (jitmap != NULL);
+
+  real_iter->jitmap = jitmap;
+  real_iter->buf = jitmap->data;
+  real_iter->i = 0;
+}
+
+bool
+sysprof_capture_jitmap_iter_next (SysprofCaptureJitmapIter  *iter,
+                                  SysprofCaptureAddress     *addr,
+                                  const char               **name)
+{
+  RealSysprofCaptureJitmapIter *real_iter = (RealSysprofCaptureJitmapIter *) iter;
+  const char *_name;
+
+  assert (iter != NULL);
+
+  if (real_iter->i >= real_iter->jitmap->n_jitmaps)
+    return false;
+
+  if (addr != NULL)
+    memcpy (addr, real_iter->buf, sizeof (*addr));
+  real_iter->buf += sizeof (*addr);
+
+  _name = (const char *) real_iter->buf;
+  if (name != NULL)
+    *name = _name;
+  real_iter->buf += strlen (_name) + 1;
+
+  real_iter->i++;
+
+  return true;
+}

src/libsysprof-capture/sysprof-capture-reader.h

@@ -115,7 +115,7 @@ const SysprofCaptureProcess        *sysprof_capture_reader_read_process        (
 SYSPROF_AVAILABLE_IN_ALL
 const SysprofCaptureSample         *sysprof_capture_reader_read_sample         (SysprofCaptureReader      *self);
 SYSPROF_AVAILABLE_IN_ALL
-GHashTable                         *sysprof_capture_reader_read_jitmap         (SysprofCaptureReader      *self);
+const SysprofCaptureJitmap         *sysprof_capture_reader_read_jitmap         (SysprofCaptureReader      *self);
 SYSPROF_AVAILABLE_IN_ALL
 const SysprofCaptureCounterDefine  *sysprof_capture_reader_read_counter_define (SysprofCaptureReader      *self);
 SYSPROF_AVAILABLE_IN_ALL
@@ -150,5 +150,21 @@ bool                                sysprof_capture_reader_read_file_fd        (
                                                                                 const char                *path,
                                                                                 int                        fd);
 
+typedef struct {
+  /*< private >*/
+  void *p1;
+  void *p2;
+  unsigned int u1;
+  void *p3;
+  void *p4;
+} SysprofCaptureJitmapIter;
+
+SYSPROF_AVAILABLE_IN_3_38
+void                                sysprof_capture_jitmap_iter_init           (SysprofCaptureJitmapIter    *iter,
+                                                                                const SysprofCaptureJitmap  *jitmap);
+SYSPROF_AVAILABLE_IN_3_38
+bool                                sysprof_capture_jitmap_iter_next           (SysprofCaptureJitmapIter    *iter,
+                                                                                SysprofCaptureAddress       *addr,
+                                                                                const char                 **path);
 
 SYSPROF_END_DECLS

src/libsysprof-capture/sysprof-capture-writer-cat.c

@@ -197,10 +197,10 @@ sysprof_capture_writer_cat (SysprofCaptureWriter  *self,
    */
   while (sysprof_capture_reader_peek_type (reader, &type))
     {
-      g_autoptr(GHashTable) jitmap = NULL;
+      const SysprofCaptureJitmap *jitmap;
-      GHashTableIter iter;
+      SysprofCaptureJitmapIter iter;
+      SysprofCaptureAddress addr;
       const char *name;
-      uint64_t addr;
 
       if (type != SYSPROF_CAPTURE_FRAME_JITMAP)
         {
@@ -212,8 +212,8 @@ sysprof_capture_writer_cat (SysprofCaptureWriter  *self,
       if (!(jitmap = sysprof_capture_reader_read_jitmap (reader)))
         goto panic;
 
-      g_hash_table_iter_init (&iter, jitmap);
+      sysprof_capture_jitmap_iter_init (&iter, jitmap);
-      while (g_hash_table_iter_next (&iter, (gpointer *)&addr, (gpointer *)&name))
+      while (sysprof_capture_jitmap_iter_next (&iter, &addr, &name))
         {
           uint64_t replace = sysprof_capture_writer_add_jitmap (self, name);
           /* We need to keep a table of replacement addresses so that

src/libsysprof/sysprof-jitmap-symbol-resolver.c

@@ -74,8 +74,8 @@ sysprof_jitmap_symbol_resolver_load (SysprofSymbolResolver *resolver,
 
   while (sysprof_capture_reader_peek_type (reader, &type))
     {
-      g_autoptr(GHashTable) jitmap = NULL;
+      const SysprofCaptureJitmap *jitmap;
-      GHashTableIter iter;
+      SysprofCaptureJitmapIter iter;
       SysprofCaptureAddress addr;
       const gchar *str;
 
@@ -89,8 +89,8 @@ sysprof_jitmap_symbol_resolver_load (SysprofSymbolResolver *resolver,
       if (!(jitmap = sysprof_capture_reader_read_jitmap (reader)))
         return;
 
-      g_hash_table_iter_init (&iter, jitmap);
+      sysprof_capture_jitmap_iter_init (&iter, jitmap);
-      while (g_hash_table_iter_next (&iter, (gpointer *)&addr, (gpointer *)&str))
+      while (sysprof_capture_jitmap_iter_next (&iter, &addr, &str))
         g_hash_table_insert (self->jitmap, GSIZE_TO_POINTER (addr), g_strdup (str));
     }
 }

src/tests/test-capture.c

@@ -32,15 +32,16 @@
 #include "sysprof-capture-util-private.h"
 
 static void
-copy_into (GHashTable *src,
+copy_into (const SysprofCaptureJitmap *src,
-           GHashTable *dst)
+           GHashTable                 *dst)
 {
-  GHashTableIter iter;
+  SysprofCaptureJitmapIter iter;
-  gpointer k, v;
+  SysprofCaptureAddress addr;
+  const char *name;
 
-  g_hash_table_iter_init (&iter, src);
+  sysprof_capture_jitmap_iter_init (&iter, src);
-  while (g_hash_table_iter_next (&iter, &k, &v))
+  while (sysprof_capture_jitmap_iter_next (&iter, &addr, &name))
-    g_hash_table_insert (dst, k, g_strdup ((gchar *)v));
+    g_hash_table_insert (dst, GSIZE_TO_POINTER (addr), g_strdup (name));
 }
 
 static void
@@ -318,19 +319,19 @@ test_reader_basic (void)
   for (;;)
     {
       SysprofCaptureFrameType type = -1;
-      g_autoptr(GHashTable) ret = NULL;
+      const SysprofCaptureJitmap *jitmap;
 
       if (sysprof_capture_reader_peek_type (reader, &type))
         g_assert_cmpint (type, ==, SYSPROF_CAPTURE_FRAME_JITMAP);
       else
         break;
 
-      ret = sysprof_capture_reader_read_jitmap (reader);
+      jitmap = sysprof_capture_reader_read_jitmap (reader);
-      g_assert (ret != NULL);
+      g_assert_nonnull (jitmap);
 
-      i += g_hash_table_size (ret);
+      i += jitmap->n_jitmaps;
 
-      copy_into (ret, jmap);
+      copy_into (jitmap, jmap);
     }
 
   g_assert_cmpint (1000, ==, i);
@@ -887,7 +888,7 @@ test_reader_writer_cat_jitmap (void)
   reader = sysprof_capture_writer_create_reader (res, &error);
   g_assert_no_error (error);
   g_assert_nonnull (reader);
-  g_hash_table_unref (sysprof_capture_reader_read_jitmap (reader));
+  sysprof_capture_reader_read_jitmap (reader);
   sample = sysprof_capture_reader_read_sample (reader);
   g_assert_cmpint (sample->frame.pid, ==, getpid ());
   g_assert_cmpint (sample->n_addrs, ==, G_N_ELEMENTS (addrs));

src/tools/sysprof-dump.c

@@ -115,18 +115,18 @@ main (gint argc,
 
         case SYSPROF_CAPTURE_FRAME_JITMAP:
           {
-            g_autoptr(GHashTable) ret = sysprof_capture_reader_read_jitmap (reader);
+            const SysprofCaptureJitmap *jitmap = sysprof_capture_reader_read_jitmap (reader);
-            GHashTableIter iter;
+            SysprofCaptureJitmapIter iter;
             SysprofCaptureAddress addr;
             const gchar *str;
 
-            if (ret == NULL)
+            if (jitmap == NULL)
               return EXIT_FAILURE;
 
             g_print ("JITMAP:\n");
 
-            g_hash_table_iter_init (&iter, ret);
+            sysprof_capture_jitmap_iter_init (&iter, jitmap);
-            while (g_hash_table_iter_next (&iter, (gpointer *)&addr, (gpointer *)&str))
+            while (sysprof_capture_jitmap_iter_next (&iter, &addr, &str))
               g_print ("  " SYSPROF_CAPTURE_ADDRESS_FORMAT " : %s\n", addr, str);
 
             break;