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

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

> : > +
> : > +#include <trivia/util.h>
> : > +#include <small/ibuf.h>
> : 
> : How about just define an opaque structure?
> 
> Yeah, good point - we are introducing it here just to avoid to have 
> explicit dependency on it in header.

Hm. It is out of /* \cond public */ and /* \endcond public */ markers,
so it's fine from this point of view. But using of `struct ibuf;` in the
header instead of `#include <small/ibuf.h>` should decrease the compile
time a bit. In fact, the time difference is negligible, so the point is
more regarding rules of thumb and feeling about the code as 'clean'.

> 
> : 
> :  | struct ibuf;
> : 
> : But include small/ibuf.h explicitly in the .c file?
> : 

(Cited for convenience.)

> : > +	struct slab_cache *slabc = cord_slab_cache();
> : > +	assert(slabc != NULL);
> : > +	box_ibuf_t ibuf;
> : > +
> : > +	ibuf_create(&ibuf, slabc, 16320);
> : > +	assert(ibuf_used(&ibuf) == 0);
> : > +	box_ibuf_reserve(&ibuf, 65536);
> : > +	char **rpos;
> : > +	char **wpos;
> : > +	box_ibuf_read_range(&ibuf, &rpos, &wpos);
> : > +
> : > +	void *ptr = ibuf_alloc(&ibuf, 10);
> : > +	assert(ptr != NULL);
> : > +
> : > +	assert(ibuf_used(&ibuf) == 10);
> : > +	assert((*wpos - *rpos) == 10);
> : 
> : Now box_ibuf_read_range() should give the updated wpos.
> 
> Here I didn't get your point - wpos and rpos are pointers 
> to fields inside of ibuf structure, they will not change 
> regardless the number of calls to we perform. 

Let's show a sketch:

 | box_ibuf_read_range(&ibuf, &rpos, &wpos);
 | <...do some allocations...>
 | assert((*wpos - *rpos) == 10);

This test case verifies that the positions we obtained before the
allocations are valid after them. I proposed to also verify that
box_ibuf_read_range() will correctly return the positions after the
allocations:

 | box_ibuf_read_range(&ibuf, &rpos, &wpos);
 | <...do some allocations...>
 | assert((*wpos - *rpos) == 10);
 |
 | /* Renew positions and verify them. */
 | box_ibuf_read_range(&ibuf, &rpos, &wpos);
 | assert((*wpos - *rpos) == 10);

> 
> : 
> : > +
> : > +	ptr = ibuf_alloc(&ibuf, 10000);
> : > +	assert(ptr);
> : > +	assert(ibuf_used(&ibuf) == 10010);
> : > +	assert((*wpos - *rpos) == 10010);
> : 
> : Same here.

(Cited for convenience.)