qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [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).