Re: [PATCH v3 6/7] box: introduce field_map_builder class

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

On Tue, Apr 02, 2019 at 06:49:37PM +0300, Kirill Shcherbatov wrote:
> +/**
> + * Set data offset for a field identified by unique offset_slot.
> + *
> + * The offset_slot argument must be negative and offset must be
> + * positive (by definition).
> + */
> +static inline void
> +field_map_builder_set_slot(struct field_map_builder *builder,
> +			   int32_t offset_slot, uint32_t offset)
> +{
> +	assert(offset_slot < 0);
> +	assert(offset > 0);

An assertion making sure that the offset is within the map would be
appreciated.

> +	builder->slots[offset_slot] = offset;
> +}

> @@ -126,11 +126,11 @@ tuple_validate_raw(struct tuple_format *format, const char *tuple)
>  
>  	struct region *region = &fiber()->gc;
>  	size_t region_svp = region_used(region);
> -	uint32_t *field_map, field_map_size;
> -	int rc = tuple_field_map_create(format, tuple, true, &field_map,
> -					&field_map_size);
> -	assert(rc != 0 || field_map_size == format->field_map_size);
> +	struct field_map_builder builder;
> +	int rc = tuple_field_map_create(&builder, format, tuple, true);
>  	region_truncate(region, region_svp);
> +	assert(rc != 0 ||
> +	       field_map_build_size(&builder) == format->field_map_size);

This assertion is rather pointless after the next patch, where you
replace '==' with '>='. Please remove.

>  	return rc;
>  }