Re: [PATCH-for-10.2] Fix order of function arguments

["Daniel P. Berrangé" <berrange@redhat.com>]

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 :|