Re: [tarantool-patches] Re: [PATCH v2] xrow: print corrupted rows on decoding error.

[Vladimir Davydov <vdavydov.dev@gmail.com>]

On Fri, Apr 05, 2019 at 09:18:46AM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev@gmail.com> [19/04/04 17:57]:
> > Pushed to master, 2.1, 1.10.
> 
> This was fast.

The first version of this patch was submitted for review more than two
weeks ago, on March 15. We haven't heard from you since then. Though you
didn't care.

> 
> First of all, we already have request_str() function, and there is
> a good deal of code reuse that can take place.

request_str() prints a well-formed request. Here we want to dump
corrupted msgpack.

> 
> This comment, basically, says it all.
> 
> But provided you had to write your own implementation:
> - why use a macro?

So that diag_set() and say() preserve file/line.

> - why use malloc?

Because a corrupted request can easily be longer than 256 bytes (that's
how many bytes fits in tt_static_buf). Since it's a slow path, which is
only executed on decode error provided S_VERBOSE log level is enabled,
I reckon using malloc() is fine.

> - what's wrong with mp_snprint, and if the problem is that it
>   doesn't properly escape non-printable characters, why not fix it
>   to escape these or implement a version which does?

Because data can be corrupted, e.g. an MP_ARRAY with N items in the
header having only M < N items in the body. We can't print such data
with mp_snprint.