[PATCH 01/10] various: Fix type conflict of GLib function pointers

Kohei Tokunaga posted 10 patches 10 months ago
[PATCH 01/10] various: Fix type conflict of GLib function pointers
Posted by Kohei Tokunaga 10 months ago
On emscripten, function pointer casts can cause function call failure.
This commit fixes the function definition to match to the type of the
function call.

- qtest_set_command_cb passed to g_once should match to GThreadFunc
- object_class_cmp and cpreg_key_compare are passed to g_list_sort as
  GCopmareFunc but GLib cast them to GCompareDataFunc.

Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
---
 hw/riscv/riscv_hart.c | 9 ++++++++-
 qom/object.c          | 5 +++--
 target/arm/helper.c   | 4 ++--
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
index a55d156668..e37317dcbd 100644
--- a/hw/riscv/riscv_hart.c
+++ b/hw/riscv/riscv_hart.c
@@ -102,10 +102,17 @@ static bool csr_qtest_callback(CharBackend *chr, gchar **words)
     return false;
 }
 
+static gpointer g_qtest_set_command_cb(
+    bool (*pc_cb)(CharBackend *chr, gchar **words))
+{
+    qtest_set_command_cb(pc_cb);
+    return NULL;
+}
+
 static void riscv_cpu_register_csr_qtest_callback(void)
 {
     static GOnce once;
-    g_once(&once, (GThreadFunc)qtest_set_command_cb, csr_qtest_callback);
+    g_once(&once, (GThreadFunc)g_qtest_set_command_cb, csr_qtest_callback);
 }
 #endif
 
diff --git a/qom/object.c b/qom/object.c
index 01618d06bd..19698aae4c 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1191,7 +1191,8 @@ GSList *object_class_get_list(const char *implements_type,
     return list;
 }
 
-static gint object_class_cmp(gconstpointer a, gconstpointer b)
+static gint object_class_cmp(gconstpointer a, gconstpointer b,
+                             gpointer user_data)
 {
     return strcasecmp(object_class_get_name((ObjectClass *)a),
                       object_class_get_name((ObjectClass *)b));
@@ -1201,7 +1202,7 @@ GSList *object_class_get_list_sorted(const char *implements_type,
                                      bool include_abstract)
 {
     return g_slist_sort(object_class_get_list(implements_type, include_abstract),
-                        object_class_cmp);
+                        (GCompareFunc)object_class_cmp);
 }
 
 Object *object_ref(void *objptr)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index bb445e30cd..68f81fadfc 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -220,7 +220,7 @@ static void count_cpreg(gpointer key, gpointer opaque)
     }
 }
 
-static gint cpreg_key_compare(gconstpointer a, gconstpointer b)
+static gint cpreg_key_compare(gconstpointer a, gconstpointer b, void *d)
 {
     uint64_t aidx = cpreg_to_kvm_id((uintptr_t)a);
     uint64_t bidx = cpreg_to_kvm_id((uintptr_t)b);
@@ -244,7 +244,7 @@ void init_cpreg_list(ARMCPU *cpu)
     int arraylen;
 
     keys = g_hash_table_get_keys(cpu->cp_regs);
-    keys = g_list_sort(keys, cpreg_key_compare);
+    keys = g_list_sort(keys, (GCompareFunc)cpreg_key_compare);
 
     cpu->cpreg_array_len = 0;
 
-- 
2.25.1
Re: [PATCH 01/10] various: Fix type conflict of GLib function pointers
Posted by Paolo Bonzini 10 months ago
On 4/7/25 16:45, Kohei Tokunaga wrote:
> On emscripten, function pointer casts can cause function call failure.
> This commit fixes the function definition to match to the type of the
> function call.
> 
> - qtest_set_command_cb passed to g_once should match to GThreadFunc

Sending an alternative patch that doesn't use GOnce, this code runs in 
the main thread.

> - object_class_cmp and cpreg_key_compare are passed to g_list_sort as
>    GCopmareFunc but GLib cast them to GCompareDataFunc.

Please use g_list_sort_with_data instead, and poison 
g_slist_sort/g_list_sort in include/glib-compat.h, with a comment 
explaining that it's done this way because of Emscripten.

Paolo

> Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
> ---
>   hw/riscv/riscv_hart.c | 9 ++++++++-
>   qom/object.c          | 5 +++--
>   target/arm/helper.c   | 4 ++--
>   3 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
> index a55d156668..e37317dcbd 100644
> --- a/hw/riscv/riscv_hart.c
> +++ b/hw/riscv/riscv_hart.c
> @@ -102,10 +102,17 @@ static bool csr_qtest_callback(CharBackend *chr, gchar **words)
>       return false;
>   }
>   
> +static gpointer g_qtest_set_command_cb(
> +    bool (*pc_cb)(CharBackend *chr, gchar **words))
> +{
> +    qtest_set_command_cb(pc_cb);
> +    return NULL;
> +}
> +
>   static void riscv_cpu_register_csr_qtest_callback(void)
>   {
>       static GOnce once;
> -    g_once(&once, (GThreadFunc)qtest_set_command_cb, csr_qtest_callback);
> +    g_once(&once, (GThreadFunc)g_qtest_set_command_cb, csr_qtest_callback);
>   }
>   #endif
>   
> diff --git a/qom/object.c b/qom/object.c
> index 01618d06bd..19698aae4c 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1191,7 +1191,8 @@ GSList *object_class_get_list(const char *implements_type,
>       return list;
>   }
>   
> -static gint object_class_cmp(gconstpointer a, gconstpointer b)
> +static gint object_class_cmp(gconstpointer a, gconstpointer b,
> +                             gpointer user_data)
>   {
>       return strcasecmp(object_class_get_name((ObjectClass *)a),
>                         object_class_get_name((ObjectClass *)b));
> @@ -1201,7 +1202,7 @@ GSList *object_class_get_list_sorted(const char *implements_type,
>                                        bool include_abstract)
>   {
>       return g_slist_sort(object_class_get_list(implements_type, include_abstract),
> -                        object_class_cmp);
> +                        (GCompareFunc)object_class_cmp);
>   }
>   
>   Object *object_ref(void *objptr)
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index bb445e30cd..68f81fadfc 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -220,7 +220,7 @@ static void count_cpreg(gpointer key, gpointer opaque)
>       }
>   }
>   
> -static gint cpreg_key_compare(gconstpointer a, gconstpointer b)
> +static gint cpreg_key_compare(gconstpointer a, gconstpointer b, void *d)
>   {
>       uint64_t aidx = cpreg_to_kvm_id((uintptr_t)a);
>       uint64_t bidx = cpreg_to_kvm_id((uintptr_t)b);
> @@ -244,7 +244,7 @@ void init_cpreg_list(ARMCPU *cpu)
>       int arraylen;
>   
>       keys = g_hash_table_get_keys(cpu->cp_regs);
> -    keys = g_list_sort(keys, cpreg_key_compare);
> +    keys = g_list_sort(keys, (GCompareFunc)cpreg_key_compare);
>   
>       cpu->cpreg_array_len = 0;
>
Re: [PATCH 01/10] various: Fix type conflict of GLib function pointers
Posted by Kohei Tokunaga 10 months ago
Hi Paolo,

> > On emscripten, function pointer casts can cause function call failure.
> > This commit fixes the function definition to match to the type of the
> > function call.
> >
> > - qtest_set_command_cb passed to g_once should match to GThreadFunc
>
> Sending an alternative patch that doesn't use GOnce, this code runs in
> the main thread.

Thank you for addressing this issue. I've sent a review to that patch.

> > - object_class_cmp and cpreg_key_compare are passed to g_list_sort as
> >    GCopmareFunc but GLib cast them to GCompareDataFunc.
>
> Please use g_list_sort_with_data instead, and poison
> g_slist_sort/g_list_sort in include/glib-compat.h, with a comment
> explaining that it's done this way because of Emscripten.

Sure, I’ll fix this in the next version of the series.
Re: [PATCH 01/10] various: Fix type conflict of GLib function pointers
Posted by Philippe Mathieu-Daudé 10 months ago
Hi Kohei,

On 7/4/25 16:45, Kohei Tokunaga wrote:
> On emscripten, function pointer casts can cause function call failure.
> This commit fixes the function definition to match to the type of the
> function call.
> 
> - qtest_set_command_cb passed to g_once should match to GThreadFunc
> - object_class_cmp and cpreg_key_compare are passed to g_list_sort as
>    GCopmareFunc but GLib cast them to GCompareDataFunc.
> 
> Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
> ---
>   hw/riscv/riscv_hart.c | 9 ++++++++-
>   qom/object.c          | 5 +++--
>   target/arm/helper.c   | 4 ++--
>   3 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
> index a55d156668..e37317dcbd 100644
> --- a/hw/riscv/riscv_hart.c
> +++ b/hw/riscv/riscv_hart.c
> @@ -102,10 +102,17 @@ static bool csr_qtest_callback(CharBackend *chr, gchar **words)
>       return false;
>   }
>   
> +static gpointer g_qtest_set_command_cb(
> +    bool (*pc_cb)(CharBackend *chr, gchar **words))
> +{

Why not use a GThreadFunc prototype, casting the argument?

> +    qtest_set_command_cb(pc_cb);
> +    return NULL;
> +}
> +
>   static void riscv_cpu_register_csr_qtest_callback(void)
>   {
>       static GOnce once;
> -    g_once(&once, (GThreadFunc)qtest_set_command_cb, csr_qtest_callback);
> +    g_once(&once, (GThreadFunc)g_qtest_set_command_cb, csr_qtest_callback);
>   }
>   #endif
>   

OK for the rest, but it might help to merge by corresponding maintainers
if split in 3 distinct patches.

> diff --git a/qom/object.c b/qom/object.c
> index 01618d06bd..19698aae4c 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1191,7 +1191,8 @@ GSList *object_class_get_list(const char *implements_type,
>       return list;
>   }
>   
> -static gint object_class_cmp(gconstpointer a, gconstpointer b)
> +static gint object_class_cmp(gconstpointer a, gconstpointer b,
> +                             gpointer user_data)
>   {
>       return strcasecmp(object_class_get_name((ObjectClass *)a),
>                         object_class_get_name((ObjectClass *)b));
> @@ -1201,7 +1202,7 @@ GSList *object_class_get_list_sorted(const char *implements_type,
>                                        bool include_abstract)
>   {
>       return g_slist_sort(object_class_get_list(implements_type, include_abstract),
> -                        object_class_cmp);
> +                        (GCompareFunc)object_class_cmp);
>   }
>   
>   Object *object_ref(void *objptr)
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index bb445e30cd..68f81fadfc 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -220,7 +220,7 @@ static void count_cpreg(gpointer key, gpointer opaque)
>       }
>   }
>   
> -static gint cpreg_key_compare(gconstpointer a, gconstpointer b)
> +static gint cpreg_key_compare(gconstpointer a, gconstpointer b, void *d)
>   {
>       uint64_t aidx = cpreg_to_kvm_id((uintptr_t)a);
>       uint64_t bidx = cpreg_to_kvm_id((uintptr_t)b);
> @@ -244,7 +244,7 @@ void init_cpreg_list(ARMCPU *cpu)
>       int arraylen;
>   
>       keys = g_hash_table_get_keys(cpu->cp_regs);
> -    keys = g_list_sort(keys, cpreg_key_compare);
> +    keys = g_list_sort(keys, (GCompareFunc)cpreg_key_compare);
>   
>       cpu->cpreg_array_len = 0;
>
Re: [PATCH 01/10] various: Fix type conflict of GLib function pointers
Posted by Kohei Tokunaga 10 months ago
Hi Philippe, thank you for the comments.

> > +static gpointer g_qtest_set_command_cb(
> > +    bool (*pc_cb)(CharBackend *chr, gchar **words))
> > +{
>
> Why not use a GThreadFunc prototype, casting the argument?

Sure, I'll fix this.

> OK for the rest, but it might help to merge by corresponding maintainers
> if split in 3 distinct patches.

Thank you for the suggestion. I'll split it to distinct patches in the next
version of the series.