On 09/02/23 5:42 pm, Juan Quintela wrote:
Markus Armbruster <arm...@redhat.com> wrote:
Het Gala <het.g...@nutanix.com> writes:
renamed hmp_split_at_comma() --> str_split_at_comma()
Shifted helper function to qapi-util.c file.
Not an appropriate home.
I don't have an opinion here.
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".
In this case, we are already screwed up O:-)
the uri value in migration has to be split.
What this series does is creating a new way to express the command
(something on the lines that you describe), but we still have to
maintain the old way of doing it for some time, so we need this
function.
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.
Ok, I drop my review-by.
See why I was asking for reviews from QAPI libvirt folks for this code O:-)
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.
You are (way) more C layer than me.
Once told that, I think that what he wanted to do is making this c
standard, no gcc standard.
Once told that, I think that every C compiler worth its salt has this
extension.
https://urldefense.proofpoint.com/v2/url?u=https-3A__gcc.gnu.org_onlinedocs_gcc_Conditionals.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=b-y7IKTlkPluPqpf6lI-BKMLDSrV5JJRRzU39eSq6CpslAITuH5Cxi6l_XugJfkM&s=Z1FND19C0lNnL8v7t_pifjUxyCxbHC8OL2fX-euPRb4&e=
Besides, please don't mix code motion with code changes.
Agreed.
Thankyou for your comments Juan. After discussing on the same with
Daniel, this patch will be dropped in the next patchset version as the
str_split func. it would not be necessary in the first place.
Later, Juan.
Regards,
Het Gala
