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

  • From: Vladislav Shpilevoy <v.shpilevoy@xxxxxxxxxxxxx>
  • To: tarantool-patches@xxxxxxxxxxxxx, AlexeyIvushkin <ivushkinalex@xxxxxxxxx>
  • Date: Wed, 3 Oct 2018 23:06:57 +0300

Thanks for the patch! See 2 comments below.

On 02/10/2018 23:50, AlexeyIvushkin wrote:

From: Morgan-iv <ivushkinalex@xxxxxxxxx>

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))


Other related posts: