[tarantool-patches] Re: [PATCH 2/2] sql: fix tuple format leak

[n.pettik]

>1. If you use internal structures like struct iterator, then lets
>manage them using internal API - iterator_delete(). I know, that
>box_iterator_t is the same as struct iterator, but lets be
>consistent.

Fixed:

-               box_iterator_free(cursor->iter);
+               iterator_delete(cursor->iter);

>2. Lets store comments near to function declarations rather
>than implementations.

Fixed:

--- a/src/box/sql/cursor.c
+++ b/src/box/sql/cursor.c
@@ -33,17 +33,11 @@
 #include "tarantoolInt.h"
 #include "box/tuple.h"
 
-/**
- * Release tuple, free iterator, invalidate cursor's state.
- * Note that this routine doesn't nullify space and index:
- * it is also used during OP_NullRow opcode to refresh given
- * cursor.
- */

@@ -72,10 +66,6 @@ sqlite3CursorZero(BtCursor * p)
        memset(p, 0, sizeof(*p));
 }
 
-/**
- * Close a cursor and invalidate its state. In case of
- * ephemeral cursor, corresponding space should be dropped.
- */

--- a/src/box/sql/cursor.h
+++ b/src/box/sql/cursor.h
@@ -72,6 +72,10 @@ struct BtCursor {
 void sqlite3CursorZero(BtCursor *);
 void sqlite3CursorHintFlags(BtCursor *, unsigned);
 
+/**
+ * Close a cursor and invalidate its state. In case of
+ * ephemeral cursor, corresponding space should be dropped.
+ */
 void
 sql_cursor_close(struct BtCursor *cursor);
 int sqlite3CursorMovetoUnpacked(BtCursor *, UnpackedRecord * pUnKey, int *pRes);
@@ -80,6 +84,12 @@ int sqlite3CursorNext(BtCursor *, int *pRes);
 int sqlite3CursorPrevious(BtCursor *, int *pRes);
 int sqlite3CursorPayload(BtCursor *, u32 offset, u32 amt, void *);
 
+/**
+ * Release tuple, free iterator, invalidate cursor's state.
+ * Note that this routine doesn't nullify space and index:
+ * it is also used during OP_NullRow opcode to refresh given
+ * cursor.
+ */
 void
 sql_cursor_cleanup(struct BtCursor *cursor);