Re: [Tarantool-patches] [PATCH 2.X 1/7] module api: export box_tuple_validate

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

Hi! Thanks for the patch!

See 2 comments below.

On 24.09.2020 19:00, Timur Safin wrote:
> For external merger we need means to valudate tuple data,

1. valudate -> validate.

> thus exporting `box_tuple_validate` which is wrapper around
> `tuple_validate_raw` without revealing access to tuple
> internals.
> 
> diff --git a/src/box/tuple.h b/src/box/tuple.h
> index 53ae690cc..ed0501464 100644
> --- a/src/box/tuple.h
> +++ b/src/box/tuple.h
> @@ -283,6 +283,17 @@ box_tuple_update(box_tuple_t *tuple, const char *expr, const char *expr_end);
>  box_tuple_t *
>  box_tuple_upsert(box_tuple_t *tuple, const char *expr, const char *expr_end);
>  
> +/**
> + * Check tuple data correspondence to the space format.
> + * @param format Format to which the tuple must match.
> + * @param tuple  Tuple to validate.
> + *
> + * @retval  0 The tuple is valid.
> + * @retval -1 The tuple is invalid.

I see you are a doxygen-master, nice.

> + */
> +int
> +box_tuple_validate(box_tuple_format_t *format, box_tuple_t *tuple);

2. OCD mode on. I would propose either make tuple the first
argument, or rename it to box_tuple_format_validate_tuple().
So as to be consistent with our agreement, that if something
is a method of <type>, then the <type> argument goes first,
and the method name is <type>_<action>.

I see we currently have in the public API the functions:

	box_tuple_validate - your new function, a bit
		inconsistent.

	box_tuple_validate_key_parts - this should have been
		box_key_def_validate_tuple from the beginning,
		but we can't do anything about it now.

	box_key_def_validate_key - correct. Key_def goes first,
		and the name is consistent.

So if you will make box_tuple_validate consistent, we will have
more correct signatures (2/3) than incorrect, for validation
methods at least.