On 2/9/2023 5:02 AM, Markus Armbruster wrote:
> Alex Bennée <[email protected]> writes:
>
>> Steven Sistare <[email protected]> writes:
>>
>>> On 2/8/2023 1:43 AM, Marc-André Lureau wrote:
>>>> Hi
>>>>
>>>> On Tue, Feb 7, 2023 at 10:50 PM Steve Sistare <[email protected]>
>>>> 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 <[email protected]>
>>>
>>> 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.
Hi Markus, I am sorry, I intended no slight. I will document your commit
in this commit message. And in response to Alex' comment, I will use your
version as the basis of the new function.
For more context, this patch has been part of my larger series for live update,
and I am submitting this separately to reduce the size of that series and make
forward progress:
https://lore.kernel.org/qemu-devel/[email protected]/
In that series, strList_from_string is used to parse a space-separated list of
args
in an HMP command, and pass them to the new qemu binary.
https://lore.kernel.org/qemu-devel/[email protected]/
I moved and renamed the generalized function because I thought it might be
useful
to others in the future, along with the other functions in this 'string list
functions'
patch series. But if you disagree, I can minimally modify hmp_split_at_comma()
in its
current location.
- Steve
