Re: [Qemu-devel] [PATCH 07/56] check-qjson: Cover escaped characters more thoroughly, part 1

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 08/08/2018 07:02 AM, Markus Armbruster wrote:
>> escaped_string() first tests double quoted strings, then repeats a few
>> tests with single quotes.  Repeat all of them: store the strings to
>> test without quotes, and wrap them in either kind of quote for
>> testing.
>
> Does that properly cover the fact that "'" and '"' are valid, but the
> counterparts need escaping?
>
>>
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>>   tests/check-qjson.c | 94 ++++++++++++++++++++++++++-------------------
>>   1 file changed, 55 insertions(+), 39 deletions(-)
>>
>> diff --git a/tests/check-qjson.c b/tests/check-qjson.c
>> index 0a9a054c7b..1c7f24bc4d 100644
>> --- a/tests/check-qjson.c
>> +++ b/tests/check-qjson.c
>> @@ -22,55 +22,71 @@
>>   #include "qapi/qmp/qstring.h"
>>   #include "qemu-common.h"
>>   +static QString *from_json_str(const char *jstr, Error **errp,
>> bool single)
>> +{
>> +    char quote = single ? '\'' : '"';
>> +    char *qjstr = g_strdup_printf("%c%s%c", quote, jstr, quote);
>> +
>> +    return qobject_to(QString, qobject_from_json(qjstr, errp));
>
> Memory leak of qjstr.

Fixing.

>> +}
>> +
>> +static char *to_json_str(QString *str)
>> +{
>> +    QString *json = qobject_to_json(QOBJECT(str));
>> +    char *jstr;
>> +
>> +    if (!json) {
>> +        return NULL;
>> +    }
>> +    /* peel off double quotes */
>> +    jstr = g_strndup(qstring_get_str(json) + 1,
>> +                     qstring_get_length(json) - 2);
>> +    qobject_unref(json);
>> +    return jstr;
>> +}
>> +
>>   static void escaped_string(void)
>>   {
>> -    int i;
>>       struct {
>> -        const char *encoded;
>> -        const char *decoded;
>> +        /* Content of JSON string to parse with qobject_from_json() */
>> +        const char *json_in;
>> +        /* Expected parse output; to unparse with qobject_to_json() */
>> +        const char *utf8_out;
>>           int skip;
>>       } test_cases[] = {
>
>> +        { "\\\"", "\"" },
>
> This covers the escaped version of ", but not of ', and not the
> unescaped version of either (per my comment above, the latter can only
> be done with the opposite quoting).

escaped_string() is about testing \-escapes.  Unescaped quotes are
covered by simple_string() and single_quote_string().

However, I drop both in PATCH 10.  That's actually a bad idea.

>> Otherwise looks sane.

Thanks!