Re: [Tarantool-patches] [PATCH 2/9] error: introduce error_payload

[Serge Petrenko via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

06.11.2021 02:56, Vladislav Shpilevoy пишет:
> It is a dictionary-like struct which stores keys with binary data
> values. The values are supposed to be arbitrary MessagePack blobs:
> number, string, bool, UUID, array, map, anything.
>
> The payload is an array inside instead of a hash table because
> number of keys will be <= 3 in all cases. And even when it will be
> public, it is very unlikely it will be bigger.
>
> Object of error_payload in a future patch will be stored in struct
> error and will allow to extend it dynamically with more members.
>
> This in turn is needed to extend ER_READONLY error with more
> details about why it happened.
>
> Part of #5568

Hi! Thanks for the patch!
Please, find 6 comments below.

> ---
>   src/lib/core/CMakeLists.txt  |   1 +
>   src/lib/core/error_payload.c | 282 +++++++++++++++++++++
>   src/lib/core/error_payload.h | 166 +++++++++++++
>   src/lib/uuid/mp_uuid.c       |  26 ++
>   src/lib/uuid/tt_uuid.h       |  24 ++
>   test/unit/CMakeLists.txt     |   2 +
>   test/unit/error.c            | 461 +++++++++++++++++++++++++++++++++++
>   test/unit/error.result       | 160 ++++++++++++
>   8 files changed, 1122 insertions(+)
>   create mode 100644 src/lib/core/error_payload.c
>   create mode 100644 src/lib/core/error_payload.h
>   create mode 100644 test/unit/error.c
>   create mode 100644 test/unit/error.result
>
> diff --git a/src/lib/core/CMakeLists.txt b/src/lib/core/CMakeLists.txt
> index 0cc742a1c..860758515 100644
> --- a/src/lib/core/CMakeLists.txt
> +++ b/src/lib/core/CMakeLists.txt
> @@ -21,6 +21,7 @@ set(core_sources
>       fio.c
>       exception.cc
>       errinj.c
> +    error_payload.c
>       reflection.c
>       assoc.c
>       util.c
> diff --git a/src/lib/core/error_payload.c b/src/lib/core/error_payload.c
> new file mode 100644
> index 000000000..98ef08ada
> --- /dev/null
> +++ b/src/lib/core/error_payload.c
> @@ -0,0 +1,282 @@
> +/*
> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright 2010-2021, Tarantool AUTHORS, please see AUTHORS file.
> + */
> +#include "error_payload.h"
> +#include "msgpuck.h"
> +
> +void
> +error_payload_create(struct error_payload *p)
> +{
> +	p->count = 0;
> +	p->fields = NULL;
> +}
> +
> +void
> +error_payload_destroy(struct error_payload *p)
> +{
> +	for (int i = 0; i < p->count; ++i) {
> +		TRASH(p->fields[i]);
> +		free(p->fields[i]);
> +	}
> +	free(p->fields);
> +	TRASH(p);
> +}
> +
> +const char *
> +error_payload_get_str(const struct error_payload *e, const char *name)
> +{
> +	const struct error_field *f = error_payload_find(e, name);
> +	if (f == NULL)
> +		return NULL;
> +	const char *data = f->data;
> +	if (mp_typeof(*data) != MP_STR)
> +		return NULL;
> +	uint32_t len;
> +	/*
> +	 * 0 is explicitly written after encoded string to be able to return
> +	 * them without copying.
> +	 */
> +	data = mp_decode_str(&data, &len);
> +	assert(data[len] == 0);

1. Nit: `data[len] == '\0'`. Here and everywhere else. This is up to you,
    I'm just more used to '\0'.

> +	return data;
> +}
> +
> +void
> +error_payload_set_str(struct error_payload *e, const char *name,
> +		      const char *value)
> +{
> +	uint32_t len = strlen(value);
> +	char *data = error_payload_prepare(
> +		e, name, mp_sizeof_str(len), 1)->data;
> +	/*
> +	 * 1 extra byte in the end is reserved to append 0 after the encoded
> +	 * string. To be able to return it from get() without copying.
> +	 */

2. This comment is almost the same as the one in payload_get_str().
    I propose to leave only one of them. Preferably the one in set_str().

> +	*mp_encode_str(data, value, len) = 0;
> +}
> +
> +bool
> +error_payload_get_uint(const struct error_payload *e, const char *name,
> +		       uint64_t *value)
> +{
> +	const struct error_field *f = error_payload_find(e, name);
> +	if (f == NULL)
> +		goto not_found;
> +	const char *data = f->data;
> +	if (mp_typeof(*data) != MP_UINT)
> +		goto not_found;
> +	*value = mp_decode_uint(&data);
> +	return true;
> +
> +not_found:
> +	*value = 0;
> +	return false;
> +}
> +

3. Consider this diff here.
    (Similar places below do need a label, but this one doesn't).

============================

diff --git a/src/lib/core/error_payload.c b/src/lib/core/error_payload.c
index 98ef08ada..cfa30919e 100644
--- a/src/lib/core/error_payload.c
+++ b/src/lib/core/error_payload.c
@@ -62,17 +62,14 @@ error_payload_get_uint(const struct error_payload 
*e, const char *name,
                        uint64_t *value)
  {
         const struct error_field *f = error_payload_find(e, name);
+       *value = 0;
         if (f == NULL)
-               goto not_found;
+               return false;
         const char *data = f->data;
         if (mp_typeof(*data) != MP_UINT)
-               goto not_found;
+               return false;
         *value = mp_decode_uint(&data);
         return true;
-
-not_found:
-       *value = 0;
-       return false;
  }

  void

============================

> +void
> +error_payload_set_uint(struct error_payload *e, const char *name,
> +		       uint64_t value)
> +{
> +	char *data = error_payload_prepare(
> +		e, name, mp_sizeof_uint(value), 0)->data;
> +	mp_encode_uint(data, value);
> +}
> +
> +bool
> +error_payload_get_int(const struct error_payload *e, const char *name,
> +		      int64_t *value)
> +{
> +	const struct error_field *f = error_payload_find(e, name);
> +	if (f == NULL)
> +		goto not_found;
> +	const char *data = f->data;
> +	if (mp_typeof(*data) == MP_UINT) {
> +		uint64_t u = mp_decode_uint(&data);
> +		if (u > INT64_MAX)
> +			goto not_found;
> +		*value = u;
> +		return true;
> +	} else if (mp_typeof(*data) == MP_INT) {
> +		*value = mp_decode_int(&data);
> +		return true;
> +	}
> +not_found:
> +	*value = 0;
> +	return false;
> +}
> +
> +void
> +error_payload_set_int(struct error_payload *e, const char *name, int64_t value)
> +{
> +	if (value >= 0)
> +		return error_payload_set_uint(e, name, value);
> +	char *data = error_payload_prepare(
> +		e, name, mp_sizeof_int(value), 0)->data;
> +	mp_encode_int(data, value);
> +}
> +
> +bool
> +error_payload_get_double(const struct error_payload *e, const char *name,
> +			 double *value)
> +{
> +	const struct error_field *f = error_payload_find(e, name);
> +	if (f == NULL)
> +		goto not_found;
> +	const char *data = f->data;
> +	if (mp_typeof(*data) == MP_DOUBLE) {
> +		*value = mp_decode_double(&data);
> +		return true;
> +	} else if (mp_typeof(*data) == MP_FLOAT) {
> +		*value = mp_decode_float(&data);
> +		return true;
> +	}

4. Why do you check for both MP_DOUBLE and MP_FLOAT here?
    You only encode MP_DOUBLE.

    I think we might have set_float() and get_float() one day, but looks
    like they're not needed now.
    get_float() would only accept MP_FLOAT then, and get_double() should
    only accept MP_DOUBLE.

> +not_found:
> +	*value = 0;
> +	return false;
> +}
> +
> +void
> +error_payload_set_double(struct error_payload *e, const char *name,
> +			 double value)
> +{
> +	char *data = error_payload_prepare(
> +		e, name, mp_sizeof_double(value), 0)->data;
> +	mp_encode_double(data, value);
> +}
> +
> +bool
> +error_payload_get_bool(const struct error_payload *e, const char *name,
> +		       bool *value)
> +{
> +	const struct error_field *f = error_payload_find(e, name);
> +	if (f == NULL)
> +		goto not_found;
> +	const char *data = f->data;
> +	if (mp_typeof(*data) != MP_BOOL)
> +		goto not_found;
> +	*value = mp_decode_bool(&data);
> +	return true;
> +
> +not_found:
> +	*value = false;
> +	return false;
> +}
> +
> +void
> +error_payload_set_bool(struct error_payload *e, const char *name, bool value)
> +{
> +	char *data = error_payload_prepare(
> +		e, name, mp_sizeof_bool(value), 0)->data;
> +	mp_encode_bool(data, value);
> +}
> +
> +const char *
> +error_payload_get_mp(const struct error_payload *e, const char *name,
> +		     uint32_t *size)
> +{
> +	const struct error_field *f = error_payload_find(e, name);
> +	if (f == NULL) {
> +		*size = 0;
> +		return NULL;
> +	}
> +	*size = f->size;
> +	return f->data;
> +}
> +
> +void
> +error_payload_set_mp(struct error_payload *e, const char *name,
> +		     const char *src, uint32_t size)
> +{
> +	char *data;
> +	if (mp_typeof(*src) == MP_STR) {
> +		data = error_payload_prepare(e, name, size, 1)->data;
> +		/*
> +		 * Add the terminating zero after the buffer so as get_str()
> +		 * would work without copying.
> +		 */

5. Maybe shorten this comment to "\sa error_payload_set_str()" ?

> +		data[size] = 0;
> +	} else {
> +		data = error_payload_prepare(e, name, size, 0)->data;
> +	}
> +	memcpy(data, src, size);
> +}
> +
> +void
> +error_payload_unset(struct error_payload *e, const char *name)
> +{
> +	struct error_field **fields = e->fields;
> +	for (int i = 0; i < e->count; ++i) {
> +		struct error_field *f = fields[i];
> +		if (strcmp(name, f->name) != 0)
> +			continue;
> +		TRASH(f);
> +		free(f);
> +		int count = --e->count;
> +		if (count == 0) {
> +			free(fields);
> +			e->fields = NULL;
> +			return;
> +		}
> +		/* Cyclic deletion. Order does not matter in a dictionary. */
> +		fields[i] = fields[count];
> +		return;
> +	}
> +}
> +
> +void
> +error_payload_move(struct error_payload *dst, struct error_payload *src)
> +{
> +	for (int i = 0; i < dst->count; ++i) {
> +		TRASH(dst->fields[i]);
> +		free(dst->fields[i]);
> +	}
> +	free(dst->fields);
> +	dst->fields = src->fields;
> +	dst->count = src->count;
> +	src->fields = NULL;
> +	src->count = 0;
> +}
> +
> +const struct error_field *
> +error_payload_find(const struct error_payload *e, const char *name)
> +{
> +	for (int i = 0; i < e->count; ++i) {
> +		const struct error_field *f = e->fields[i];
> +		if (strcmp(name, f->name) == 0)
> +			return f;
> +	}
> +	return NULL;
> +}
> +
> +struct error_field *
> +error_payload_prepare(struct error_payload *e, const char *name,
> +		      uint32_t value_size, uint32_t extra)
> +{
> +	struct error_field *f;
> +	uint32_t name_size = strlen(name) + 1;
> +	uint32_t data_offset = name_size + sizeof(*f);
> +	uint32_t total = data_offset + value_size + extra;
> +
> +	struct error_field **fields = e->fields;
> +	int count = e->count;
> +	int i;
> +	for (i = 0; i < count; ++i) {
> +		f = fields[i];
> +		if (strcmp(name, f->name) != 0)
> +			continue;
> +		f = xrealloc(f, total);
> +		goto set;
> +	}
> +	e->count = ++count;
> +	fields = xrealloc(fields, sizeof(fields[0]) * count);
> +	e->fields = fields;
> +	f = xmalloc(total);
> +	memcpy(f->name, name, name_size);
> +set:
> +	f->data = (char *)f + data_offset;
> +	f->size = value_size;
> +	fields[i] = f;
> +	return f;
> +}
> diff --git a/src/lib/core/error_payload.h b/src/lib/core/error_payload.h
> new file mode 100644
> index 000000000..b6bf9ceee
> --- /dev/null
> +++ b/src/lib/core/error_payload.h
> @@ -0,0 +1,166 @@
> +/*
> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright 2010-2021, Tarantool AUTHORS, please see AUTHORS file.
> + */
> +#pragma once
> +
> +#include "trivia/util.h"
> +
> +#if defined(__cplusplus)
> +extern "C" {
> +#endif /* defined(__cplusplus) */
> +
> +/** Single field of error payload. */
> +struct error_field {
> +	/** Data size. */
> +	uint32_t size;
> +	/** MessagePack field value. */
> +	char *data;
> +	/** Zero terminated field name. */
> +	char name[0];
> +};
> +
> +/** An array of key-value pairs. */
> +struct error_payload {
> +	/** Number of fields. */
> +	int count;
> +	/**
> +	 * Array of field pointers. Not very optimized, but the errors are
> +	 * supposed to be rare. Also not storing them by values simplifies
> +	 * addition of new fields and their removal.
> +	 */
> +	struct error_field **fields;
> +};
> +
> +/** Create error payload. */
> +void
> +error_payload_create(struct error_payload *p);
> +
> +/** Destroy error payload. */
> +void
> +error_payload_destroy(struct error_payload *p);
> +
> +/**
> + * Get value of a payload field as a string. If it is not string or is not
> + * found - return NULL.
> + */
> +const char *
> +error_payload_get_str(const struct error_payload *e, const char *name);
> +
> +/**
> + * Set value of a payload field to a string. If the field existed before, it is
> + * overwritten.
> + */
> +void
> +error_payload_set_str(struct error_payload *e, const char *name,
> +		      const char *value);
> +
> +/**
> + * Get value of a payload field as a uint. If it is not uint or is not found -
> + * return false and the out parameter is set to 0.
> + */
> +bool
> +error_payload_get_uint(const struct error_payload *e, const char *name,
> +		       uint64_t *value);
> +
> +/**
> + * Set value of a payload field to a uint. If the field existed before, it is
> + * overwritten.
> + */
> +void
> +error_payload_set_uint(struct error_payload *e, const char *name,
> +		       uint64_t value);
> +
> +/**
> + * Get value of a payload field as an int. If it is not int, or is not found, or
> + * does not fit int64_t - return false and the out parameter is set to 0.
> + */
> +bool
> +error_payload_get_int(const struct error_payload *e, const char *name,
> +		      int64_t *value);
> +
> +/**
> + * Set value of a payload field to an int. If the field existed before, it is
> + * overwritten.
> + */
> +void
> +error_payload_set_int(struct error_payload *e, const char *name, int64_t value);
> +
> +/**
> + * Get value of a payload field as a double. If it is not double or is not
> + * found - return false and the out parameter is set to 0.
> + */
> +bool
> +error_payload_get_double(const struct error_payload *e, const char *name,
> +			 double *value);
> +
> +/**
> + * Set value of a payload field to a double. If the field existed before, it is
> + * overwritten.
> + */
> +void
> +error_payload_set_double(struct error_payload *e, const char *name,
> +			 double value);
> +
> +/**
> + * Get value of a payload field as a bool. If it is not bool or is not found -
> + * return false and the out parameter is set to false.
> + */
> +bool
> +error_payload_get_bool(const struct error_payload *e, const char *name,
> +		       bool *value);
> +
> +/**
> + * Set value of a payload field to a bool. If the field existed before, it is
> + * overwritten.
> + */
> +void
> +error_payload_set_bool(struct error_payload *e, const char *name, bool value);
> +
> +/**
> + * Get MessagePack value of a payload field. If it is not found - return NULL
> + * and the out parameter is set to 0.
> + */
> +const char *
> +error_payload_get_mp(const struct error_payload *e, const char *name,
> +		     uint32_t *size);
> +
> +/**
> + * Set value of a payload field to a MessagePack buffer. If the field existed
> + * before, it is overwritten.
> + */
> +void
> +error_payload_set_mp(struct error_payload *e, const char *name,
> +		     const char *src, uint32_t size);
> +
> +/** Remove the given field from the payload. */
> +void
> +error_payload_unset(struct error_payload *e, const char *name);
> +
> +/**
> + * Move all fields of one payload into another. Old fields of the destination
> + * are all deleted. The source stays valid but empty.
> + */
> +void
> +error_payload_move(struct error_payload *dst, struct error_payload *src);
> +
> +/** Find a payload field by name. */
> +const struct error_field *
> +error_payload_find(const struct error_payload *e, const char *name);
> +
> +/**
> + * Prepare a payload field to get a new value. If the field didn't exist, it is
> + * added. If it existed then it is reallocated if was necessary and should be
> + * overwritten. Name is already filled in the field. Only need to fill the
> + * value.
> + * Extra parameter allows to allocate extra memory for arbitrary usage after the
> + * error field and its value.
> + */
> +struct error_field *
> +error_payload_prepare(struct error_payload *e, const char *name,
> +		      uint32_t value_size, uint32_t extra);
> +
> +#if defined(__cplusplus)
> +} /* extern "C" */
> +#endif /* defined(__cplusplus) */
> diff --git a/src/lib/uuid/mp_uuid.c b/src/lib/uuid/mp_uuid.c
> index b2341ae36..a60716eb5 100644
> --- a/src/lib/uuid/mp_uuid.c
> +++ b/src/lib/uuid/mp_uuid.c
> @@ -32,6 +32,7 @@
>   #include "mp_uuid.h"
>   #include "msgpuck.h"
>   #include "mp_extension_types.h"
> +#include "error_payload.h"
>   
>   inline uint32_t
>   mp_sizeof_uuid(void)
> @@ -113,3 +114,28 @@ mp_fprint_uuid(FILE *file, const char **data, uint32_t len)
>   		return -1;
>   	return fprintf(file, "%s", tt_uuid_str(&uuid));
>   }
> +
> +bool
> +error_payload_get_uuid(const struct error_payload *e, const char *name,
> +		       struct tt_uuid *uuid)
> +{
> +	const struct error_field *f = error_payload_find(e, name);
> +	if (f == NULL)
> +		goto not_found;
> +	const char *data = f->data;
> +	if (mp_decode_uuid(&data, uuid) == NULL)
> +		goto not_found;
> +	return true;
> +
> +not_found:
> +	*uuid = uuid_nil;
> +	return false;
> +}
> +

6. I propose to get rid of the label here and set uuid to nil 
unconditionally.

    Would it be too expensive to do 2 struct assignments instead of one?
    (uuid = uuid_nil; uuid = decoded_value).

> +void
> +error_payload_set_uuid(struct error_payload *e, const char *name,
> +		       const struct tt_uuid *uuid)
> +{
> +	char *data = error_payload_prepare(e, name, mp_sizeof_uuid(), 0)->data;
> +	mp_encode_uuid(data, uuid);
> +}
> diff --git a/src/lib/uuid/tt_uuid.h b/src/lib/uuid/tt_uuid.h
> index 70c3b98b1..866764e77 100644
> --- a/src/lib/uuid/tt_uuid.h
> +++ b/src/lib/uuid/tt_uuid.h
> @@ -41,6 +41,8 @@
>   extern "C" {
>   #endif
>   
> +struct error_payload;
> +
>   enum { UUID_LEN = 16, UUID_STR_LEN = 36 };
>   
>   /**
> @@ -182,6 +184,28 @@ tt_uuid_str(const struct tt_uuid *uu);
>   int
>   tt_uuid_from_strl(const char *in, size_t len, struct tt_uuid *uu);
>   
> +/**
> + * Error payload couldn't be implemented in libcore because requires UUID type
> + * and its methods. That would be a cyclic dep. Thus in places where need to use
> + * UUID fields in error payload also need to include libuuid.
> + */
> +
> +/**
> + * Get value of a payload field as a UUID. If it is not UUID or is not found -
> + * return false and the out parameter is set to UUID with zeros.
> + */
> +bool
> +error_payload_get_uuid(const struct error_payload *e, const char *name,
> +		       struct tt_uuid *uuid);
> +
> +/**
> + * Set value of a payload field to a UUID. If the field existed before, it is
> + * overwritten.
> + */
> +void
> +error_payload_set_uuid(struct error_payload *e, const char *name,
> +		       const struct tt_uuid *uuid);
> +
>   #if defined(__cplusplus)
>   } /* extern "C" */
>   #endif
> diff --git a/test/unit/CMakeLists.txt b/test/unit/CMakeLists.txt
> index ae8b5b9ac..0bb71ccce 100644
> --- a/test/unit/CMakeLists.txt
> +++ b/test/unit/CMakeLists.txt
> @@ -60,6 +60,8 @@ add_executable(xmalloc.test xmalloc.c core_test_utils.c)
>   target_link_libraries(xmalloc.test unit)
>   add_executable(datetime.test datetime.c)
>   target_link_libraries(datetime.test tzcode core cdt unit)
> +add_executable(error.test error.c core_test_utils.c)
> +target_link_libraries(error.test unit core uuid)
>   
>   add_executable(bps_tree.test bps_tree.cc)
>   target_link_libraries(bps_tree.test small misc)
> diff --git a/test/unit/error.c b/test/unit/error.c
> new file mode 100644
> index 000000000..ef17c56ec
> --- /dev/null
> +++ b/test/unit/error.c
> @@ -0,0 +1,461 @@
> +/*
> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright 2010-2021, Tarantool AUTHORS, please see AUTHORS file.
> + */
> +#include "error_payload.h"
> +#include "msgpuck.h"
> +#include "random.h"
> +#include "unit.h"
> +#include "uuid/mp_uuid.h"
> +#include "uuid/tt_uuid.h"
> +
> +#include <float.h>
> +
> +static void
> +test_payload_field_str(void)
> +{
> +	header();
> +	plan(15);
> +
> +	struct error_payload p;
> +	error_payload_create(&p);
> +	is(p.count, 0, "no fields in the beginning");
> +	is(error_payload_get_str(&p, "key"), NULL, "get_str() empty");
> +
> +	error_payload_set_str(&p, "key1", "value1");
> +	is(p.count, 1, "++count");
> +	is(strcmp(error_payload_get_str(&p, "key1"), "value1"), 0,
> +	   "get_str() finds");
> +
> +	error_payload_set_str(&p, "key2", "value2");
> +	is(p.count, 2, "++count");
> +	is(strcmp(error_payload_get_str(&p, "key1"), "value1"), 0,
> +	   "get_str() finds old");
> +	is(strcmp(error_payload_get_str(&p, "key2"), "value2"), 0,
> +	   "get_str() finds new");
> +	is(error_payload_find(&p, "key1")->size,
> +	   mp_sizeof_str(strlen("value1")),
> +	   "size does not include terminating zero");
> +
> +	error_payload_set_str(&p, "key1", "new_value1");
> +	is(p.count, 2, "field count is same");
> +	is(strcmp(error_payload_get_str(&p, "key1"), "new_value1"), 0,
> +	   "get_str() finds new value");
> +	is(strcmp(error_payload_get_str(&p, "key2"), "value2"), 0,
> +	   "get_str() finds other key old value");
> +
> +	error_payload_unset(&p, "key2");
> +	is(p.count, 1, "--count");
> +	is(strcmp(error_payload_get_str(&p, "key1"), "new_value1"), 0,
> +	   "get_str() finds new value");
> +	is(error_payload_get_str(&p, "key2"), NULL,
> +	   "get_str() can't find deleted value");
> +
> +	error_payload_set_uint(&p, "key2", 1);
> +	is(error_payload_get_str(&p, "key2"), NULL, "wrong type");
> +
> +	error_payload_destroy(&p);
> +
> +	check_plan();
> +	footer();
> +}
> +
> +static void
> +test_payload_field_uint(void)
> +{
> +	header();
> +	plan(17);
> +
> +	struct error_payload p;
> +	error_payload_create(&p);
> +	uint64_t val = 1;
> +	ok(!error_payload_get_uint(&p, "key", &val) && val == 0,
> +	   "get_uint() empty");
> +
> +	error_payload_set_uint(&p, "key1", 1);
> +	is(p.count, 1, "++count");
> +	ok(error_payload_get_uint(&p, "key1", &val), "get_uint() finds");
> +	is(val, 1, "value match");
> +
> +	val = 100;
> +	ok(!error_payload_get_uint(&p, "key2", &val), "get_uint() fails");
> +	is(val, 0, "value is default");
> +
> +	is(error_payload_find(&p, "key1")->size, mp_sizeof_uint(1),
> +	   "small number size");
> +
> +	error_payload_set_uint(&p, "key2", UINT32_MAX);
> +	ok(error_payload_get_uint(&p, "key2", &val), "get_uint() 32 bit");
> +	is(val, UINT32_MAX, "value match");
> +	is(error_payload_find(&p, "key2")->size, mp_sizeof_uint(UINT32_MAX),
> +	   "middle number size");
> +	is(p.count, 2, "field count is same");
> +
> +	error_payload_set_uint(&p, "key1", UINT64_MAX);
> +	ok(error_payload_get_uint(&p, "key1", &val) && val == UINT64_MAX,
> +	   "value 1");
> +	ok(error_payload_get_uint(&p, "key2", &val) && val == UINT32_MAX,
> +	   "value 2");
> +
> +	error_payload_unset(&p, "key2");
> +	is(p.count, 1, "--count");
> +	ok(error_payload_get_uint(&p, "key1", &val) && val == UINT64_MAX,
> +	   "remained value");
> +	ok(!error_payload_get_uint(&p, "key2", &val) && val == 0,
> +	   "deleted value");
> +
> +	error_payload_set_str(&p, "key2", "1");
> +	ok(!error_payload_get_uint(&p, "key2", &val) && val == 0, "wrong type");
> +
> +	error_payload_destroy(&p);
> +
> +	check_plan();
> +	footer();
> +}
> +
> +static void
> +test_payload_field_int(void)
> +{
> +	header();
> +	plan(20);
> +
> +	struct error_payload p;
> +	error_payload_create(&p);
> +	int64_t val = 1;
> +	ok(!error_payload_get_int(&p, "key", &val) && val == 0,
> +	   "get_int() empty");
> +
> +	error_payload_set_int(&p, "key1", 1);
> +	is(p.count, 1, "++count");
> +	ok(error_payload_get_int(&p, "key1", &val), "get_int() finds");
> +	is(val, 1, "value match");
> +
> +	val = 100;
> +	ok(!error_payload_get_int(&p, "key2", &val), "get_int() fails");
> +	is(val, 0, "value is default");
> +
> +	is(error_payload_find(&p, "key1")->size, mp_sizeof_uint(1),
> +	   "small number size");
> +
> +	error_payload_set_int(&p, "key2", UINT32_MAX);
> +	ok(error_payload_get_int(&p, "key2", &val), "get_int() 32 bit");
> +	is(val, UINT32_MAX, "value match");
> +	is(error_payload_find(&p, "key2")->size, mp_sizeof_uint(UINT32_MAX),
> +	   "middle number size");
> +	is(p.count, 2, "field count is same");
> +
> +	error_payload_set_int(&p, "key1", INT64_MAX);
> +	ok(error_payload_get_int(&p, "key1", &val) && val == INT64_MAX,
> +	   "value 1");
> +	ok(error_payload_get_int(&p, "key2", &val) && val == UINT32_MAX,
> +	   "value 2");
> +
> +	error_payload_unset(&p, "key2");
> +	is(p.count, 1, "--count");
> +	ok(error_payload_get_int(&p, "key1", &val) && val == INT64_MAX,
> +	   "remained value");
> +	ok(!error_payload_get_int(&p, "key2", &val) && val == 0,
> +	   "deleted value");
> +
> +	error_payload_set_str(&p, "key2", "1");
> +	ok(!error_payload_get_int(&p, "key2", &val) && val == 0, "wrong type");
> +
> +	error_payload_set_uint(&p, "key2", (uint64_t)INT64_MAX + 1);
> +	ok(!error_payload_get_int(&p, "key2", &val) && val == 0, "overflow");
> +
> +	error_payload_set_uint(&p, "key2", 100);
> +	ok(error_payload_get_int(&p, "key2", &val) && val == 100, "conversion");
> +
> +	error_payload_set_int(&p, "key2", INT64_MIN);
> +	ok(error_payload_get_int(&p, "key2", &val) && val == INT64_MIN,
> +	   "min value");
> +
> +	error_payload_destroy(&p);
> +
> +	check_plan();
> +	footer();
> +}
> +
> +static void
> +test_payload_field_double(void)
> +{
> +	header();
> +	plan(13);
> +
> +	struct error_payload p;
> +	error_payload_create(&p);
> +	double val = 1;
> +	ok(!error_payload_get_double(&p, "key", &val) && val == 0,
> +	   "get_double() empty");
> +
> +	error_payload_set_double(&p, "key1", 1.5);
> +	is(p.count, 1, "++count");
> +	ok(error_payload_get_double(&p, "key1", &val), "get_double() finds");
> +	is(val, 1.5, "value match");
> +
> +	val = 1;
> +	ok(!error_payload_get_double(&p, "key2", &val), "get_double() fails");
> +	is(val, 0, "value is default");
> +
> +	is(error_payload_find(&p, "key1")->size, mp_sizeof_double(1.5), "size");
> +
> +	error_payload_set_double(&p, "key2", DBL_MAX);
> +	ok(error_payload_get_double(&p, "key1", &val) && val == 1.5, "value 1");
> +	ok(error_payload_get_double(&p, "key2", &val) && val == DBL_MAX,
> +	   "value 2");
> +
> +	error_payload_unset(&p, "key2");
> +	is(p.count, 1, "--count");
> +	ok(error_payload_get_double(&p, "key1", &val) && val == 1.5,
> +	   "remained value");
> +	ok(!error_payload_get_double(&p, "key2", &val) && val == 0,
> +	   "deleted value");
> +
> +	error_payload_set_str(&p, "key2", "1");
> +	ok(!error_payload_get_double(&p, "key2", &val) && val == 0,
> +	   "wrong type");
> +
> +	error_payload_destroy(&p);
> +
> +	check_plan();
> +	footer();
> +}
> +
> +static void
> +test_payload_field_bool(void)
> +{
> +	header();
> +	plan(13);
> +
> +	struct error_payload p;
> +	error_payload_create(&p);
> +	bool val = true;
> +	ok(!error_payload_get_bool(&p, "key", &val) && !val,
> +	   "get_bool() empty");
> +
> +	error_payload_set_bool(&p, "key1", true);
> +	is(p.count, 1, "++count");
> +	ok(error_payload_get_bool(&p, "key1", &val), "get_bool() finds");
> +	ok(val, "value match");
> +
> +	val = true;
> +	ok(!error_payload_get_bool(&p, "key2", &val), "get_bool() fails");
> +	ok(!val, "value is default");
> +
> +	error_payload_set_bool(&p, "key2", false);
> +	ok(error_payload_get_bool(&p, "key2", &val), "get_bool() finds");
> +	ok(!val, "value match");
> +
> +	is(error_payload_find(&p, "key1")->size, mp_sizeof_bool(true), "size");
> +
> +	error_payload_unset(&p, "key2");
> +	is(p.count, 1, "--count");
> +	ok(error_payload_get_bool(&p, "key1", &val) && val, "remained value");
> +	ok(!error_payload_get_bool(&p, "key2", &val) && !val, "deleted value");
> +
> +	error_payload_set_str(&p, "key2", "true");
> +	ok(!error_payload_get_bool(&p, "key2", &val) && !val, "wrong type");
> +
> +	error_payload_destroy(&p);
> +
> +	check_plan();
> +	footer();
> +}
> +
> +static void
> +test_payload_field_uuid(void)
> +{
> +	header();
> +	plan(17);
> +
> +	struct error_payload p;
> +	error_payload_create(&p);
> +	struct tt_uuid val1;
> +	tt_uuid_create(&val1);
> +	ok(!error_payload_get_uuid(&p, "key", &val1), "get_uuid() empty");
> +	ok(tt_uuid_is_nil(&val1), "default value");
> +
> +	tt_uuid_create(&val1);
> +	error_payload_set_uuid(&p, "key1", &val1);
> +	is(p.count, 1, "++count");
> +	struct tt_uuid val2;
> +	ok(error_payload_get_uuid(&p, "key1", &val2), "get_uuid() finds");
> +	ok(tt_uuid_is_equal(&val1, &val2), "value match");
> +
> +	ok(!error_payload_get_uuid(&p, "key2", &val2), "get_uuid() fails");
> +	ok(tt_uuid_is_nil(&val2), "value is default");
> +
> +	tt_uuid_create(&val2);
> +	error_payload_set_uuid(&p, "key2", &val2);
> +	struct tt_uuid val3;
> +	ok(error_payload_get_uuid(&p, "key2", &val3), "get_uuid() finds");
> +	ok(tt_uuid_is_equal(&val3, &val2), "value match");
> +
> +	is(error_payload_find(&p, "key1")->size, mp_sizeof_uuid(), "size");
> +
> +	error_payload_unset(&p, "key2");
> +	is(p.count, 1, "--count");
> +	ok(error_payload_get_uuid(&p, "key1", &val3), "remained value");
> +	ok(tt_uuid_is_equal(&val1, &val3), "value match");
> +	ok(!error_payload_get_uuid(&p, "key2", &val3), "deleted value");
> +	ok(tt_uuid_is_nil(&val3), "value match");
> +
> +	error_payload_set_str(&p, "key2", "1");
> +	ok(!error_payload_get_uuid(&p, "key2", &val3), "wrong type");
> +	ok(tt_uuid_is_nil(&val3), "value match");
> +
> +	error_payload_destroy(&p);
> +
> +	check_plan();
> +	footer();
> +}
> +
> +static void
> +test_payload_field_mp(void)
> +{
> +	header();
> +	plan(6);
> +	char buf[1024];
> +	char *data;
> +	const char *cdata;
> +	uint32_t size;
> +
> +	struct error_payload p;
> +	error_payload_create(&p);
> +
> +	data = mp_encode_str(buf, "value1", 6);
> +	error_payload_set_mp(&p, "key1", buf, data - buf);
> +	is(strcmp(error_payload_get_str(&p, "key1"), "value1"), 0, "mp str");
> +
> +	cdata = error_payload_get_mp(&p, "key1", &size);
> +	is(memcmp(cdata, buf, size), 0, "mp str cmp");
> +
> +	data = mp_encode_uint(buf, 100);
> +	error_payload_set_mp(&p, "key2", buf, data - buf);
> +	uint64_t val;
> +	ok(error_payload_get_uint(&p, "key2", &val) && val == 100, "mp uint");
> +
> +	cdata = error_payload_get_mp(&p, "key2", &size);
> +	is(memcmp(cdata, buf, size), 0, "mp uint cmp");
> +
> +	data = mp_encode_array(buf, 1);
> +	data = mp_encode_uint(data, 2);
> +	error_payload_set_mp(&p, "key3", buf, data - buf);
> +
> +	cdata = error_payload_get_mp(&p, "key3", &size);
> +	is(memcmp(cdata, buf, size), 0, "mp array");
> +
> +	ok(!error_payload_get_uint(&p, "key3", &val) && val == 0,
> +	   "mp uint from array");
> +
> +	error_payload_destroy(&p);
> +
> +	check_plan();
> +	footer();
> +}
> +
> +static void
> +test_payload_unset(void)
> +{
> +	header();
> +	plan(13);
> +
> +	struct error_payload p;
> +	error_payload_create(&p);
> +
> +	error_payload_set_uint(&p, "key1", 1);
> +	error_payload_set_uint(&p, "key2", 2);
> +	error_payload_set_uint(&p, "key3", 3);
> +	error_payload_set_uint(&p, "key4", 4);
> +	error_payload_set_uint(&p, "key5", 5);
> +
> +	error_payload_unset(&p, "key5");
> +	is(p.count, 4, "unset last, count");
> +	is(error_payload_find(&p, "key5"), NULL, "unset last, key");
> +
> +	error_payload_unset(&p, "key1");
> +	is(p.count, 3, "unset first, count");
> +	is(error_payload_find(&p, "key1"), NULL, "unset first, key");
> +
> +	uint64_t val;
> +	ok(error_payload_get_uint(&p, "key2", &val) && val == 2, "check key2");
> +	ok(error_payload_get_uint(&p, "key3", &val) && val == 3, "check key3");
> +	ok(error_payload_get_uint(&p, "key4", &val) && val == 4, "check key4");
> +
> +	is(strcmp(p.fields[0]->name, "key4"), 0, "deletion is cyclic");
> +
> +	error_payload_unset(&p, "key2");
> +	is(p.count, 2, "unset middle, count");
> +	is(error_payload_find(&p, "key2"), NULL, "unset middle, key");
> +	ok(error_payload_get_uint(&p, "key3", &val) && val == 3, "check key3");
> +	ok(error_payload_get_uint(&p, "key4", &val) && val == 4, "check key4");
> +
> +	error_payload_unset(&p, "key3");
> +	error_payload_unset(&p, "key4");
> +
> +	is(p.count, 0, "unset all");
> +
> +	error_payload_destroy(&p);
> +
> +	check_plan();
> +	footer();
> +}
> +
> +static void
> +test_payload_move(void)
> +{
> +	header();
> +	plan(7);
> +
> +	struct error_payload p1, p2;
> +	error_payload_create(&p1);
> +	error_payload_create(&p2);
> +
> +	error_payload_move(&p1, &p2);
> +	ok(p1.count == 0 && p1.fields == NULL, "empty");
> +
> +	error_payload_set_str(&p1, "key", "value");
> +	error_payload_move(&p1, &p2);
> +	ok(p1.count == 0 && p1.fields == NULL, "emptied on move");
> +
> +	error_payload_set_str(&p1, "key", "value");
> +	error_payload_set_str(&p2, "key1", "value1");
> +	error_payload_set_str(&p2, "key2", "value2");
> +	error_payload_move(&p1, &p2);
> +	is(p1.count, 2, "got 2 fields");
> +	isnt(p1.fields, NULL, "got 2 fields");
> +	is(strcmp(error_payload_get_str(&p1, "key1"), "value1"), 0, "key1");
> +	is(strcmp(error_payload_get_str(&p1, "key2"), "value2"), 0, "key2");
> +	is(error_payload_get_str(&p1, "key"), NULL, "key");
> +
> +	error_payload_destroy(&p2);
> +	error_payload_destroy(&p1);
> +
> +	check_plan();
> +	footer();
> +}
> +
> +int
> +main(void)
> +{
> +	header();
> +	plan(9);
> +
> +	random_init();
> +
> +	test_payload_field_str();
> +	test_payload_field_uint();
> +	test_payload_field_int();
> +	test_payload_field_double();
> +	test_payload_field_bool();
> +	test_payload_field_uuid();
> +	test_payload_field_mp();
> +	test_payload_unset();
> +	test_payload_move();
> +
> +	random_free();
> +
> +	footer();
> +	return check_plan();
> +}
> diff --git a/test/unit/error.result b/test/unit/error.result
> new file mode 100644
> index 000000000..f0c4266e6
> --- /dev/null
> +++ b/test/unit/error.result
> @@ -0,0 +1,160 @@
> +	*** main ***
> +1..9
> +	*** test_payload_field_str ***
> +    1..15
> +    ok 1 - no fields in the beginning
> +    ok 2 - get_str() empty
> +    ok 3 - ++count
> +    ok 4 - get_str() finds
> +    ok 5 - ++count
> +    ok 6 - get_str() finds old
> +    ok 7 - get_str() finds new
> +    ok 8 - size does not include terminating zero
> +    ok 9 - field count is same
> +    ok 10 - get_str() finds new value
> +    ok 11 - get_str() finds other key old value
> +    ok 12 - --count
> +    ok 13 - get_str() finds new value
> +    ok 14 - get_str() can't find deleted value
> +    ok 15 - wrong type
> +ok 1 - subtests
> +	*** test_payload_field_str: done ***
> +	*** test_payload_field_uint ***
> +    1..17
> +    ok 1 - get_uint() empty
> +    ok 2 - ++count
> +    ok 3 - get_uint() finds
> +    ok 4 - value match
> +    ok 5 - get_uint() fails
> +    ok 6 - value is default
> +    ok 7 - small number size
> +    ok 8 - get_uint() 32 bit
> +    ok 9 - value match
> +    ok 10 - middle number size
> +    ok 11 - field count is same
> +    ok 12 - value 1
> +    ok 13 - value 2
> +    ok 14 - --count
> +    ok 15 - remained value
> +    ok 16 - deleted value
> +    ok 17 - wrong type
> +ok 2 - subtests
> +	*** test_payload_field_uint: done ***
> +	*** test_payload_field_int ***
> +    1..20
> +    ok 1 - get_int() empty
> +    ok 2 - ++count
> +    ok 3 - get_int() finds
> +    ok 4 - value match
> +    ok 5 - get_int() fails
> +    ok 6 - value is default
> +    ok 7 - small number size
> +    ok 8 - get_int() 32 bit
> +    ok 9 - value match
> +    ok 10 - middle number size
> +    ok 11 - field count is same
> +    ok 12 - value 1
> +    ok 13 - value 2
> +    ok 14 - --count
> +    ok 15 - remained value
> +    ok 16 - deleted value
> +    ok 17 - wrong type
> +    ok 18 - overflow
> +    ok 19 - conversion
> +    ok 20 - min value
> +ok 3 - subtests
> +	*** test_payload_field_int: done ***
> +	*** test_payload_field_double ***
> +    1..13
> +    ok 1 - get_double() empty
> +    ok 2 - ++count
> +    ok 3 - get_double() finds
> +    ok 4 - value match
> +    ok 5 - get_double() fails
> +    ok 6 - value is default
> +    ok 7 - size
> +    ok 8 - value 1
> +    ok 9 - value 2
> +    ok 10 - --count
> +    ok 11 - remained value
> +    ok 12 - deleted value
> +    ok 13 - wrong type
> +ok 4 - subtests
> +	*** test_payload_field_double: done ***
> +	*** test_payload_field_bool ***
> +    1..13
> +    ok 1 - get_bool() empty
> +    ok 2 - ++count
> +    ok 3 - get_bool() finds
> +    ok 4 - value match
> +    ok 5 - get_bool() fails
> +    ok 6 - value is default
> +    ok 7 - get_bool() finds
> +    ok 8 - value match
> +    ok 9 - size
> +    ok 10 - --count
> +    ok 11 - remained value
> +    ok 12 - deleted value
> +    ok 13 - wrong type
> +ok 5 - subtests
> +	*** test_payload_field_bool: done ***
> +	*** test_payload_field_uuid ***
> +    1..17
> +    ok 1 - get_uuid() empty
> +    ok 2 - default value
> +    ok 3 - ++count
> +    ok 4 - get_uuid() finds
> +    ok 5 - value match
> +    ok 6 - get_uuid() fails
> +    ok 7 - value is default
> +    ok 8 - get_uuid() finds
> +    ok 9 - value match
> +    ok 10 - size
> +    ok 11 - --count
> +    ok 12 - remained value
> +    ok 13 - value match
> +    ok 14 - deleted value
> +    ok 15 - value match
> +    ok 16 - wrong type
> +    ok 17 - value match
> +ok 6 - subtests
> +	*** test_payload_field_uuid: done ***
> +	*** test_payload_field_mp ***
> +    1..6
> +    ok 1 - mp str
> +    ok 2 - mp str cmp
> +    ok 3 - mp uint
> +    ok 4 - mp uint cmp
> +    ok 5 - mp array
> +    ok 6 - mp uint from array
> +ok 7 - subtests
> +	*** test_payload_field_mp: done ***
> +	*** test_payload_unset ***
> +    1..13
> +    ok 1 - unset last, count
> +    ok 2 - unset last, key
> +    ok 3 - unset first, count
> +    ok 4 - unset first, key
> +    ok 5 - check key2
> +    ok 6 - check key3
> +    ok 7 - check key4
> +    ok 8 - deletion is cyclic
> +    ok 9 - unset middle, count
> +    ok 10 - unset middle, key
> +    ok 11 - check key3
> +    ok 12 - check key4
> +    ok 13 - unset all
> +ok 8 - subtests
> +	*** test_payload_unset: done ***
> +	*** test_payload_move ***
> +    1..7
> +    ok 1 - empty
> +    ok 2 - emptied on move
> +    ok 3 - got 2 fields
> +    ok 4 - got 2 fields
> +    ok 5 - key1
> +    ok 6 - key2
> +    ok 7 - key
> +ok 9 - subtests
> +	*** test_payload_move: done ***
> +	*** main: done ***

-- 
Serge Petrenko