Re: [PATCH v2 1/6] migration: moved hmp_split_at_commma() helper func to qapi-util.c file

[Het Gala]

On 09/02/23 5:30 pm, Markus Armbruster wrote:
> Het Gala <het.gala@nutanix.com> writes:
>
> > renamed hmp_split_at_comma() --> str_split_at_comma()
> > Shifted helper function to qapi-util.c file.
> Not an appropriate home.
>
> If we have to split up a string in QAPI/QMP, we screwed up.  Let me
> explain.
>
> In QAPI/QMP, data is not to be encoded in strings, it is to be
> represented directly in JSON.  Thus, ["a", "b", "c"], *not* "a,b,c".
>
> When you find yourself parsing QAPI/QMP string values, you're dealing
> with a case where we violated this interface design principle.  Happens,
> but the proper home for code helping to deal with this is *not* qapi/.
> Simply because QAPI is not about parsing strings.

Yes Markus, I agree with you. But we are also supporting 'uri':'str' 
right now for backward compatibility. So splitting of string might have 
needed. But I misunderstood a crucial part of exec string, and hence had 
to introduce this patch in the first place. Daniel made my understanding 
clear now in the 4th patch.

Now, there is no need to introduce this patch. In the upcoming patchset 
version, this patch will be dropped.

> >                                               Give external linkage, as
> > this function will be handy in coming commit for migration.
> It already has external linkage.
>
> > Minor correction:
> > g_strsplit(str ?: "", ",", -1) --> g_strsplit(str ? str : "", ",", -1)
> This is not actually a correction :)
>
> Omitting the second operand of a conditional expression like x ?: y is
> equivalent to x ? x : y, except it evaluates x only once.
>
> https://urldefense.proofpoint.com/v2/url?u=https-3A__gcc.gnu.org_onlinedocs_gcc_Conditionals.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=gAsWrkiPm3MCpqkWQzGYo9-M2_2bkxfDAGmW8lgXmOAnW2YoDs5AhxGPt-Sc5xI3&s=Onmed5Fm0PImk6PD44bAvu8yQDrGuYU44yRYAw3Abjs&e=
Ack. Thankyou for getting it into my knowledge. Will take care from next 
time
> Besides, please don't mix code motion with code changes.
Yes sure Markus. I apologise for not knowing it earlier and introducing 
such a patch, but lesson learned :)
> [...]
Regards,
Het Gala