From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <alexander.turenko@tarantool.org>
Received: from smtp44.i.mail.ru (smtp44.i.mail.ru [94.100.177.104])
 (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 7A79C469719
 for <tarantool-patches@dev.tarantool.org>;
 Fri, 16 Oct 2020 00:42:05 +0300 (MSK)
Date: Fri, 16 Oct 2020 00:42:25 +0300
From: Alexander Turenko <alexander.turenko@tarantool.org>
Message-ID: <20201015214225.dwgcpi7fvizrkcem@tkn_work_nb>
References: <cover.1602629628.git.tsafin@tarantool.org>
 <23f2b9468c529a6276ac75c1cfe81e60e1fabfb2.1602629628.git.tsafin@tarantool.org>
 <20201014033117.ipfnfm4bxfklhp4c@tkn_work_nb>
 <053201d6a33b$0c1c2f60$24548e20$@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <053201d6a33b$0c1c2f60$24548e20$@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: Timur Safin <tsafin@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org

On Fri, Oct 16, 2020 at 12:35:05AM +0300, Timur Safin wrote:
> After our discussions within messenger, I've updated ibuf.c to
> raise error if there is invalid argument passed, to follow
> current box_* functions convention. E.g.

Nit: it *sets* an error, not raise.

> 
> +void
> +box_ibuf_read_range(box_ibuf_t *ibuf, char ***rpos, char ***wpos)
> +{
> +       if (ibuf == NULL) {
> +               diag_set(ClientError, ER_ILLEGAL_PARAMS,
> +                        "Invalid ibuf argument");
> +               return;
> +       }
> +       if (rpos != NULL)
> +               *rpos = &ibuf->rpos;
> +       if (wpos != NULL)
> +               *wpos = &ibuf->wpos;
> +}

You should return a code (0 / -1) so. The diagnostics area is like
errno: a caller should lean on the code, not the diagnostics area to
determine whether an error took place. If it occurs, then the
diagnostics is guaranteed to exist and to be actual.