Hi Pierrick, thanks for your reply, and I will modify and push the patch later.
thanks Xingtao > -----Original Message----- > From: Pierrick Bouvier <pierrick.bouv...@linaro.org> > Sent: Monday, March 25, 2024 12:31 PM > To: Yao, Xingtao/姚 幸涛 <yaoxt.f...@fujitsu.com>; Peter Maydell > <peter.mayd...@linaro.org> > Cc: alex.ben...@linaro.org; erdn...@crans.org; ma.mando...@gmail.com; > qemu-devel@nongnu.org > Subject: Re: [PATCH] contrib/plugins/execlog: Fix compiler warning > > On 3/25/24 07:00, Xingtao Yao (Fujitsu) wrote: > > 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? > > > > Hi Xingtao, > > you're doing the right thing here. In my original patch, there was no > consideration for backward compatibility, to the opposite of what you did. > > Alex will be out for several weeks, so it might take some time to get this > merged, > but I'm definitely voting for this 👍. > > Pierrick > > >> -----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