[PATCH v2] contrib/plugins/execlog: Fix compiler warning

Yao Xingtao via posted 1 patch 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20240325060657.3934-1-yaoxt.fnst@fujitsu.com
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, Alexandre Iooss <erdnaxe@crans.org>, Mahmoud Mandour <ma.mandourr@gmail.com>, Pierrick Bouvier <pierrick.bouvier@linaro.org>
There is a newer version of this series
contrib/plugins/execlog.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
[PATCH v2] contrib/plugins/execlog: Fix compiler warning
Posted by Yao Xingtao via 1 month ago
1. The g_pattern_match_string() is deprecated when glib2 version >= 2.70.
   Use g_pattern_spec_match_string() instead to avoid this problem.

2. The type of second parameter in g_ptr_array_add() is
   'gpointer' {aka 'void *'}, but the type of reg->name is 'const char*'.
   Cast the type of reg->name to 'gpointer' to avoid this problem.

compiler warning message:
/root/qemu/contrib/plugins/execlog.c:330:17: warning: ‘g_pattern_match_string’
is deprecated: Use 'g_pattern_spec_match_string'
instead [-Wdeprecated-declarations]
  330 |                 if (g_pattern_match_string(pat, rd->name) ||
      |                 ^~
In file included from /usr/include/glib-2.0/glib.h:67,
                 from /root/qemu/contrib/plugins/execlog.c:9:
/usr/include/glib-2.0/glib/gpattern.h:57:15: note: declared here
   57 | gboolean      g_pattern_match_string   (GPatternSpec *pspec,
      |               ^~~~~~~~~~~~~~~~~~~~~~
/root/qemu/contrib/plugins/execlog.c:331:21: warning: ‘g_pattern_match_string’
is deprecated: Use 'g_pattern_spec_match_string'
instead [-Wdeprecated-declarations]
  331 |                     g_pattern_match_string(pat, rd_lower)) {
      |                     ^~~~~~~~~~~~~~~~~~~~~~
/usr/include/glib-2.0/glib/gpattern.h:57:15: note: declared here
   57 | gboolean      g_pattern_match_string   (GPatternSpec *pspec,
      |               ^~~~~~~~~~~~~~~~~~~~~~
/root/qemu/contrib/plugins/execlog.c:339:63: warning: passing argument 2 of
‘g_ptr_array_add’ discards ‘const’ qualifier from pointer target type
[-Wdiscarded-qualifiers]
  339 |                             g_ptr_array_add(all_reg_names, reg->name);
      |                                                            ~~~^~~~~~
In file included from /usr/include/glib-2.0/glib.h:33:
/usr/include/glib-2.0/glib/garray.h:198:62: note: expected
‘gpointer’ {aka ‘void *’} but argument is of type ‘const char *’
  198 |                                            gpointer          data);
      |                                            ~~~~~~~~~~~~~~~~~~^~~~

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2210
Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
---
 contrib/plugins/execlog.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
index a1dfd59ab7..09654910ee 100644
--- a/contrib/plugins/execlog.c
+++ b/contrib/plugins/execlog.c
@@ -327,8 +327,13 @@ static GPtrArray *registers_init(int vcpu_index)
             for (int p = 0; p < rmatches->len; p++) {
                 g_autoptr(GPatternSpec) pat = g_pattern_spec_new(rmatches->pdata[p]);
                 g_autofree gchar *rd_lower = g_utf8_strdown(rd->name, -1);
+#if GLIB_CHECK_VERSION(2, 70, 0)
+                if (g_pattern_spec_match_string(pat, rd->name) ||
+                    g_pattern_spec_match_string(pat, rd_lower)) {
+#else
                 if (g_pattern_match_string(pat, rd->name) ||
                     g_pattern_match_string(pat, rd_lower)) {
+#endif
                     Register *reg = init_vcpu_register(rd);
                     g_ptr_array_add(registers, reg);
 
@@ -336,7 +341,7 @@ static GPtrArray *registers_init(int vcpu_index)
                     if (disas_assist) {
                         g_mutex_lock(&add_reg_name_lock);
                         if (!g_ptr_array_find(all_reg_names, reg->name, NULL)) {
-                            g_ptr_array_add(all_reg_names, reg->name);
+                            g_ptr_array_add(all_reg_names, (gpointer)reg->name);
                         }
                         g_mutex_unlock(&add_reg_name_lock);
                     }
-- 
2.37.3


Re: [PATCH v2] contrib/plugins/execlog: Fix compiler warning
Posted by Pierrick Bouvier 1 month ago
On 3/25/24 10:06, Yao Xingtao wrote:
> 1. The g_pattern_match_string() is deprecated when glib2 version >= 2.70.
>     Use g_pattern_spec_match_string() instead to avoid this problem.
> 
> 2. The type of second parameter in g_ptr_array_add() is
>     'gpointer' {aka 'void *'}, but the type of reg->name is 'const char*'.
>     Cast the type of reg->name to 'gpointer' to avoid this problem.
> 
> compiler warning message:
> /root/qemu/contrib/plugins/execlog.c:330:17: warning: ‘g_pattern_match_string’
> is deprecated: Use 'g_pattern_spec_match_string'
> instead [-Wdeprecated-declarations]
>    330 |                 if (g_pattern_match_string(pat, rd->name) ||
>        |                 ^~
> In file included from /usr/include/glib-2.0/glib.h:67,
>                   from /root/qemu/contrib/plugins/execlog.c:9:
> /usr/include/glib-2.0/glib/gpattern.h:57:15: note: declared here
>     57 | gboolean      g_pattern_match_string   (GPatternSpec *pspec,
>        |               ^~~~~~~~~~~~~~~~~~~~~~
> /root/qemu/contrib/plugins/execlog.c:331:21: warning: ‘g_pattern_match_string’
> is deprecated: Use 'g_pattern_spec_match_string'
> instead [-Wdeprecated-declarations]
>    331 |                     g_pattern_match_string(pat, rd_lower)) {
>        |                     ^~~~~~~~~~~~~~~~~~~~~~
> /usr/include/glib-2.0/glib/gpattern.h:57:15: note: declared here
>     57 | gboolean      g_pattern_match_string   (GPatternSpec *pspec,
>        |               ^~~~~~~~~~~~~~~~~~~~~~
> /root/qemu/contrib/plugins/execlog.c:339:63: warning: passing argument 2 of
> ‘g_ptr_array_add’ discards ‘const’ qualifier from pointer target type
> [-Wdiscarded-qualifiers]
>    339 |                             g_ptr_array_add(all_reg_names, reg->name);
>        |                                                            ~~~^~~~~~
> In file included from /usr/include/glib-2.0/glib.h:33:
> /usr/include/glib-2.0/glib/garray.h:198:62: note: expected
> ‘gpointer’ {aka ‘void *’} but argument is of type ‘const char *’
>    198 |                                            gpointer          data);
>        |                                            ~~~~~~~~~~~~~~~~~~^~~~
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2210
> Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
> ---
>   contrib/plugins/execlog.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
> index a1dfd59ab7..09654910ee 100644
> --- a/contrib/plugins/execlog.c
> +++ b/contrib/plugins/execlog.c
> @@ -327,8 +327,13 @@ static GPtrArray *registers_init(int vcpu_index)
>               for (int p = 0; p < rmatches->len; p++) {
>                   g_autoptr(GPatternSpec) pat = g_pattern_spec_new(rmatches->pdata[p]);
>                   g_autofree gchar *rd_lower = g_utf8_strdown(rd->name, -1);
> +#if GLIB_CHECK_VERSION(2, 70, 0)
> +                if (g_pattern_spec_match_string(pat, rd->name) ||
> +                    g_pattern_spec_match_string(pat, rd_lower)) {
> +#else
>                   if (g_pattern_match_string(pat, rd->name) ||
>                       g_pattern_match_string(pat, rd_lower)) {
> +#endif
>                       Register *reg = init_vcpu_register(rd);
>                       g_ptr_array_add(registers, reg);
>   

As suggested by Peter on previous version, you can declare a new 
function `g_pattern_match_string_qemu` in include/glib-compat.h which 
abstract this.
You'll need to add include/ to Makefile as well, so glib-compat.h will 
be accessible to contrib plugins too.

> @@ -336,7 +341,7 @@ static GPtrArray *registers_init(int vcpu_index)
>                       if (disas_assist) {
>                           g_mutex_lock(&add_reg_name_lock);
>                           if (!g_ptr_array_find(all_reg_names, reg->name, NULL)) {
> -                            g_ptr_array_add(all_reg_names, reg->name);
> +                            g_ptr_array_add(all_reg_names, (gpointer)reg->name);
>                           }
>                           g_mutex_unlock(&add_reg_name_lock);
>                       }
Re: [PATCH v2] contrib/plugins/execlog: Fix compiler warning
Posted by Peter Maydell 1 month ago
On Mon, 25 Mar 2024 at 06:41, Pierrick Bouvier
<pierrick.bouvier@linaro.org> wrote:
>
> On 3/25/24 10:06, Yao Xingtao wrote:
> > diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
> > index a1dfd59ab7..09654910ee 100644
> > --- a/contrib/plugins/execlog.c
> > +++ b/contrib/plugins/execlog.c
> > @@ -327,8 +327,13 @@ static GPtrArray *registers_init(int vcpu_index)
> >               for (int p = 0; p < rmatches->len; p++) {
> >                   g_autoptr(GPatternSpec) pat = g_pattern_spec_new(rmatches->pdata[p]);
> >                   g_autofree gchar *rd_lower = g_utf8_strdown(rd->name, -1);
> > +#if GLIB_CHECK_VERSION(2, 70, 0)
> > +                if (g_pattern_spec_match_string(pat, rd->name) ||
> > +                    g_pattern_spec_match_string(pat, rd_lower)) {
> > +#else
> >                   if (g_pattern_match_string(pat, rd->name) ||
> >                       g_pattern_match_string(pat, rd_lower)) {
> > +#endif
> >                       Register *reg = init_vcpu_register(rd);
> >                       g_ptr_array_add(registers, reg);
> >
>
> As suggested by Peter on previous version, you can declare a new
> function `g_pattern_match_string_qemu` in include/glib-compat.h which
> abstract this.

We should have an abstraction function, but it should *not*
be in glib-compat.h, but local to this plugin's .c file. This is
because the plugins are deliberately standalone binaries which do not
rely on any of QEMU's include files or build process (you'll
see they don't use osdep.h, for example).

thanks
-- PMM
Re: [PATCH v2] contrib/plugins/execlog: Fix compiler warning
Posted by Pierrick Bouvier 1 month ago
On 3/25/24 13:58, Peter Maydell wrote:
> On Mon, 25 Mar 2024 at 06:41, Pierrick Bouvier
> <pierrick.bouvier@linaro.org> wrote:
>>
>> On 3/25/24 10:06, Yao Xingtao wrote:
>>> diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
>>> index a1dfd59ab7..09654910ee 100644
>>> --- a/contrib/plugins/execlog.c
>>> +++ b/contrib/plugins/execlog.c
>>> @@ -327,8 +327,13 @@ static GPtrArray *registers_init(int vcpu_index)
>>>                for (int p = 0; p < rmatches->len; p++) {
>>>                    g_autoptr(GPatternSpec) pat = g_pattern_spec_new(rmatches->pdata[p]);
>>>                    g_autofree gchar *rd_lower = g_utf8_strdown(rd->name, -1);
>>> +#if GLIB_CHECK_VERSION(2, 70, 0)
>>> +                if (g_pattern_spec_match_string(pat, rd->name) ||
>>> +                    g_pattern_spec_match_string(pat, rd_lower)) {
>>> +#else
>>>                    if (g_pattern_match_string(pat, rd->name) ||
>>>                        g_pattern_match_string(pat, rd_lower)) {
>>> +#endif
>>>                        Register *reg = init_vcpu_register(rd);
>>>                        g_ptr_array_add(registers, reg);
>>>
>>
>> As suggested by Peter on previous version, you can declare a new
>> function `g_pattern_match_string_qemu` in include/glib-compat.h which
>> abstract this.
> 
> We should have an abstraction function, but it should *not*
> be in glib-compat.h, but local to this plugin's .c file. This is
> because the plugins are deliberately standalone binaries which do not
> rely on any of QEMU's include files or build process (you'll
> see they don't use osdep.h, for example).
> 

Sorry, I misunderstood that, as it was discussed with Alex that maybe 
plugins should be able to access glib-compat.h.

> thanks
> -- PMM