Re: [Tarantool-patches] [PATCH v2] iproto: add an empty body to the unprepare response

[Vladislav Shpilevoy <v.shpilevoy@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;