From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp5.mail.ru (smtp5.mail.ru [94.100.179.24]) (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 25E81469719 for ; Wed, 4 Mar 2020 01:33:41 +0300 (MSK) References: <20200303161649.62470-1-k.sosnin@tarantool.org> From: Vladislav Shpilevoy Message-ID: Date: Tue, 3 Mar 2020 23:33:38 +0100 MIME-Version: 1.0 In-Reply-To: <20200303161649.62470-1-k.sosnin@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v2] iproto: add an empty body to the unprepare response List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Chris Sosnin , tarantool-patches@dev.tarantool.org, korablev@tarantool.org Hi! Thanks for the patch! On 03/03/2020 17:16, Chris Sosnin wrote: > Absence of the body in the unprepare response forces users to perform > additional checks to avoid errors. Adding an empty body fixes this problem. > > Closes #4769 > --- > branch: https://github.com/tarantool/tarantool/tree/ksosnin/gh-4769-unprepare-response-body > issue: https://github.com/tarantool/tarantool/issues/4769 > > As Nikita suggested, I created box/iproto.test.lua, and basically > inserted wrappers for requests testing from box-py for future usage. > > src/box/iproto.cc | 17 ++++---- > test/box/iproto.result | 86 ++++++++++++++++++++++++++++++++++++++++ > test/box/iproto.test.lua | 54 +++++++++++++++++++++++++ > 3 files changed, 150 insertions(+), 7 deletions(-) > create mode 100644 test/box/iproto.result > create mode 100644 test/box/iproto.test.lua > > diff --git a/src/box/iproto.cc b/src/box/iproto.cc > index f313af6ae..d5891391d 100644 > --- a/src/box/iproto.cc > +++ b/src/box/iproto.cc > @@ -1781,20 +1781,23 @@ tx_process_sql(struct cmsg *m) > */ > out = msg->connection->tx.p_obuf; > struct obuf_svp header_svp; > + if (is_unprepare) { Looks like you don't need that flag anymore. Consider the diff: ==================== diff --git a/src/box/iproto.cc b/src/box/iproto.cc index d5891391d..28095f5b8 100644 --- a/src/box/iproto.cc +++ b/src/box/iproto.cc @@ -1716,10 +1716,9 @@ tx_process_sql(struct cmsg *m) struct obuf *out; struct port port; struct sql_bind *bind = NULL; - int bind_count = 0; + int rc, bind_count = 0; const char *sql; uint32_t len; - bool is_unprepare = false; if (tx_check_schema(msg->header.schema_version)) goto error; @@ -1771,7 +1770,12 @@ tx_process_sql(struct cmsg *m) uint32_t stmt_id = mp_decode_uint(&sql); if (sql_unprepare(stmt_id) != 0) goto error; - is_unprepare = true; + out = msg->connection->tx.p_obuf; + if (iproto_reply_ok(out, msg->header.sync, + schema_version) != 0) + goto error; + iproto_wpos_create(&msg->wpos, out); + return; } } @@ -1781,23 +1785,17 @@ tx_process_sql(struct cmsg *m) */ out = msg->connection->tx.p_obuf; struct obuf_svp header_svp; - if (is_unprepare) { - if (iproto_reply_ok(out, msg->header.sync, schema_version) != 0) - goto error; - iproto_wpos_create(&msg->wpos, out); - return; - } /* Prepare memory for the iproto header. */ if (iproto_prepare_header(out, &header_svp, IPROTO_HEADER_LEN) != 0) { port_destroy(&port); goto error; } - if (port_dump_msgpack(&port, out) != 0) { - port_destroy(&port); + rc = port_dump_msgpack(&port, out); + port_destroy(&port); + if (rc != 0) { obuf_rollback_to_svp(out, &header_svp); goto error; } - port_destroy(&port); iproto_reply_sql(out, &header_svp, msg->header.sync, schema_version); iproto_wpos_create(&msg->wpos, out); return;