All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: Christian Schoenebeck <qemu_oss@crudebyte.com>
Cc: qemu-devel@nongnu.org,
	Richard Henderson <richard.henderson@linaro.org>,
	Greg Kurz <groug@kaod.org>, Markus Armbruster <armbru@redhat.com>
Subject: Re: [PATCH v3 0/5] introduce QArray
Date: Tue, 28 Sep 2021 14:37:45 +0100	[thread overview]
Message-ID: <87v92k7qyd.fsf@linaro.org> (raw)
In-Reply-To: <1811981.VuayHpH01O@silver>


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.

>
> Best regards,
> Christian Schoenebeck


-- 
Alex Bennée


  reply	other threads:[~2021-09-28 13:50 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-26 12:47 [PATCH v3 0/5] introduce QArray Christian Schoenebeck
2021-08-26 12:30 ` [PATCH v3 1/5] qemu/qarray.h: " Christian Schoenebeck
2021-08-26 12:31 ` [PATCH v3 2/5] qemu/qarray.h: check scalar type in QARRAY_CREATE() Christian Schoenebeck
2021-08-26 12:31 ` [PATCH v3 3/5] 9pfs: make V9fsString usable via QArray API Christian Schoenebeck
2021-08-26 12:31 ` [PATCH v3 4/5] 9pfs: make V9fsPath " Christian Schoenebeck
2021-08-26 12:32 ` [PATCH v3 5/5] 9pfs: use QArray in v9fs_walk() Christian Schoenebeck
2021-08-31 11:58 ` [PATCH v3 0/5] introduce QArray Greg Kurz
2021-08-31 12:25   ` Christian Schoenebeck
2021-09-27 10:35     ` Christian Schoenebeck
2021-09-27 10:59       ` Greg Kurz
2021-09-28 12:25         ` Christian Schoenebeck
2021-09-28 13:37           ` Alex Bennée [this message]
2021-09-28 16:37             ` Christian Schoenebeck
2021-09-28 14:17           ` Daniel P. Berrangé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87v92k7qyd.fsf@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=armbru@redhat.com \
    --cc=groug@kaod.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu_oss@crudebyte.com \
    --cc=richard.henderson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.