g_list_free should be passed the head of the list.

Signed-off-by: Alex Bradbury <asb@igalia.com>
---
 contrib/plugins/hotblocks.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/contrib/plugins/hotblocks.c b/contrib/plugins/hotblocks.c
index 98404b6885..d3dd23ed9f 100644
--- a/contrib/plugins/hotblocks.c
+++ b/contrib/plugins/hotblocks.c
@@ -73,15 +73,16 @@ static void exec_count_free(gpointer key, gpointer value, gpointer user_data)
 static void plugin_exit(qemu_plugin_id_t id, void *p)
 {
     g_autoptr(GString) report = g_string_new("collected ");
-    GList *counts, *it;
+    GList *counts, *sorted_counts, *it;
     int i;
 
     g_string_append_printf(report, "%d entries in the hash table\n",
                            g_hash_table_size(hotblocks));
     counts = g_hash_table_get_values(hotblocks);
-    it = g_list_sort_with_data(counts, cmp_exec_count, NULL);
+    sorted_counts = g_list_sort_with_data(counts, cmp_exec_count, NULL);
 
-    if (it) {
+    if (sorted_counts) {
+        it = sorted_counts;
         g_string_append_printf(report, "pc, tcount, icount, ecount\n");
 
         for (i = 0; i < limit && it->next; i++, it = it->next) {
@@ -94,7 +95,7 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
                     qemu_plugin_scoreboard_u64(rec->exec_count)));
         }
 
-        g_list_free(it);
+        g_list_free(sorted_counts);
     }
 
     qemu_plugin_outs(report->str);
-- 
2.50.1

On 7/29/25 11:41 PM, Alex Bradbury wrote:
> g_list_free should be passed the head of the list.
> 
> Signed-off-by: Alex Bradbury <asb@igalia.com>
> ---
>   contrib/plugins/hotblocks.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)

Thanks for catching this.
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

On Wed, Jul 30, 2025 at 4:50 PM Alex Bradbury <asb@igalia.com> wrote:
>
> g_list_free should be passed the head of the list.
>
> Signed-off-by: Alex Bradbury <asb@igalia.com>
> ---
>  contrib/plugins/hotblocks.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/contrib/plugins/hotblocks.c b/contrib/plugins/hotblocks.c
> index 98404b6885..d3dd23ed9f 100644
> --- a/contrib/plugins/hotblocks.c
> +++ b/contrib/plugins/hotblocks.c
> @@ -73,15 +73,16 @@ static void exec_count_free(gpointer key, gpointer value, gpointer user_data)
>  static void plugin_exit(qemu_plugin_id_t id, void *p)
>  {
>      g_autoptr(GString) report = g_string_new("collected ");
> -    GList *counts, *it;
> +    GList *counts, *sorted_counts, *it;
>      int i;
>
>      g_string_append_printf(report, "%d entries in the hash table\n",
>                             g_hash_table_size(hotblocks));
>      counts = g_hash_table_get_values(hotblocks);
> -    it = g_list_sort_with_data(counts, cmp_exec_count, NULL);
> +    sorted_counts = g_list_sort_with_data(counts, cmp_exec_count, NULL);
>
> -    if (it) {
> +    if (sorted_counts) {
> +        it = sorted_counts;
>          g_string_append_printf(report, "pc, tcount, icount, ecount\n");
>
>          for (i = 0; i < limit && it->next; i++, it = it->next) {
> @@ -94,7 +95,7 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
>                      qemu_plugin_scoreboard_u64(rec->exec_count)));
>          }
>
> -        g_list_free(it);
> +        g_list_free(sorted_counts);
>      }
>
>      qemu_plugin_outs(report->str);
> --
> 2.50.1
>
>

This looks correct to me. `g_hash_table_get_values` documentation says
the returned value must be freed with `g_list_free`, but
`g_list_sort_with_data` might change the list head, so neither
`counts` nor `it` should be freed, but `sorted_counts` which is the
new list head like the commit message says.

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>