[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/6] migration: moved hmp_split_at_commma() helper func to
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v2 1/6] migration: moved hmp_split_at_commma() helper func to qapi-util.c file |
Date: |
Thu, 09 Feb 2023 13:00:25 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
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.
> 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://gcc.gnu.org/onlinedocs/gcc/Conditionals.html
Besides, please don't mix code motion with code changes.
[...]