[tarantool-patches] Re: [PATCH 2/2] sql: refactor cursor closing routine
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Thu Apr 12 16:28:44 MSK 2018
See below a pair of comments.
> diff --git a/src/box/sql/cursor.c b/src/box/sql/cursor.c
> index 533bfb587..c38629f3b 100644
> --- a/src/box/sql/cursor.c
> +++ b/src/box/sql/cursor.c
> @@ -31,18 +31,26 @@
>
> #include "sqliteInt.h"
> #include "tarantoolInt.h"
> +#include "box/tuple.h"
>
> -/*
> - * Clear the current cursor position.
> +/**
> + * 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
> -sqlite3ClearCursor(BtCursor * pCur)
> +sql_cursor_cleanup(struct BtCursor *cursor)
> {
> - free(pCur->key);
> - pCur->key = 0;
> - pCur->iter = NULL;
> - pCur->last_tuple = NULL;
> - pCur->eState = CURSOR_INVALID;
> + if (cursor->iter)
> + box_iterator_free(cursor->iter);
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.
> diff --git a/src/box/sql/cursor.h b/src/box/sql/cursor.h
> index 5b8e7c29d..e5d2aae3a 100644
> --- a/src/box/sql/cursor.h
> +++ b/src/box/sql/cursor.h
> @@ -72,14 +72,16 @@ struct BtCursor {
> void sqlite3CursorZero(BtCursor *);
> void sqlite3CursorHintFlags(BtCursor *, unsigned);
>
> -int sqlite3CloseCursor(BtCursor *);
> +void
> +sql_cursor_close(struct BtCursor *cursor);
2. Lets store comments near to function declarations rather
than implementations.
More information about the Tarantool-patches
mailing list