In plugin_exit() we call g_hash_table_get_values() to get a GList
which we look at to print some information. This code has
multiple issues:
 * it names the local variable for the GList "count", which
   shadows the "qemu_plugin_scoreboard *count". This isn't
   incorrect, but it is unnecessarily confusing
 * it doesn't free the list, and the leak sanitizer complains:

   Indirect leak of 2328 byte(s) in 97 object(s) allocated from:
    #0 0x5589b0b72293 in malloc (/home/pm215/qemu/build/x86-tgt-san/qemu-system-i386+0x1a2f293) (BuildId: 26964cad9e3f81d35fc144d7cc88b53adf6f60c7)
    #1 0x78fd8cfa1ac9 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62ac9) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
    #2 0x78fd8cf96e4a in g_list_prepend (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x57e4a) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
    #3 0x78fd8cf8b318 in g_hash_table_get_values (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4c318) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
    #4 0x78fd84d1a90c in plugin_exit /home/pm215/qemu/build/x86-tgt-san/../../tests/tcg/plugins/mem.c:87:25
 * in iterating through the list it updates "count", so by the
   time we get to the end of the loop we no longer have a pointer
   to the head of the list that we could use to free it
 * it checks for the list being NULL twice (once in an if()
   and once in the for() loop's "while" condition), which is
   redundant
 * it skips the loop if g_list_next(counts) is NULL, which means
   it will wrongly skip the loop if the list has only one entry

Rewrite the iteration code to fix these problems.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I am assuming that not printing the region info when there is
exactly one region is a mistake, but perhaps it is intentional?

 tests/tcg/plugins/mem.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/tests/tcg/plugins/mem.c b/tests/tcg/plugins/mem.c
index f3992abc8f..1ee257f855 100644
--- a/tests/tcg/plugins/mem.c
+++ b/tests/tcg/plugins/mem.c
@@ -84,24 +84,22 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
 
 
     if (do_region_summary) {
-        GList *counts = g_hash_table_get_values(regions);
+        g_autoptr(GList) regionlist = g_hash_table_get_values(regions);
 
-        counts = g_list_sort_with_data(counts, addr_order, NULL);
+        regionlist = g_list_sort_with_data(regionlist, addr_order, NULL);
 
         g_string_printf(out, "Region Base, Reads, Writes, Seen all\n");
 
-        if (counts && g_list_next(counts)) {
-            for (/* counts */; counts; counts = counts->next) {
-                RegionInfo *ri = (RegionInfo *) counts->data;
+        for (GList *l = regionlist; l; l = g_list_next(l)) {
+            RegionInfo *ri = (RegionInfo *) l->data;
 
-                g_string_append_printf(out,
-                                       "0x%016"PRIx64", "
-                                       "%"PRId64", %"PRId64", %s\n",
-                                       ri->region_address,
-                                       ri->reads,
-                                       ri->writes,
-                                       ri->seen_all ? "true" : "false");
-            }
+            g_string_append_printf(out,
+                                   "0x%016"PRIx64", "
+                                   "%"PRId64", %"PRId64", %s\n",
+                                   ri->region_address,
+                                   ri->reads,
+                                   ri->writes,
+                                   ri->seen_all ? "true" : "false");
         }
         qemu_plugin_outs(out->str);
     }
-- 
2.43.0

On 3/5/26 8:15 AM, Peter Maydell wrote:
> In plugin_exit() we call g_hash_table_get_values() to get a GList
> which we look at to print some information. This code has
> multiple issues:
>   * it names the local variable for the GList "count", which
>     shadows the "qemu_plugin_scoreboard *count". This isn't
>     incorrect, but it is unnecessarily confusing
>   * it doesn't free the list, and the leak sanitizer complains:
> 
>     Indirect leak of 2328 byte(s) in 97 object(s) allocated from:
>      #0 0x5589b0b72293 in malloc (/home/pm215/qemu/build/x86-tgt-san/qemu-system-i386+0x1a2f293) (BuildId: 26964cad9e3f81d35fc144d7cc88b53adf6f60c7)
>      #1 0x78fd8cfa1ac9 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62ac9) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
>      #2 0x78fd8cf96e4a in g_list_prepend (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x57e4a) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
>      #3 0x78fd8cf8b318 in g_hash_table_get_values (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4c318) (BuildId: 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
>      #4 0x78fd84d1a90c in plugin_exit /home/pm215/qemu/build/x86-tgt-san/../../tests/tcg/plugins/mem.c:87:25
>   * in iterating through the list it updates "count", so by the
>     time we get to the end of the loop we no longer have a pointer
>     to the head of the list that we could use to free it
>   * it checks for the list being NULL twice (once in an if()
>     and once in the for() loop's "while" condition), which is
>     redundant
>   * it skips the loop if g_list_next(counts) is NULL, which means
>     it will wrongly skip the loop if the list has only one entry
> 
> Rewrite the iteration code to fix these problems.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I am assuming that not printing the region info when there is
> exactly one region is a mistake, but perhaps it is intentional?
> 
>   tests/tcg/plugins/mem.c | 24 +++++++++++-------------
>   1 file changed, 11 insertions(+), 13 deletions(-)
> 

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>