Re: [Tarantool-patches] [PATCH 5/5] msgpuck: activate MP_EXT custom serializers

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Thanks for the review!

>> On 13/05/2020 23:06, Nikita Pettik wrote:
>>> On 12 May 01:45, Vladislav Shpilevoy wrote:
>>>> +
>>>> +static int
>>>> +msgpack_fprint_ext(FILE *file, const char **data, int depth)
>>>> +{
>>>> +	int8_t type;
>>>> +	uint32_t len = mp_decode_extl(data, &type);
>>>> +	switch(type) {
>>>> +	case MP_DECIMAL:
>>>> +		return mp_fprint_decimal(file, data, len);
>>>> +	case MP_UUID:
>>>> +		return mp_fprint_uuid(file, data, len);
>>>> +	case MP_ERROR:
>>>> +		return mp_fprint_error(file, data, depth);
>>>> +	default:
>>>> +		return fprintf(file, "undefined");
>>>
>>> I'd come up with more sensible message in case of "undefined" mp_ type.
>>> For instance: ("undefined mgpack extension (%d)", type).
>>
>> This is not an error message.
> 
> I understand that. Still I stick to the point that simple "undefined"
> looks poor.

Ok, I just noticed that you wrapped your proposal into (). It is not just
a bare string or something else, that could be treated in 2 ways. Then I
guess it is ok. Anyway both 'undefined', and parenthesis are JS JSON parsers
features only. So by adding a few details we are not breaking normal JSON
parsers any further.

I changed this commit, and added a new commit to the msgpuck patchset.
The message is changed a little.

====================
diff --git a/src/box/msgpack.c b/src/box/msgpack.c
index 37bb3920c..4013aec4c 100644
--- a/src/box/msgpack.c
+++ b/src/box/msgpack.c
@@ -49,7 +49,8 @@ msgpack_fprint_ext(FILE *file, const char **data, int depth)
 	case MP_ERROR:
 		return mp_fprint_error(file, data, depth);
 	default:
-		return fprintf(file, "undefined");
+		return fprintf(file, "(extension: type %d, len %u)", (int)type,
+			       (unsigned)len);
 	}
 }
 
@@ -66,7 +67,8 @@ msgpack_snprint_ext(char *buf, int size, const char **data, int depth)
 	case MP_ERROR:
 		return mp_snprint_error(buf, size, data, depth);
 	default:
-		return snprintf(buf, size, "undefined");
+		return snprintf(buf, size, "(extension: type %d, len %u)",
+				(int)type, (unsigned)len);
 	}
 }