Re: [Tarantool-patches] [PATCH 2.X v4 4/4] module api: box_ibuf_* wrappers

[Alexander Turenko <alexander.turenko@tarantool.org>]

> > +void
> > +box_ibuf_read_range(box_ibuf_t *ibuf, char ***rpos, char ***wpos)
> > +{
> > +	assert(ibuf != NULL);
> > +	if (ibuf == NULL)
> > +		return;
> > +	if (rpos != NULL)
> > +		*rpos = &ibuf->rpos;
> > +	if (wpos != NULL)
> > +		*wpos = &ibuf->wpos;
> 
> 3. I would better assume all the arguments are not NULL, and document
> it. Especially ibuf itself. We need some border where to stop the
> sanity checks, and this looks like an overkill already. For example,
> box_tuple_ref() also works with a pointer, but it does not check for
> it being not NULL. It is just stupid to call method of an object with
> that object passed as NULL. The same below.

I would allow 'end' pointer to be NULL in box_ibuf_write_range() at
least. When I want to do a reserve+fill in a loop, the end position is
not needed. The same for 'wpos' in box_ibuf_read_range(): say, you're
sure about the data and just want to use mp_next() or something like.

What about allowing to omit the second parameters: I don't know a case
for this. Maybe it actually worth to forbid to pass NULL to them.

> > +}
> > +
> > +void
> > +box_ibuf_write_range(box_ibuf_t *ibuf, char ***wpos, char ***end)
> > +{
> > +	if (ibuf == NULL)
> > +		return;
> > +	if (wpos != NULL)
> > +		*wpos = &ibuf->wpos;
> > +	if (end != NULL)
> > +		*end = &ibuf->end;
> > +
> > +}