[PATCH v3 6/7] box: introduce field_map_builder class
Vladimir Davydov
vdavydov.dev at gmail.com
Wed Apr 3 17:38:06 MSK 2019
On Tue, Apr 02, 2019 at 06:49:37PM +0300, Kirill Shcherbatov wrote:
> diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
> index 070897ec2..9a643b700 100644
> --- a/src/box/tuple_format.c
> +++ b/src/box/tuple_format.c
> @@ -769,27 +769,21 @@ tuple_format1_can_store_format2_tuples(struct tuple_format *format1,
>
> /** @sa declaration for details. */
> int
> -tuple_field_map_create(struct tuple_format *format, const char *tuple,
> - bool validate, uint32_t **field_map,
> - uint32_t *field_map_size)
> +tuple_field_map_create(struct field_map_builder *builder,
> + struct tuple_format *format, const char *tuple,
> + bool validate)
The field map builder should be the last in the argument list as it is
an out parameter.
> {
> + struct region *region = &fiber()->gc;
> if (tuple_format_field_count(format) == 0) {
> - *field_map = NULL;
> - *field_map_size = 0;
> + /* Nothing to initialize */
> + if (field_map_builder_create(builder, 0, region) != 0)
> + unreachable();
Nit: no need in this branching. You could create a builder with
format->field_map_size in both cases.
> return 0; /* Nothing to initialize */
> }
> - struct region *region = &fiber()->gc;
> - *field_map_size = format->field_map_size;
> - *field_map = region_alloc(region, *field_map_size);
> - if (*field_map == NULL) {
> - diag_set(OutOfMemory, *field_map_size, "region_alloc",
> - "field_map");
> + if (field_map_builder_create(builder, format->field_map_size,
> + region) != 0)
> return -1;
> - }
> - *field_map = (uint32_t *)((char *)*field_map + *field_map_size);
> -
> const char *pos = tuple;
> - int rc = 0;
> /* Check to see if the tuple has a sufficient number of fields. */
> uint32_t field_count = mp_decode_array(&pos);
> if (validate && format->exact_field_count > 0 &&
> diff --git a/src/box/tuple_format.h b/src/box/tuple_format.h
> index bef1d0903..88f03d5eb 100644
> --- a/src/box/tuple_format.h
> +++ b/src/box/tuple_format.h
> @@ -36,6 +36,7 @@
> #include "errinj.h"
> #include "json/json.h"
> #include "tuple_dictionary.h"
> +#include "field_map.h"
>
> #if defined(__cplusplus)
> extern "C" {
> @@ -169,8 +170,9 @@ struct tuple_format {
> */
> bool is_ephemeral;
> /**
> - * Size of field map of tuple in bytes.
> - * \sa struct tuple
> + * Size of minimal field map of tuple where each indexed
> + * field has own offset slot (in bytes).
> + * \sa struct field_map_builder
> */
Minimal? What's that supposed to mean? Is it from the next patch?
Other than that, looks okay to me.
More information about the Tarantool-patches
mailing list