From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp53.i.mail.ru (smtp53.i.mail.ru [94.100.177.113]) (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 33176446425 for ; Tue, 31 Mar 2020 02:24:55 +0300 (MSK) References: From: Vladislav Shpilevoy Message-ID: Date: Tue, 31 Mar 2020 01:24:53 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v2 09/10] iproto: refactor error encoding with mpstream List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikita Pettik , tarantool-patches@dev.tarantool.org Thanks for the patch! > diff --git a/src/box/xrow.c b/src/box/xrow.c > index 5e3cb0709..256dd4d91 100644 > --- a/src/box/xrow.c > +++ b/src/box/xrow.c > @@ -478,46 +475,77 @@ iproto_reply_vote(struct obuf *out, const struct ballot *ballot, > int > iproto_reply_error(struct obuf *out, const struct error *e, uint64_t sync, > uint32_t schema_version) > { > - uint32_t msg_len = strlen(e->errmsg); > - uint32_t errcode = box_error_code(e); > - > - struct iproto_body_bin body = iproto_error_bin; > char *header = (char *)obuf_alloc(out, IPROTO_HEADER_LEN); > if (header == NULL) > return -1; > > + bool is_error = false; > + struct mpstream stream; > + mpstream_init(&stream, out, obuf_reserve_cb, obuf_alloc_cb, > + mpstream_error_handler, &is_error); > + > + uint32_t used = obuf_size(out); > + mpstream_iproto_encode_error(&stream, e); > + mpstream_flush(&stream); > + > + uint32_t errcode = box_error_code(e); > iproto_header_encode(header, iproto_encode_error(errcode), sync, > - schema_version, sizeof(body) + msg_len); > - body.v_data_len = mp_bswap_u32(msg_len); > + schema_version, obuf_size(out) - used); > + > /* Malformed packet appears to be a lesser evil than abort. */ > - return obuf_dup(out, &body, sizeof(body)) != sizeof(body) || > - obuf_dup(out, e->errmsg, msg_len) != msg_len ? -1 : 0; > + return is_error; The function is supposed to return 0/-1. So better keep this convention. You can declare is_error as 'int rc', and set it to -1 in mpstream_error_handler(). Should be fine. Or make it return is_error ? -1 : 0;