[sysprof: 51/63] libsysprof-capture: Add SysprofCaptureJitmapIter to replace GHashTable

[Christian Hergert <chergert src gnome org>]

commit 6a45f020f7e5f8d6a442028869a1e51a689c26fc
Author: Philip Withnall <withnall endlessm com>
Date:   Thu Jul 2 12:43:15 2020 +0100

    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    | 65 ++++++++++++++++++----
 src/libsysprof-capture/sysprof-capture-reader.h    | 18 +++++-
 .../sysprof-capture-writer-cat.c                   | 10 ++--
 src/libsysprof/sysprof-jitmap-symbol-resolver.c    |  8 +--
 src/tests/test-capture.c                           | 27 ++++-----
 src/tools/sysprof-dump.c                           | 10 ++--
 6 files changed, 99 insertions(+), 39 deletions(-)
---
diff --git a/src/libsysprof-capture/sysprof-capture-reader.c b/src/libsysprof-capture/sysprof-capture-reader.c
index 1166ae5..cfa42a6 100644
--- a/src/libsysprof-capture/sysprof-capture-reader.c
+++ b/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];
-
-  ret = g_hash_table_new_full (NULL, NULL, NULL, g_free);
+  jitmap = (SysprofCaptureJitmap *)(void *)&self->buf[self->pos];
 
   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;
+}
diff --git a/src/libsysprof-capture/sysprof-capture-reader.h b/src/libsysprof-capture/sysprof-capture-reader.h
index 0fffa6a..18561ad 100644
--- a/src/libsysprof-capture/sysprof-capture-reader.h
+++ b/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
diff --git a/src/libsysprof-capture/sysprof-capture-writer-cat.c 
b/src/libsysprof-capture/sysprof-capture-writer-cat.c
index 8ee52c7..d33b4bb 100644
--- a/src/libsysprof-capture/sysprof-capture-writer-cat.c
+++ b/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;
-      GHashTableIter iter;
+      const SysprofCaptureJitmap *jitmap;
+      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);
-      while (g_hash_table_iter_next (&iter, (gpointer *)&addr, (gpointer *)&name))
+      sysprof_capture_jitmap_iter_init (&iter, jitmap);
+      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
diff --git a/src/libsysprof/sysprof-jitmap-symbol-resolver.c b/src/libsysprof/sysprof-jitmap-symbol-resolver.c
index ef63e8f..2015eb5 100644
--- a/src/libsysprof/sysprof-jitmap-symbol-resolver.c
+++ b/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;
-      GHashTableIter iter;
+      const SysprofCaptureJitmap *jitmap;
+      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);
-      while (g_hash_table_iter_next (&iter, (gpointer *)&addr, (gpointer *)&str))
+      sysprof_capture_jitmap_iter_init (&iter, jitmap);
+      while (sysprof_capture_jitmap_iter_next (&iter, &addr, &str))
         g_hash_table_insert (self->jitmap, GSIZE_TO_POINTER (addr), g_strdup (str));
     }
 }
diff --git a/src/tests/test-capture.c b/src/tests/test-capture.c
index dda2ab1..cb88f01 100644
--- a/src/tests/test-capture.c
+++ b/src/tests/test-capture.c
@@ -32,15 +32,16 @@
 #include "sysprof-capture-util-private.h"
 
 static void
-copy_into (GHashTable *src,
-           GHashTable *dst)
+copy_into (const SysprofCaptureJitmap *src,
+           GHashTable                 *dst)
 {
-  GHashTableIter iter;
-  gpointer k, v;
+  SysprofCaptureJitmapIter iter;
+  SysprofCaptureAddress addr;
+  const char *name;
 
-  g_hash_table_iter_init (&iter, src);
-  while (g_hash_table_iter_next (&iter, &k, &v))
-    g_hash_table_insert (dst, k, g_strdup ((gchar *)v));
+  sysprof_capture_jitmap_iter_init (&iter, src);
+  while (sysprof_capture_jitmap_iter_next (&iter, &addr, &name))
+    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);
-      g_assert (ret != NULL);
+      jitmap = sysprof_capture_reader_read_jitmap (reader);
+      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));
diff --git a/src/tools/sysprof-dump.c b/src/tools/sysprof-dump.c
index 17bd3a9..d90963d 100644
--- a/src/tools/sysprof-dump.c
+++ b/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);
-            GHashTableIter iter;
+            const SysprofCaptureJitmap *jitmap = sysprof_capture_reader_read_jitmap (reader);
+            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);
-            while (g_hash_table_iter_next (&iter, (gpointer *)&addr, (gpointer *)&str))
+            sysprof_capture_jitmap_iter_init (&iter, jitmap);
+            while (sysprof_capture_jitmap_iter_next (&iter, &addr, &str))
               g_print ("  " SYSPROF_CAPTURE_ADDRESS_FORMAT " : %s\n", addr, str);
 
             break;