RE: [PATCH v3] contrib/plugins/execlog: Fix compiler warning

[Xingtao Yao (Fujitsu)]

> -----Original Message-----
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Sent: Tuesday, March 26, 2024 8:04 PM
> To: Peter Maydell <peter.maydell@linaro.org>
> Cc: Pierrick Bouvier <pierrick.bouvier@linaro.org>; Yao, Xingtao/姚 幸涛
> <yaoxt.fnst@fujitsu.com>; qemu-devel@nongnu.org; alex.bennee@linaro.org;
> erdnaxe@crans.org; ma.mandourr@gmail.com
> Subject: Re: [PATCH v3] contrib/plugins/execlog: Fix compiler warning
> 
> On 26/3/24 11:33, Peter Maydell wrote:
> > On Tue, 26 Mar 2024 at 09:54, Philippe Mathieu-Daudé <philmd@linaro.org>
> wrote:
> >>
> >> On 26/3/24 04:33, Pierrick Bouvier wrote:
> >>> On 3/26/24 05:52, 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 | 24 +++++++++++++++++++++---
> >>>>    1 file changed, 21 insertions(+), 3 deletions(-)
> >>>>
> >>>> diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
> >>>> index a1dfd59ab7..fab18113d4 100644
> >>>> --- a/contrib/plugins/execlog.c
> >>>> +++ b/contrib/plugins/execlog.c
> >>>> @@ -311,6 +311,24 @@ static Register
> >>>> *init_vcpu_register(qemu_plugin_reg_descriptor *desc)
> >>>>        return reg;
> >>>>    }
> >>>> +/*
> >>>> + * g_pattern_match_string has been deprecated in Glib since 2.70
> >>>> +and
> >>>> + * will complain about it if you try to use it. Fortunately the
> >>>> + * signature of both functions is the same making it easy to work
> >>>> + * around.
> >>>> + */
> >>>> +static inline
> >>>> +gboolean g_pattern_spec_match_string_qemu(GPatternSpec *pspec,
> >>>> +                                          const gchar *string) {
> >>>> +#if GLIB_CHECK_VERSION(2, 70, 0)
> >>>> +    return g_pattern_spec_match_string(pspec, string); #else
> >>>> +    return g_pattern_match_string(pspec, string); #endif };
> >>>> +#define g_pattern_spec_match_string(p, s)
> >>>> g_pattern_spec_match_string_qemu(p, s)
> >>>> +
> >>>>    static GPtrArray *registers_init(int vcpu_index)
> >>>>    {
> >>>>        g_autoptr(GPtrArray) registers = g_ptr_array_new(); @@
> >>>> -327,8 +345,8 @@ 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 (g_pattern_match_string(pat, rd->name) ||
> >>>> -                    g_pattern_match_string(pat, rd_lower)) {
> >>>> +                if (g_pattern_spec_match_string(pat, rd->name) ||
> >>>> +                    g_pattern_spec_match_string(pat, rd_lower)) {
> >>>>                        Register *reg = init_vcpu_register(rd);
> >>>>                        g_ptr_array_add(registers, reg); @@ -336,7
> >>>> +354,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);
> >>>>                        }
> >>>
> >>> Would be nice if it's still possible to merge this in 9.0 Peter.
> >>
> >> I will post a small PR later today, so until Peter has something else
> >> planned, I can take it, since the patch LGTM now.
> >
> > That would be great (I don't have any more patches I wanted to put in
> > a PR).
> >
> > Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> OK, patch queued then.
> 
> Yao, for your future contributions, please post patch iterations as new thread
> rather than replying to previous versions. You can see tips here:
> https://www.qemu.org/docs/master/devel/submitting-a-patch.html#submit
> ting-your-patches
> in particular:
> 
>    Patches are easier to find if they start a new top-level thread,
>    rather than being buried in-reply-to another existing thread.
> 
thanks, I will.
> Regards,
> 
> Phil.