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

[Peter Maydell <peter.maydell@linaro.org>]

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>

thanks
-- PMM