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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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#submitting-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.

Regards,

Phil.