Re: [PATCH RFC 11/15] hw/virtio/vhost-user: create isolation region

Stefan Hajnoczi <[email protected]> Tue, 4 Aug 2026 14:23:40 -0400

Newsgroups	gmane.comp.emulators.qemu.block,gmane.comp.emulators.qemu
Message-ID	<20260804182340.GC488200@fedora>

On Mon, Aug 03, 2026 at 03:25:19PM +0200, Hanna Czenczek wrote:
> > +    for (int i = 0; i < buffer_regions->len; i++) {
> > +        map = &g_array_index(buffer_regions, DMAMap, i);
> 
> God, I *really*, *really* hate this, and find it really disgusting that the
> documentation actually recommends doing this (`&g_array_index()`) instead of
> just offering a separate macro to get a reference.
> 
> And existing qemu code does it all over the place, too.
> 
> So I cannot really fault you for it.
> 
> Still. Too ugly for me to keep completely silent about it.
> 
> </rant>

Do we need GArray here, maybe just allocate a C array with g_new()
instead?

The reason I didn't say anything was because I expected GArray to
perform bounds checking. Plain C arrays don't offer that. After reading
your comment and looking at the glib source, I realized it does not.
There is no way to handle an error and glib does not abort() when the
index is invalid.

GArray still has the advantage of allowing elements to be added
dynamically, but in this case we know the number of elements ahead of
time and don't need that feature.

Stefan

signature.asc (application/pgp-signature, 488 B)