Re: [Tarantool-patches] [PATCH 2/3] box: introduce port_c

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

Hi! Thanks for the review!

>> diff --git a/src/box/port.c b/src/box/port.c
>> index 6e2fe3a6e..925a4b2d5 100644
>> --- a/src/box/port.c
>> +++ b/src/box/port.c
>> @@ -127,28 +132,194 @@ port_tuple_dump_msgpack(struct port *base, struct obuf *out)
>>  extern void
>>  port_tuple_dump_lua(struct port *base, struct lua_State *L, bool is_flat);
>>  
> 
> <snipped>
> 
>> +
>> +static void
>> +port_c_destroy(struct port *base)
>> +{
>> +	struct port_c *port = (struct port_c *)base;
>> +	struct port_c_entry *pe = port->first;
>> +	if (pe == NULL)
>> +		return;
>> +	port_c_destroy_entry(pe);
> 
> Minor: It's worth to drop a few words here that the port->first points
> to port->first_entry and it is released with the port_c structure.

No problem:

====================
diff --git a/src/box/port.c b/src/box/port.c
index 925a4b2d5..124625c7e 100644
--- a/src/box/port.c
+++ b/src/box/port.c
@@ -151,6 +151,11 @@ port_c_destroy(struct port *base)
 	if (pe == NULL)
 		return;
 	port_c_destroy_entry(pe);
+	/*
+	 * Port->first is skipped, it is pointing at
+	 * port_c.first_entry, and is freed together with the
+	 * port.
+	 */
 	pe = pe->next;
 	while (pe != NULL) {
 		struct port_c_entry *cur = pe;

====================

>> +	pe = pe->next;
>> +	while (pe != NULL) {
>> +		struct port_c_entry *cur = pe;
>> +		pe = pe->next;
>> +		port_c_destroy_entry(cur);
>> +		mempool_free(&port_entry_pool, cur);
>> +	}
>> +}
>> +
> 
> <snipped>
> 
>> +
>> +int
>> +port_c_add_mp(struct port *base, const char *mp, const char *mp_end)
>> +{
>> +	struct port_c *port = (struct port_c *)base;
>> +	struct port_c_entry *pe;
>> +	uint32_t size = mp_end - mp;
> 
> Minor: since there are no guarantees except the existing contract and
> common sense making this value be non-negative, I propose to add a
> corresponding assert. Feel free to ignore.

I don't mind.

====================
diff --git a/src/box/port.c b/src/box/port.c
index 925a4b2d5..3498f7af0 100644
--- a/src/box/port.c
+++ b/src/box/port.c
@@ -202,6 +207,7 @@ port_c_add_mp(struct port *base, const char *mp, const char *mp_end)
 {
 	struct port_c *port = (struct port_c *)base;
 	struct port_c_entry *pe;
+	assert(mp_end > mp);
 	uint32_t size = mp_end - mp;
 	char *dst;
 	if (size <= PORT_ENTRY_SIZE) {

====================