Pete:
Thanks for your comment.
I also find a similar patch written by Pierrick:
https://lore.kernel.org/qemu-devel/20240118032400.3762658-15-pierrick.bouv...@linaro.org/
but for some reason, the patch was not merged yet.
shall I need to continue tracking the fixes of this bug?
> -----Original Message-----
> From: Peter Maydell <peter.mayd...@linaro.org>
> Sent: Friday, March 22, 2024 7:50 PM
> To: Yao, Xingtao/姚 幸涛 <yaoxt.f...@fujitsu.com>
> Cc: alex.ben...@linaro.org; erdn...@crans.org; ma.mando...@gmail.com;
> pierrick.bouv...@linaro.org; qemu-devel@nongnu.org
> Subject: Re: [PATCH] contrib/plugins/execlog: Fix compiler warning
>
> On Wed, 20 Mar 2024 at 02:05, Yao Xingtao via <qemu-devel@nongnu.org>
> 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);
> > |
> ~~~~~~~~~~~~~~~~~~^~~~
> >
>
> Hi; thanks for this patch.
>
> This fixes a bug reported in the bug tracker so we should put
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2210
>
> in the commit message just above your signed-off-by tag.
>
>
> > Signed-off-by: Yao Xingtao <yaoxt.f...@fujitsu.com>
I will if needed.
> > ---
> > 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..41f6774116 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_VERSION_MAX_ALLOWED < G_ENCODE_VERSION(2, 70)
>
> Elsewhere we do glib version checks with
>
> #if GLIB_CHECK_VERSION(2, 70, 0)
> code for 2.70.0 and up;
> #else
> code for older versions
> #endif
>
> so I think we should probably do that here too.
>
> > if (g_pattern_match_string(pat, rd->name) ||
> > g_pattern_match_string(pat, rd_lower)) {
> > +#else
> > + if (g_pattern_spec_match_string(pat, rd->name) ||
> > + g_pattern_spec_match_string(pat, rd_lower)) {
> > +#endif
thanks, I got it.
>
> Rather than putting this ifdef in the middle of this function, I think it
> would be
> easier to read if we abstract out a function which does the pattern matching
> and whose body calls the right glib function depending on glib version. We
> generally call these functions the same as the glib function but with a _qemu
> suffix (compare the ones in include/glib-compat.h), so here that would be
> g_pattern_spec_match_string_qemu().
>
> > 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);
> > }
>
I think it's not worth adding this to glib-compat.h too.
> thanks
> -- PMM
thanks
Xingtao
