Re: [PATCH v3 0/5] introduce QArray

[Christian Schoenebeck <qemu_oss@crudebyte.com>]

On Dienstag, 28. September 2021 15:37:45 CEST Alex Bennée wrote:
> Christian Schoenebeck <qemu_oss@crudebyte.com> writes:
> > On Montag, 27. September 2021 12:59:40 CEST Greg Kurz wrote:
> >> On Mon, 27 Sep 2021 12:35:16 +0200
> >> 
> >> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> >> > On Dienstag, 31. August 2021 14:25:04 CEST Christian Schoenebeck wrote:
> >> > > On Dienstag, 31. August 2021 13:58:02 CEST Greg Kurz wrote:
> >> > > > On Thu, 26 Aug 2021 14:47:26 +0200
> >> > > > 
> >> > > > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> >> > > > > Patches 1 and 2 introduce include/qemu/qarray.h which implements
> >> > > > > a
> >> > > > > deep
> >> > > > > auto free mechanism for arrays. See commit log of patch 1 for a
> >> > > > > detailed
> >> > > > > explanation and motivation for introducing QArray.
> >> > > > > 
> >> > > > > Patches 3..5 are provided (e.g. as example) for 9p being the
> >> > > > > first
> >> > > > > user
> >> > > > > of
> >> > > > > this new QArray API. These particular patches 3..5 are rebased on
> >> > > > > my
> >> > > > > current 9p queue:
> >> > > > > https://github.com/cschoenebeck/qemu/commits/9p.next
> >> > > > 
> >> > > > > which are basically just the following two queued patches:
> >> > > > This looks nice indeed but I have the impression the same could be
> >> > > > achieved using glib's g_autoptr framework with less code being
> >> > > > added
> >> > > > to QEMU (at the cost of being less generic maybe).
> >> > > 
> >> > > I haven't seen a way doing this with glib, except of GArray which has
> >> > > some
> >> > > disadvantages. But who knows, maybe I was missing something.
> >> > 
> >> > Ping
> >> > 
> >> > Let's do this?
> >> 
> >> Hi Christian,
> >> 
> >> Sorry I don't have enough bandwidth to review or to look for an alternate
> >> way... :-\ So I suggest you just go forward with this series. Hopefully
> >> you can get some reviews from Markus and/or Richard.
> > 
> > Ok, then I wait for few more days, and if there are no repsonses, nor
> > vetos
> > then I'll queue these patches anyway.
> 
> As far as I can make out the main argument for introducing a QEMU
> specific array handler is to avoid needing to use g_array_index() to
> reference the elements of the array. This in itself doesn't seem enough
> justification to me.
> 
> I also see you handle deep frees which I admit is not something I've
> really done with GArray as usually I'm using it when I want to have
> everything local to each other.

The main motivation for this API is to simplify and reduce code, which also 
increases safety.

Most of the suggested header file are really just comments. If you strip those 
comments away, you actually only have a few lines of code left.

There are user code functions which are already complex enough on its own, 
where I don't want to additionally need to think about for *each* individual 
branch whether I need to free something here and there, and if yes what 
exactly, how deep, and whether there are any exceptions I have to take care 
about, etc.

Best regards,
Christian Schoenebeck