From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <alexander.turenko@tarantool.org>
Received: from smtp32.i.mail.ru (smtp32.i.mail.ru [94.100.177.92])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
 (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id BBE80469719
 for <tarantool-patches@dev.tarantool.org>;
 Fri, 16 Oct 2020 01:20:28 +0300 (MSK)
Date: Fri, 16 Oct 2020 01:20:49 +0300
From: Alexander Turenko <alexander.turenko@tarantool.org>
Message-ID: <20201015222049.52plw5y72pcavoyf@tkn_work_nb>
References: <cover.1602629628.git.tsafin@tarantool.org>
 <23f2b9468c529a6276ac75c1cfe81e60e1fabfb2.1602629628.git.tsafin@tarantool.org>
 <db10366a-44fc-d172-b6f3-9ab32228e7e9@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <db10366a-44fc-d172-b6f3-9ab32228e7e9@tarantool.org>
Subject: Re: [Tarantool-patches] [PATCH 2.X v4 4/4] module api: box_ibuf_*
 wrappers
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Cc: tarantool-patches@dev.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;
> > +
> > +}