Re: [Tarantool-patches] [PATCH v2 1/1] box: export box_session_push to the public C API

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hi! Thanks for the review!

>> diff --git a/src/box/lua/console.c b/src/box/lua/console.c
>> index 886120eac..ff2db5832 100644
>> --- a/src/box/lua/console.c
>> +++ b/src/box/lua/console.c
>> @@ -461,11 +465,10 @@ port_msgpack_dump_plain_via_lua(struct lua_State *L)
>>  	 */
>>  	luamp_decode(L, luaL_msgpack_default, &data);
>>  	data = console_dump_plain(L, size);
>> -	if (data == NULL) {
>> -		*result = NULL;
>> -		return 0;
>> -	}
>> -	*result = port_mspack_set_plain((struct port *)port, data, *size);
>> +	if (data == NULL)
>> +		assert(port->plain == NULL);
>> +	else
>> +		port_mspack_set_plain((struct port *)port, data, *size);
> 
> Typo: s/port_mspack_set_plain/port_msgpack_set_plain/g.

Thanks, fixed:

====================
diff --git a/src/box/call.c b/src/box/call.c
index 599df7f60..7c70d8169 100644
--- a/src/box/call.c
+++ b/src/box/call.c
@@ -91,7 +91,7 @@ port_msgpack_destroy(struct port *base)
 }
 
 int
-port_mspack_set_plain(struct port *base, const char *plain, uint32_t len)
+port_msgpack_set_plain(struct port *base, const char *plain, uint32_t len)
 {
 	struct port_msgpack *port = (struct port_msgpack *)base;
 	assert(port->plain == NULL);
diff --git a/src/box/lua/console.c b/src/box/lua/console.c
index ff2db5832..c647c39d7 100644
--- a/src/box/lua/console.c
+++ b/src/box/lua/console.c
@@ -468,7 +468,7 @@ port_msgpack_dump_plain_via_lua(struct lua_State *L)
 	if (data == NULL)
 		assert(port->plain == NULL);
 	else
-		port_mspack_set_plain((struct port *)port, data, *size);
+		port_msgpack_set_plain((struct port *)port, data, *size);
 	return 0;
  }
 
diff --git a/src/box/port.h b/src/box/port.h
index 32d13f589..fa6c1374d 100644
--- a/src/box/port.h
+++ b/src/box/port.h
@@ -108,7 +108,7 @@ port_msgpack_destroy(struct port *base);
  * Set plain text version of data in the given port. It is copied.
  */
 int
-port_mspack_set_plain(struct port *base, const char *plain, uint32_t len);
+port_msgpack_set_plain(struct port *base, const char *plain, uint32_t len);
 
 /** Port for storing the result of a Lua CALL/EVAL. */
 struct port_lua {
====================

> The result value is not checked, but <port_msgpack_set_plain> returns -1
> when OOM occurs.

No need to check result. port_mspack_set_plain() not only returns -1, it
also keeps port->plain NULL. If it is NULL, it is an error.

port_mspack_dump_plain() returns port->plain. Therefore if there was an
error, it will return NULL with a correctly installed diag.