All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: "Alex Bennée" <alex.bennee@linaro.org>
Cc: "Steven Sistare" <steven.sistare@oracle.com>,
	"Marc-André Lureau" <marcandre.lureau@gmail.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	"Michael Roth" <michael.roth@amd.com>,
	qemu-devel@nongnu.org
Subject: Re: [PATCH V2 1/4] qapi: strList_from_string
Date: Thu, 09 Feb 2023 11:02:03 +0100	[thread overview]
Message-ID: <87cz6j6tt0.fsf@pond.sub.org> (raw)
In-Reply-To: <87h6vwnstx.fsf@linaro.org> ("Alex Bennée"'s message of "Wed, 08 Feb 2023 14:17:54 +0000")

Alex Bennée <alex.bennee@linaro.org> writes:

> Steven Sistare <steven.sistare@oracle.com> writes:
>
>> On 2/8/2023 1:43 AM, Marc-André Lureau wrote:
>>> Hi
>>> 
>>> On Tue, Feb 7, 2023 at 10:50 PM Steve Sistare <steven.sistare@oracle.com> wrote:
>>>>
>>>> Generalize hmp_split_at_comma() to take any delimiter character, rename
>>>> as strList_from_string(), and move it to qapi/util.c.
>>>>
>>>> No functional change.
>>> 
>>> The g_strsplit() version was a bit simpler, but if you want to
>>> optimize it a bit for 1 char delimiter only, ok.
>>> 
>>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> Yes, and it saves a malloc+free for the array.  Small stuff, but I
>> thought it worth a few lines of code.  Thanks for the speedy review!
>
> But is the HMP path that performance critical? Otherwise I'd favour
> consistent use of the glib APIs because its one less thing to get wrong.

The patch reverts my recent commit 0d79271b570 "hmp: Rewrite
strlist_from_comma_list() as hmp_split_at_comma()", with a different
function name and place, and an additional parameter.

There is no explanation for the revert.

An intentional revert without even mentioning it would be uncourteous.
I don't think this is the case here.  I figure you wrote this patch
before you saw my commit, then rebased, keeping the old code.  A simple
rebase mistake, easy enough to correct.



  reply	other threads:[~2023-02-09 10:02 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-07 18:48 [PATCH V2 0/4] string list functions Steve Sistare
2023-02-07 18:48 ` [PATCH V2 1/4] qapi: strList_from_string Steve Sistare
2023-02-08  6:43   ` Marc-André Lureau
2023-02-08 13:05     ` Steven Sistare
2023-02-08 14:17       ` Alex Bennée
2023-02-09 10:02         ` Markus Armbruster [this message]
2023-02-09 14:41           ` Steven Sistare
2023-02-09 16:46             ` Markus Armbruster
2023-02-09 17:00               ` Steven Sistare
2023-02-09 18:59                 ` Markus Armbruster
2023-02-09 21:34                   ` Steven Sistare
2023-02-10  9:25                     ` Markus Armbruster
2023-06-07 13:54                       ` Steven Sistare
2023-06-13 12:33                         ` Markus Armbruster
2023-06-15 21:25                           ` Steven Sistare
2023-06-19  5:52                             ` Markus Armbruster
2023-02-07 18:48 ` [PATCH V2 2/4] qapi: QAPI_LIST_LENGTH Steve Sistare
2023-02-07 18:48 ` [PATCH V2 3/4] qapi: strv_from_strList Steve Sistare
2023-02-07 18:48 ` [PATCH V2 4/4] qapi: strList unit tests Steve Sistare
2023-02-09 10:05 ` [PATCH V2 0/4] string list functions Markus Armbruster
2023-02-09 14:42   ` Steven Sistare
2023-02-09 16:39     ` Markus Armbruster
2023-02-09 10:48 ` Daniel P. Berrangé
2023-02-09 14:42   ` Steven Sistare
2023-02-10  8:57   ` Markus Armbruster

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=87cz6j6tt0.fsf@pond.sub.org \
    --to=armbru@redhat.com \
    --cc=alex.bennee@linaro.org \
    --cc=dgilbert@redhat.com \
    --cc=marcandre.lureau@gmail.com \
    --cc=michael.roth@amd.com \
    --cc=qemu-devel@nongnu.org \
    --cc=steven.sistare@oracle.com \
    /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.