* [PATCH-for-10.2] Fix order of function arguments
@ 2025-12-09 12:50 Stefan Weil via
2025-12-09 12:59 ` Laurent Vivier
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Stefan Weil via @ 2025-12-09 12:50 UTC (permalink / raw)
To: Peter Xu, Fabiano Rosas; +Cc: qemu-devel, qemu-trivial, Stefan Weil
This fixes a compiler error when higher warning levels are enabled:
../migration/postcopy-ram.c: In function ‘postcopy_temp_pages_setup’:
../migration/postcopy-ram.c:1483:50: error: ‘g_malloc0_n’ sizes specified with ‘sizeof’ in the earlier argument and not in the later argument [-Werror=calloc-transposed-args]
1483 | mis->postcopy_tmp_pages = g_malloc0_n(sizeof(PostcopyTmpPage), channels);
| ^~~~~~~~~~~~~~~
../migration/postcopy-ram.c:1483:50: note: earlier argument should specify number of elements, later size of each element
Avoid also a related int/unsigned mismatch by fixing the type of
two local variables.
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
migration/postcopy-ram.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 3f98dcb6fd..8bef0192aa 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -1467,7 +1467,8 @@ retry:
static int postcopy_temp_pages_setup(MigrationIncomingState *mis)
{
PostcopyTmpPage *tmp_page;
- int err, i, channels;
+ int err;
+ unsigned i, channels;
void *temp_page;
if (migrate_postcopy_preempt()) {
@@ -1479,7 +1480,7 @@ static int postcopy_temp_pages_setup(MigrationIncomingState *mis)
}
channels = mis->postcopy_channels;
- mis->postcopy_tmp_pages = g_malloc0_n(sizeof(PostcopyTmpPage), channels);
+ mis->postcopy_tmp_pages = g_malloc0_n(channels, sizeof(PostcopyTmpPage));
for (i = 0; i < channels; i++) {
tmp_page = &mis->postcopy_tmp_pages[i];
--
2.47.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH-for-10.2] Fix order of function arguments
2025-12-09 12:50 [PATCH-for-10.2] Fix order of function arguments Stefan Weil via
@ 2025-12-09 12:59 ` Laurent Vivier
2025-12-09 13:03 ` Philippe Mathieu-Daudé
2025-12-09 19:03 ` Peter Xu
2 siblings, 0 replies; 7+ messages in thread
From: Laurent Vivier @ 2025-12-09 12:59 UTC (permalink / raw)
To: Stefan Weil; +Cc: Peter Xu, Fabiano Rosas, qemu-devel, qemu-trivial
On 12/9/25 13:50, Stefan Weil via wrote:
> This fixes a compiler error when higher warning levels are enabled:
>
> ../migration/postcopy-ram.c: In function 'postcopy_temp_pages_setup':
> ../migration/postcopy-ram.c:1483:50: error: 'g_malloc0_n' sizes specified with 'sizeof' in the earlier argument and not in the later argument [-Werror=calloc-transposed-args]
> 1483 | mis->postcopy_tmp_pages = g_malloc0_n(sizeof(PostcopyTmpPage), channels);
> | ^~~~~~~~~~~~~~~
> ../migration/postcopy-ram.c:1483:50: note: earlier argument should specify number of elements, later size of each element
>
> Avoid also a related int/unsigned mismatch by fixing the type of
> two local variables.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> migration/postcopy-ram.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH-for-10.2] Fix order of function arguments
2025-12-09 12:50 [PATCH-for-10.2] Fix order of function arguments Stefan Weil via
2025-12-09 12:59 ` Laurent Vivier
@ 2025-12-09 13:03 ` Philippe Mathieu-Daudé
2025-12-09 13:09 ` Daniel P. Berrangé
2025-12-09 19:03 ` Peter Xu
2 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-12-09 13:03 UTC (permalink / raw)
To: Stefan Weil, Peter Xu, Fabiano Rosas
Cc: qemu-devel, qemu-trivial, Daniel P. Berrangé
On 9/12/25 13:50, Stefan Weil via wrote:
> This fixes a compiler error when higher warning levels are enabled:
>
> ../migration/postcopy-ram.c: In function ‘postcopy_temp_pages_setup’:
> ../migration/postcopy-ram.c:1483:50: error: ‘g_malloc0_n’ sizes specified with ‘sizeof’ in the earlier argument and not in the later argument [-Werror=calloc-transposed-args]
> 1483 | mis->postcopy_tmp_pages = g_malloc0_n(sizeof(PostcopyTmpPage), channels);
> | ^~~~~~~~~~~~~~~
> ../migration/postcopy-ram.c:1483:50: note: earlier argument should specify number of elements, later size of each element
>
> Avoid also a related int/unsigned mismatch by fixing the type of
> two local variables.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> migration/postcopy-ram.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 3f98dcb6fd..8bef0192aa 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -1467,7 +1467,8 @@ retry:
> static int postcopy_temp_pages_setup(MigrationIncomingState *mis)
> {
> PostcopyTmpPage *tmp_page;
> - int err, i, channels;
> + int err;
> + unsigned i, channels;
> void *temp_page;
>
> if (migrate_postcopy_preempt()) {
> @@ -1479,7 +1480,7 @@ static int postcopy_temp_pages_setup(MigrationIncomingState *mis)
> }
>
> channels = mis->postcopy_channels;
> - mis->postcopy_tmp_pages = g_malloc0_n(sizeof(PostcopyTmpPage), channels);
> + mis->postcopy_tmp_pages = g_malloc0_n(channels, sizeof(PostcopyTmpPage));
I suppose we wanted to use:
g_new0(PostcopyTmpPage, channels)
What is the benefit of g_malloc0_n() over g_new0()?
>
> for (i = 0; i < channels; i++) {
> tmp_page = &mis->postcopy_tmp_pages[i];
Anyhow,
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH-for-10.2] Fix order of function arguments
2025-12-09 13:03 ` Philippe Mathieu-Daudé
@ 2025-12-09 13:09 ` Daniel P. Berrangé
2025-12-09 14:17 ` Philippe Mathieu-Daudé
2025-12-09 21:17 ` Stefan Weil via
0 siblings, 2 replies; 7+ messages in thread
From: Daniel P. Berrangé @ 2025-12-09 13:09 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Stefan Weil, Peter Xu, Fabiano Rosas, qemu-devel, qemu-trivial
On Tue, Dec 09, 2025 at 02:03:27PM +0100, Philippe Mathieu-Daudé wrote:
> On 9/12/25 13:50, Stefan Weil via wrote:
> > This fixes a compiler error when higher warning levels are enabled:
> >
> > ../migration/postcopy-ram.c: In function ‘postcopy_temp_pages_setup’:
> > ../migration/postcopy-ram.c:1483:50: error: ‘g_malloc0_n’ sizes specified with ‘sizeof’ in the earlier argument and not in the later argument [-Werror=calloc-transposed-args]
> > 1483 | mis->postcopy_tmp_pages = g_malloc0_n(sizeof(PostcopyTmpPage), channels);
> > | ^~~~~~~~~~~~~~~
> > ../migration/postcopy-ram.c:1483:50: note: earlier argument should specify number of elements, later size of each element
> >
> > Avoid also a related int/unsigned mismatch by fixing the type of
> > two local variables.
> >
> > Signed-off-by: Stefan Weil <sw@weilnetz.de>
> > ---
> > migration/postcopy-ram.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> > index 3f98dcb6fd..8bef0192aa 100644
> > --- a/migration/postcopy-ram.c
> > +++ b/migration/postcopy-ram.c
> > @@ -1467,7 +1467,8 @@ retry:
> > static int postcopy_temp_pages_setup(MigrationIncomingState *mis)
> > {
> > PostcopyTmpPage *tmp_page;
> > - int err, i, channels;
> > + int err;
> > + unsigned i, channels;
> > void *temp_page;
> > if (migrate_postcopy_preempt()) {
> > @@ -1479,7 +1480,7 @@ static int postcopy_temp_pages_setup(MigrationIncomingState *mis)
> > }
> > channels = mis->postcopy_channels;
> > - mis->postcopy_tmp_pages = g_malloc0_n(sizeof(PostcopyTmpPage), channels);
> > + mis->postcopy_tmp_pages = g_malloc0_n(channels, sizeof(PostcopyTmpPage));
>
> I suppose we wanted to use:
>
> g_new0(PostcopyTmpPage, channels)
>
> What is the benefit of g_malloc0_n() over g_new0()?
g_malloc0_n should be used only in scenarios where you do NOT have a type
you can pass to g_new0. For example in the audio code
cap->buf = g_malloc0_n(hw->mix_buf.size, hw->info.bytes_per_frame);
any calls to g_malloc0_n which use sizeof(some-type) in an arg are
code style laws IMHO and should be changed to g_new0.
Or to put it another way. g_new0 must always be used/preferred except
in the handful of cases where it isn't possible.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH-for-10.2] Fix order of function arguments
2025-12-09 13:09 ` Daniel P. Berrangé
@ 2025-12-09 14:17 ` Philippe Mathieu-Daudé
2025-12-09 21:17 ` Stefan Weil via
1 sibling, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-12-09 14:17 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Stefan Weil, Peter Xu, Fabiano Rosas, qemu-devel, qemu-trivial
On 9/12/25 14:09, Daniel P. Berrangé wrote:
> On Tue, Dec 09, 2025 at 02:03:27PM +0100, Philippe Mathieu-Daudé wrote:
>> On 9/12/25 13:50, Stefan Weil via wrote:
>>> This fixes a compiler error when higher warning levels are enabled:
>>>
>>> ../migration/postcopy-ram.c: In function ‘postcopy_temp_pages_setup’:
>>> ../migration/postcopy-ram.c:1483:50: error: ‘g_malloc0_n’ sizes specified with ‘sizeof’ in the earlier argument and not in the later argument [-Werror=calloc-transposed-args]
>>> 1483 | mis->postcopy_tmp_pages = g_malloc0_n(sizeof(PostcopyTmpPage), channels);
>>> | ^~~~~~~~~~~~~~~
>>> ../migration/postcopy-ram.c:1483:50: note: earlier argument should specify number of elements, later size of each element
>>>
>>> Avoid also a related int/unsigned mismatch by fixing the type of
>>> two local variables.
>>>
>>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>>> ---
>>> migration/postcopy-ram.c | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
>>> index 3f98dcb6fd..8bef0192aa 100644
>>> --- a/migration/postcopy-ram.c
>>> +++ b/migration/postcopy-ram.c
>>> @@ -1467,7 +1467,8 @@ retry:
>>> static int postcopy_temp_pages_setup(MigrationIncomingState *mis)
>>> {
>>> PostcopyTmpPage *tmp_page;
>>> - int err, i, channels;
>>> + int err;
>>> + unsigned i, channels;
>>> void *temp_page;
>>> if (migrate_postcopy_preempt()) {
>>> @@ -1479,7 +1480,7 @@ static int postcopy_temp_pages_setup(MigrationIncomingState *mis)
>>> }
>>> channels = mis->postcopy_channels;
>>> - mis->postcopy_tmp_pages = g_malloc0_n(sizeof(PostcopyTmpPage), channels);
>>> + mis->postcopy_tmp_pages = g_malloc0_n(channels, sizeof(PostcopyTmpPage));
>>
>> I suppose we wanted to use:
>>
>> g_new0(PostcopyTmpPage, channels)
>>
>> What is the benefit of g_malloc0_n() over g_new0()?
>
> g_malloc0_n should be used only in scenarios where you do NOT have a type
> you can pass to g_new0. For example in the audio code
>
> cap->buf = g_malloc0_n(hw->mix_buf.size, hw->info.bytes_per_frame);
>
> any calls to g_malloc0_n which use sizeof(some-type) in an arg are
> code style laws IMHO and should be changed to g_new0.
>
> Or to put it another way. g_new0 must always be used/preferred except
> in the handful of cases where it isn't possible.
Thanks (you confirmed my view).
Regards,
Phil.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH-for-10.2] Fix order of function arguments
2025-12-09 12:50 [PATCH-for-10.2] Fix order of function arguments Stefan Weil via
2025-12-09 12:59 ` Laurent Vivier
2025-12-09 13:03 ` Philippe Mathieu-Daudé
@ 2025-12-09 19:03 ` Peter Xu
2 siblings, 0 replies; 7+ messages in thread
From: Peter Xu @ 2025-12-09 19:03 UTC (permalink / raw)
To: Stefan Weil; +Cc: Fabiano Rosas, qemu-devel, qemu-trivial
On Tue, Dec 09, 2025 at 01:50:49PM +0100, Stefan Weil via wrote:
> This fixes a compiler error when higher warning levels are enabled:
>
> ../migration/postcopy-ram.c: In function ‘postcopy_temp_pages_setup’:
> ../migration/postcopy-ram.c:1483:50: error: ‘g_malloc0_n’ sizes specified with ‘sizeof’ in the earlier argument and not in the later argument [-Werror=calloc-transposed-args]
> 1483 | mis->postcopy_tmp_pages = g_malloc0_n(sizeof(PostcopyTmpPage), channels);
> | ^~~~~~~~~~~~~~~
> ../migration/postcopy-ram.c:1483:50: note: earlier argument should specify number of elements, later size of each element
>
> Avoid also a related int/unsigned mismatch by fixing the type of
> two local variables.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> migration/postcopy-ram.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 3f98dcb6fd..8bef0192aa 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -1467,7 +1467,8 @@ retry:
> static int postcopy_temp_pages_setup(MigrationIncomingState *mis)
> {
> PostcopyTmpPage *tmp_page;
> - int err, i, channels;
> + int err;
> + unsigned i, channels;
> void *temp_page;
>
> if (migrate_postcopy_preempt()) {
> @@ -1479,7 +1480,7 @@ static int postcopy_temp_pages_setup(MigrationIncomingState *mis)
> }
>
> channels = mis->postcopy_channels;
> - mis->postcopy_tmp_pages = g_malloc0_n(sizeof(PostcopyTmpPage), channels);
> + mis->postcopy_tmp_pages = g_malloc0_n(channels, sizeof(PostcopyTmpPage));
>
> for (i = 0; i < channels; i++) {
> tmp_page = &mis->postcopy_tmp_pages[i];
> --
> 2.47.3
>
Thanks for the patch, I'll wait for v2 per others' comments (or if Laurent
would pick it up).
Please also consider adding a prefix ("migration:") the subject.
--
Peter Xu
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH-for-10.2] Fix order of function arguments
2025-12-09 13:09 ` Daniel P. Berrangé
2025-12-09 14:17 ` Philippe Mathieu-Daudé
@ 2025-12-09 21:17 ` Stefan Weil via
1 sibling, 0 replies; 7+ messages in thread
From: Stefan Weil via @ 2025-12-09 21:17 UTC (permalink / raw)
To: Daniel P. Berrangé, Philippe Mathieu-Daudé
Cc: Peter Xu, Fabiano Rosas, qemu-devel
Am 09.12.25 um 14:09 schrieb Daniel P. Berrangé:
> g_malloc0_n should be used only in scenarios where you do NOT have a type
> you can pass to g_new0. For example in the audio code
>
> cap->buf = g_malloc0_n(hw->mix_buf.size, hw->info.bytes_per_frame);
>
> any calls to g_malloc0_n which use sizeof(some-type) in an arg are
> code style laws IMHO and should be changed to g_new0.
>
> Or to put it another way. g_new0 must always be used/preferred except
> in the handful of cases where it isn't possible.
There is another use of g_malloc0_n which might be replaced by g_new0 at
least in hw/9pfs/9p.c.
Regards,
Stefan
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-12-09 21:18 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-09 12:50 [PATCH-for-10.2] Fix order of function arguments Stefan Weil via
2025-12-09 12:59 ` Laurent Vivier
2025-12-09 13:03 ` Philippe Mathieu-Daudé
2025-12-09 13:09 ` Daniel P. Berrangé
2025-12-09 14:17 ` Philippe Mathieu-Daudé
2025-12-09 21:17 ` Stefan Weil via
2025-12-09 19:03 ` Peter Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).