* [PATCH] contrib/plugins/execlog: Fix compiler warning
@ 2024-03-20 2:01 Yao Xingtao via
2024-03-22 11:50 ` Peter Maydell
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Yao Xingtao via @ 2024-03-20 2:01 UTC (permalink / raw)
To: alex.bennee, erdnaxe, ma.mandourr, pierrick.bouvier
Cc: qemu-devel, Yao Xingtao
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);
| ~~~~~~~~~~~~~~~~~~^~~~
Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
---
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)
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
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);
}
--
2.37.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] contrib/plugins/execlog: Fix compiler warning
2024-03-20 2:01 [PATCH] contrib/plugins/execlog: Fix compiler warning Yao Xingtao via
@ 2024-03-22 11:50 ` Peter Maydell
2024-03-25 3:00 ` Xingtao Yao (Fujitsu) via
2024-03-25 6:06 ` [PATCH v2] " Yao Xingtao via
2024-03-26 1:52 ` [PATCH v3] " Yao Xingtao via
2 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2024-03-22 11:50 UTC (permalink / raw)
To: Yao Xingtao
Cc: alex.bennee, erdnaxe, ma.mandourr, pierrick.bouvier, qemu-devel
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.fnst@fujitsu.com>
> ---
> 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
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);
> }
thanks
-- PMM
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH] contrib/plugins/execlog: Fix compiler warning
2024-03-22 11:50 ` Peter Maydell
@ 2024-03-25 3:00 ` Xingtao Yao (Fujitsu) via
2024-03-25 4:31 ` Pierrick Bouvier
0 siblings, 1 reply; 16+ messages in thread
From: Xingtao Yao (Fujitsu) via @ 2024-03-25 3:00 UTC (permalink / raw)
To: Peter Maydell, pierrick.bouvier
Cc: alex.bennee, erdnaxe, ma.mandourr, pierrick.bouvier, qemu-devel
Pete:
Thanks for your comment.
I also find a similar patch written by Pierrick: https://lore.kernel.org/qemu-devel/20240118032400.3762658-15-pierrick.bouvier@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.maydell@linaro.org>
> Sent: Friday, March 22, 2024 7:50 PM
> To: Yao, Xingtao/姚 幸涛 <yaoxt.fnst@fujitsu.com>
> Cc: alex.bennee@linaro.org; erdnaxe@crans.org; ma.mandourr@gmail.com;
> pierrick.bouvier@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.fnst@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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] contrib/plugins/execlog: Fix compiler warning
2024-03-25 3:00 ` Xingtao Yao (Fujitsu) via
@ 2024-03-25 4:31 ` Pierrick Bouvier
2024-03-25 5:55 ` Xingtao Yao (Fujitsu) via
0 siblings, 1 reply; 16+ messages in thread
From: Pierrick Bouvier @ 2024-03-25 4:31 UTC (permalink / raw)
To: Xingtao Yao (Fujitsu), Peter Maydell
Cc: alex.bennee, erdnaxe, ma.mandourr, qemu-devel
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.bouvier@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.maydell@linaro.org>
>> Sent: Friday, March 22, 2024 7:50 PM
>> To: Yao, Xingtao/姚 幸涛 <yaoxt.fnst@fujitsu.com>
>> Cc: alex.bennee@linaro.org; erdnaxe@crans.org; ma.mandourr@gmail.com;
>> pierrick.bouvier@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.fnst@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
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH] contrib/plugins/execlog: Fix compiler warning
2024-03-25 4:31 ` Pierrick Bouvier
@ 2024-03-25 5:55 ` Xingtao Yao (Fujitsu) via
0 siblings, 0 replies; 16+ messages in thread
From: Xingtao Yao (Fujitsu) via @ 2024-03-25 5:55 UTC (permalink / raw)
To: Pierrick Bouvier, Peter Maydell
Cc: alex.bennee, erdnaxe, ma.mandourr, qemu-devel
Hi Pierrick,
thanks for your reply, and I will modify and push the patch later.
thanks
Xingtao
> -----Original Message-----
> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Sent: Monday, March 25, 2024 12:31 PM
> To: Yao, Xingtao/姚 幸涛 <yaoxt.fnst@fujitsu.com>; Peter Maydell
> <peter.maydell@linaro.org>
> Cc: alex.bennee@linaro.org; erdnaxe@crans.org; ma.mandourr@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.
> > bouvier@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.maydell@linaro.org>
> >> Sent: Friday, March 22, 2024 7:50 PM
> >> To: Yao, Xingtao/姚 幸涛 <yaoxt.fnst@fujitsu.com>
> >> Cc: alex.bennee@linaro.org; erdnaxe@crans.org; ma.mandourr@gmail.com;
> >> pierrick.bouvier@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.fnst@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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2] contrib/plugins/execlog: Fix compiler warning
2024-03-20 2:01 [PATCH] contrib/plugins/execlog: Fix compiler warning Yao Xingtao via
2024-03-22 11:50 ` Peter Maydell
@ 2024-03-25 6:06 ` Yao Xingtao via
2024-03-25 6:41 ` Pierrick Bouvier
2024-03-26 1:52 ` [PATCH v3] " Yao Xingtao via
2 siblings, 1 reply; 16+ messages in thread
From: Yao Xingtao via @ 2024-03-25 6:06 UTC (permalink / raw)
To: qemu-devel
Cc: alex.bennee, erdnaxe, ma.mandourr, pierrick.bouvier,
peter.maydell, yaoxt.fnst
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 | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
index a1dfd59ab7..09654910ee 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_CHECK_VERSION(2, 70, 0)
+ if (g_pattern_spec_match_string(pat, rd->name) ||
+ g_pattern_spec_match_string(pat, rd_lower)) {
+#else
if (g_pattern_match_string(pat, rd->name) ||
g_pattern_match_string(pat, rd_lower)) {
+#endif
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);
}
--
2.37.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2] contrib/plugins/execlog: Fix compiler warning
2024-03-25 6:06 ` [PATCH v2] " Yao Xingtao via
@ 2024-03-25 6:41 ` Pierrick Bouvier
2024-03-25 9:58 ` Peter Maydell
0 siblings, 1 reply; 16+ messages in thread
From: Pierrick Bouvier @ 2024-03-25 6:41 UTC (permalink / raw)
To: Yao Xingtao, qemu-devel; +Cc: alex.bennee, erdnaxe, ma.mandourr, peter.maydell
On 3/25/24 10:06, 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 | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
> index a1dfd59ab7..09654910ee 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_CHECK_VERSION(2, 70, 0)
> + if (g_pattern_spec_match_string(pat, rd->name) ||
> + g_pattern_spec_match_string(pat, rd_lower)) {
> +#else
> if (g_pattern_match_string(pat, rd->name) ||
> g_pattern_match_string(pat, rd_lower)) {
> +#endif
> Register *reg = init_vcpu_register(rd);
> g_ptr_array_add(registers, reg);
>
As suggested by Peter on previous version, you can declare a new
function `g_pattern_match_string_qemu` in include/glib-compat.h which
abstract this.
You'll need to add include/ to Makefile as well, so glib-compat.h will
be accessible to contrib plugins too.
> @@ -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);
> }
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] contrib/plugins/execlog: Fix compiler warning
2024-03-25 6:41 ` Pierrick Bouvier
@ 2024-03-25 9:58 ` Peter Maydell
2024-03-25 10:16 ` Pierrick Bouvier
0 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2024-03-25 9:58 UTC (permalink / raw)
To: Pierrick Bouvier
Cc: Yao Xingtao, qemu-devel, alex.bennee, erdnaxe, ma.mandourr
On Mon, 25 Mar 2024 at 06:41, Pierrick Bouvier
<pierrick.bouvier@linaro.org> wrote:
>
> On 3/25/24 10:06, Yao Xingtao wrote:
> > diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
> > index a1dfd59ab7..09654910ee 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_CHECK_VERSION(2, 70, 0)
> > + if (g_pattern_spec_match_string(pat, rd->name) ||
> > + g_pattern_spec_match_string(pat, rd_lower)) {
> > +#else
> > if (g_pattern_match_string(pat, rd->name) ||
> > g_pattern_match_string(pat, rd_lower)) {
> > +#endif
> > Register *reg = init_vcpu_register(rd);
> > g_ptr_array_add(registers, reg);
> >
>
> As suggested by Peter on previous version, you can declare a new
> function `g_pattern_match_string_qemu` in include/glib-compat.h which
> abstract this.
We should have an abstraction function, but it should *not*
be in glib-compat.h, but local to this plugin's .c file. This is
because the plugins are deliberately standalone binaries which do not
rely on any of QEMU's include files or build process (you'll
see they don't use osdep.h, for example).
thanks
-- PMM
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] contrib/plugins/execlog: Fix compiler warning
2024-03-25 9:58 ` Peter Maydell
@ 2024-03-25 10:16 ` Pierrick Bouvier
0 siblings, 0 replies; 16+ messages in thread
From: Pierrick Bouvier @ 2024-03-25 10:16 UTC (permalink / raw)
To: Peter Maydell; +Cc: Yao Xingtao, qemu-devel, alex.bennee, erdnaxe, ma.mandourr
On 3/25/24 13:58, Peter Maydell wrote:
> On Mon, 25 Mar 2024 at 06:41, Pierrick Bouvier
> <pierrick.bouvier@linaro.org> wrote:
>>
>> On 3/25/24 10:06, Yao Xingtao wrote:
>>> diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
>>> index a1dfd59ab7..09654910ee 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_CHECK_VERSION(2, 70, 0)
>>> + if (g_pattern_spec_match_string(pat, rd->name) ||
>>> + g_pattern_spec_match_string(pat, rd_lower)) {
>>> +#else
>>> if (g_pattern_match_string(pat, rd->name) ||
>>> g_pattern_match_string(pat, rd_lower)) {
>>> +#endif
>>> Register *reg = init_vcpu_register(rd);
>>> g_ptr_array_add(registers, reg);
>>>
>>
>> As suggested by Peter on previous version, you can declare a new
>> function `g_pattern_match_string_qemu` in include/glib-compat.h which
>> abstract this.
>
> We should have an abstraction function, but it should *not*
> be in glib-compat.h, but local to this plugin's .c file. This is
> because the plugins are deliberately standalone binaries which do not
> rely on any of QEMU's include files or build process (you'll
> see they don't use osdep.h, for example).
>
Sorry, I misunderstood that, as it was discussed with Alex that maybe
plugins should be able to access glib-compat.h.
> thanks
> -- PMM
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3] contrib/plugins/execlog: Fix compiler warning
2024-03-20 2:01 [PATCH] contrib/plugins/execlog: Fix compiler warning Yao Xingtao via
2024-03-22 11:50 ` Peter Maydell
2024-03-25 6:06 ` [PATCH v2] " Yao Xingtao via
@ 2024-03-26 1:52 ` Yao Xingtao via
2024-03-26 3:33 ` Pierrick Bouvier
2 siblings, 1 reply; 16+ messages in thread
From: Yao Xingtao via @ 2024-03-26 1:52 UTC (permalink / raw)
To: qemu-devel
Cc: alex.bennee, erdnaxe, ma.mandourr, pierrick.bouvier,
peter.maydell, yaoxt.fnst
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);
}
--
2.37.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3] contrib/plugins/execlog: Fix compiler warning
2024-03-26 1:52 ` [PATCH v3] " Yao Xingtao via
@ 2024-03-26 3:33 ` Pierrick Bouvier
2024-03-26 9:54 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 16+ messages in thread
From: Pierrick Bouvier @ 2024-03-26 3:33 UTC (permalink / raw)
To: Yao Xingtao, qemu-devel; +Cc: alex.bennee, erdnaxe, ma.mandourr, peter.maydell
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.
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] contrib/plugins/execlog: Fix compiler warning
2024-03-26 3:33 ` Pierrick Bouvier
@ 2024-03-26 9:54 ` Philippe Mathieu-Daudé
2024-03-26 10:33 ` Peter Maydell
2024-03-26 12:38 ` Pierrick Bouvier
0 siblings, 2 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-03-26 9:54 UTC (permalink / raw)
To: Pierrick Bouvier, Yao Xingtao, qemu-devel
Cc: alex.bennee, erdnaxe, ma.mandourr, peter.maydell
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.
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] contrib/plugins/execlog: Fix compiler warning
2024-03-26 9:54 ` Philippe Mathieu-Daudé
@ 2024-03-26 10:33 ` Peter Maydell
2024-03-26 12:03 ` Philippe Mathieu-Daudé
2024-03-26 12:38 ` Pierrick Bouvier
1 sibling, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2024-03-26 10:33 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Pierrick Bouvier, Yao Xingtao, qemu-devel, alex.bennee, erdnaxe,
ma.mandourr
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] contrib/plugins/execlog: Fix compiler warning
2024-03-26 10:33 ` Peter Maydell
@ 2024-03-26 12:03 ` Philippe Mathieu-Daudé
2024-03-27 0:21 ` Xingtao Yao (Fujitsu) via
0 siblings, 1 reply; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-03-26 12:03 UTC (permalink / raw)
To: Peter Maydell
Cc: Pierrick Bouvier, Yao Xingtao, qemu-devel, alex.bennee, erdnaxe,
ma.mandourr
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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] contrib/plugins/execlog: Fix compiler warning
2024-03-26 9:54 ` Philippe Mathieu-Daudé
2024-03-26 10:33 ` Peter Maydell
@ 2024-03-26 12:38 ` Pierrick Bouvier
1 sibling, 0 replies; 16+ messages in thread
From: Pierrick Bouvier @ 2024-03-26 12:38 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Yao Xingtao, qemu-devel
Cc: alex.bennee, erdnaxe, ma.mandourr, peter.maydell
On 3/26/24 13:54, Philippe Mathieu-Daudé wrote:
>
> I will post a small PR later today, so until Peter has something
> else planned, I can take it, since the patch LGTM now.
>
Thanks Philippe :)
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v3] contrib/plugins/execlog: Fix compiler warning
2024-03-26 12:03 ` Philippe Mathieu-Daudé
@ 2024-03-27 0:21 ` Xingtao Yao (Fujitsu) via
0 siblings, 0 replies; 16+ messages in thread
From: Xingtao Yao (Fujitsu) via @ 2024-03-27 0:21 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Peter Maydell
Cc: Pierrick Bouvier, qemu-devel, alex.bennee, erdnaxe, ma.mandourr
> -----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.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2024-03-27 0:22 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-20 2:01 [PATCH] contrib/plugins/execlog: Fix compiler warning Yao Xingtao via
2024-03-22 11:50 ` Peter Maydell
2024-03-25 3:00 ` Xingtao Yao (Fujitsu) via
2024-03-25 4:31 ` Pierrick Bouvier
2024-03-25 5:55 ` Xingtao Yao (Fujitsu) via
2024-03-25 6:06 ` [PATCH v2] " Yao Xingtao via
2024-03-25 6:41 ` Pierrick Bouvier
2024-03-25 9:58 ` Peter Maydell
2024-03-25 10:16 ` Pierrick Bouvier
2024-03-26 1:52 ` [PATCH v3] " Yao Xingtao via
2024-03-26 3:33 ` Pierrick Bouvier
2024-03-26 9:54 ` Philippe Mathieu-Daudé
2024-03-26 10:33 ` Peter Maydell
2024-03-26 12:03 ` Philippe Mathieu-Daudé
2024-03-27 0:21 ` Xingtao Yao (Fujitsu) via
2024-03-26 12:38 ` Pierrick Bouvier
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.