Re: [PATCH V2 1/4] qapi: strList_from_string

[Markus Armbruster]

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.