[tarantool-patches] Re: [PATCH 1/2] sql: refactor SQLITE_IOERR_ error codes

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

Thanks for the patch! See 2 comments below.

On 02/10/2018 23:50, AlexeyIvushkin wrote:
> From: Morgan-iv <ivushkinalex@gmail.com>
> 
> Remove error codes with prefix SQLITE_IOERR_ that only defined,
> but don't used anywhere.
> Replace '#define' macro with enum statement in implementation of
> SQLITE_IOERR_ error codes
> Remove unused function xDelete from sqlite3_vfs interface and its
> implementation, unixDelete. Also remove function openDirectory, used
> only in unixDelete
> 
> Part of #3315
> ---
>   src/box/sql/main.c      | 42 -----------------------
>   src/box/sql/os_unix.c   | 88 -------------------------------------------------
>   src/box/sql/sqliteInt.h | 43 +++++++++---------------
>   3 files changed, 15 insertions(+), 158 deletions(-)
> 
> diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
> index 53188e7..00c0309 100644
> --- a/src/box/sql/sqliteInt.h
> +++ b/src/box/sql/sqliteInt.h
> @@ -605,33 +604,21 @@ sqlite3_exec(sqlite3 *,	/* An open database */
>   	     void *,	/* 1st argument to callback */
>   	     char **errmsg	/* Error msg written here */
>   	);
> -#define SQLITE_IOERR_READ              (SQLITE_IOERR | (1<<8))
> -#define SQLITE_IOERR_SHORT_READ        (SQLITE_IOERR | (2<<8))
> -#define SQLITE_IOERR_WRITE             (SQLITE_IOERR | (3<<8))
> -#define SQLITE_IOERR_FSYNC             (SQLITE_IOERR | (4<<8))
> -#define SQLITE_IOERR_DIR_FSYNC         (SQLITE_IOERR | (5<<8))
> -#define SQLITE_IOERR_TRUNCATE          (SQLITE_IOERR | (6<<8))
> -#define SQLITE_IOERR_FSTAT             (SQLITE_IOERR | (7<<8))
> -#define SQLITE_IOERR_UNLOCK            (SQLITE_IOERR | (8<<8))
> -#define SQLITE_IOERR_RDLOCK            (SQLITE_IOERR | (9<<8))
> -#define SQLITE_IOERR_DELETE            (SQLITE_IOERR | (10<<8))
> -#define SQLITE_IOERR_BLOCKED           (SQLITE_IOERR | (11<<8))
> -#define SQLITE_IOERR_NOMEM             (SQLITE_IOERR | (12<<8))
> -#define SQLITE_IOERR_ACCESS            (SQLITE_IOERR | (13<<8))
> -#define SQLITE_IOERR_CHECKRESERVEDLOCK (SQLITE_IOERR | (14<<8))
> -#define SQLITE_IOERR_LOCK              (SQLITE_IOERR | (15<<8))
> -#define SQLITE_IOERR_CLOSE             (SQLITE_IOERR | (16<<8))
> -#define SQLITE_IOERR_DIR_CLOSE         (SQLITE_IOERR | (17<<8))
> -#define SQLITE_IOERR_SHMOPEN           (SQLITE_IOERR | (18<<8))
> -#define SQLITE_IOERR_SHMSIZE           (SQLITE_IOERR | (19<<8))
> -#define SQLITE_IOERR_SHMLOCK           (SQLITE_IOERR | (20<<8))
> -#define SQLITE_IOERR_SHMMAP            (SQLITE_IOERR | (21<<8))
> -#define SQLITE_IOERR_SEEK              (SQLITE_IOERR | (22<<8))
> -#define SQLITE_IOERR_DELETE_NOENT      (SQLITE_IOERR | (23<<8))
> -#define SQLITE_IOERR_MMAP              (SQLITE_IOERR | (24<<8))
> -#define SQLITE_IOERR_GETTEMPPATH       (SQLITE_IOERR | (25<<8))
> -#define SQLITE_IOERR_CONVPATH          (SQLITE_IOERR | (26<<8))
> -#define SQLITE_IOERR_VNODE             (SQLITE_IOERR | (27<<8))
> +
> +enum sql_ioerr_code {> +	SQLITE_IOERR_READ              = (SQLITE_IOERR | (1<<8)),

1. Please, read the issue title more attentive:

	"sql: remove SQLITE_ prefix from error codes".

But after your patch they still are SQLITE_.

> +	SQLITE_IOERR_SHORT_READ        = (SQLITE_IOERR | (2<<8)),
> +	SQLITE_IOERR_WRITE             = (SQLITE_IOERR | (3<<8)),
> +	SQLITE_IOERR_TRUNCATE          = (SQLITE_IOERR | (4<<8)),
> +	SQLITE_IOERR_FSTAT             = (SQLITE_IOERR | (5<<8)),
> +	SQLITE_IOERR_UNLOCK            = (SQLITE_IOERR | (6<<8)),
> +	SQLITE_IOERR_RDLOCK            = (SQLITE_IOERR | (7<<8)),
> +	SQLITE_IOERR_NOMEM             = (SQLITE_IOERR | (8<<8)),
> +	SQLITE_IOERR_ACCESS            = (SQLITE_IOERR | (9<<8)),
> +	SQLITE_IOERR_CLOSE             = (SQLITE_IOERR | (10<<8)),
> +	SQLITE_IOERR_GETTEMPPATH       = (SQLITE_IOERR | (11<<8))
> +};

2. You removed not all unused error codes. I found unused
SQLITE_IOERR_CLOSE. Please, find others if they exist.

> +
>   #define SQLITE_CONSTRAINT_CHECK        (SQLITE_CONSTRAINT | (1<<8))
>   #define SQLITE_CONSTRAINT_FOREIGNKEY   (SQLITE_CONSTRAINT | (3<<8))
>   #define SQLITE_CONSTRAINT_FUNCTION     (SQLITE_CONSTRAINT | (4<<8))
>