From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 292EB2C0F5 for ; Tue, 3 Apr 2018 19:01:17 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id NHmI4n5F2QYG for ; Tue, 3 Apr 2018 19:01:17 -0400 (EDT) Received: from smtp33.i.mail.ru (smtp33.i.mail.ru [94.100.177.93]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 3DF2E2C0B9 for ; Tue, 3 Apr 2018 19:01:15 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Subject: [tarantool-patches] Re: [PATCH 2/4] sql: remove unused error codes, use enum instead of defines From: "n.pettik" In-Reply-To: Date: Wed, 4 Apr 2018 02:01:06 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <3FC50B7F-FE28-4F8C-A79C-C287FD6EA1CB@tarantool.org> References: Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: Vladislav Shpilevoy Cc: tarantool-patches@freelists.org Your subject doesn=E2=80=99t fit into 50 lines and consists from two = parts. Maybe better to move it to commit message? 2 minor comments: > On 3 Apr 2018, at 18:34, Vladislav Shpilevoy = wrote: >=20 > --- > src/box/sql/build.c | 4 --- > src/box/sql/main.c | 32 ------------------ > src/box/sql/os_unix.c | 9 +----- > src/box/sql/sqliteInt.h | 86 = ++++++++++++++++++++++++++++--------------------- > src/box/sql/vdbe.c | 1 - > src/box/sql/vdbeInt.h | 5 +-- > src/box/sql/vdbeapi.c | 9 ------ > src/box/sql/vdbeaux.c | 1 - > 8 files changed, 52 insertions(+), 95 deletions(-) >=20 > diff --git a/src/box/sql/build.c b/src/box/sql/build.c > index 5e3ed0f39..4510df206 100644 > --- a/src/box/sql/build.c > +++ b/src/box/sql/build.c > @@ -105,10 +105,6 @@ sqlite3FinishCoding(Parse * pParse) >=20 > if (db->init.busy =3D=3D 0) > sqlite3VdbeChangeP5(v, 1); > - > - VdbeComment((v, "usesStmtJournal=3D%d", > - pParse->mayAbort > - && pParse->isMultiWrite)); > } >=20 > /* Code constant expressions that where factored = out of inner loops */ > diff --git a/src/box/sql/main.c b/src/box/sql/main.c > index b77348c21..8e898178a 100644 > --- a/src/box/sql/main.c > +++ b/src/box/sql/main.c > @@ -800,9 +800,6 @@ sqlite3ErrName(int rc) > case SQLITE_ERROR: > zName =3D "SQLITE_ERROR"; > break; > - case SQLITE_INTERNAL: > - zName =3D "SQLITE_INTERNAL"; > - break; > case SQLITE_PERM: > zName =3D "SQLITE_PERM"; > break; > @@ -821,9 +818,6 @@ sqlite3ErrName(int rc) > case SQLITE_NOMEM: > zName =3D "SQLITE_NOMEM"; > break; > - case SQLITE_READONLY: > - zName =3D "SQLITE_READONLY"; > - break; > case SQLITE_INTERRUPT: > zName =3D "SQLITE_INTERRUPT"; > break; > @@ -917,12 +911,6 @@ sqlite3ErrName(int rc) > case SQLITE_CANTOPEN: > zName =3D "SQLITE_CANTOPEN"; > break; > - case SQLITE_PROTOCOL: > - zName =3D "SQLITE_PROTOCOL"; > - break; > - case SQLITE_EMPTY: > - zName =3D "SQLITE_EMPTY"; > - break; > case SQLITE_SCHEMA: > zName =3D "SQLITE_SCHEMA"; > break; > @@ -959,27 +947,15 @@ sqlite3ErrName(int rc) > case SQLITE_MISUSE: > zName =3D "SQLITE_MISUSE"; > break; > - case SQLITE_NOLFS: > - zName =3D "SQLITE_NOLFS"; > - break; > - case SQLITE_FORMAT: > - zName =3D "SQLITE_FORMAT"; > - break; > case SQLITE_RANGE: > zName =3D "SQLITE_RANGE"; > break; > - case SQLITE_NOTADB: > - zName =3D "SQLITE_NOTADB"; > - break; > case SQL_TARANTOOL_ERROR: > zName =3D "SQLITE_TARANTOOL_ERROR"; > break; > case SQLITE_ROW: > zName =3D "SQLITE_ROW"; > break; > - case SQLITE_NOTICE: > - zName =3D "SQLITE_NOTICE"; > - break; > case SQLITE_WARNING: > zName =3D "SQLITE_WARNING"; > break; > @@ -1008,32 +984,24 @@ sqlite3ErrStr(int rc) > static const char *const aMsg[] =3D { > /* SQLITE_OK */ "not an error", > /* SQLITE_ERROR */ "SQL logic error or missing = database", > - /* SQLITE_INTERNAL */ 0, > /* SQLITE_PERM */ "access permission denied", > /* SQLITE_ABORT */ "callback requested query = abort", > /* SQLITE_BUSY */ "database is locked", > /* SQLITE_LOCKED */ "database table is locked", > /* SQLITE_NOMEM */ "out of memory", > - /* SQLITE_READONLY */ "attempt to write a readonly = database", > /* SQLITE_INTERRUPT */ "interrupted", > /* SQLITE_IOERR */ "disk I/O error", > /* SQLITE_CORRUPT */ "database disk image is = malformed", > /* SQLITE_NOTFOUND */ "unknown operation", > /* SQLITE_FULL */ "database or disk is full", > /* SQLITE_CANTOPEN */ "unable to open database file", > - /* SQLITE_PROTOCOL */ "locking protocol", > - /* SQLITE_EMPTY */ "table contains no data", > /* SQLITE_SCHEMA */ "database schema has changed", > /* SQLITE_TOOBIG */ "string or blob too big", > /* SQLITE_CONSTRAINT */ "constraint failed", > /* SQLITE_MISMATCH */ "datatype mismatch", > /* SQLITE_MISUSE */ > "library routine called out of sequence", > - /* SQLITE_NOLFS */ "large file support is = disabled", > - /* SQLITE_FORMAT */ "auxiliary database format = error", > /* SQLITE_RANGE */ "bind or column index out of = range", > - /* SQLITE_NOTADB */ > - "file is encrypted or is not a database", > /* SQL_TARANTOOL_ITERATOR_FAIL */ "Tarantool's iterator = failed", > /* SQL_TARANTOOL_INSERT_FAIL */ "Tarantool's insert = failed", > /* SQL_TARANTOOL_DELETE_FAIL */ "Tarantool's delete = failed", > diff --git a/src/box/sql/os_unix.c b/src/box/sql/os_unix.c > index 8dce6f0c2..9cb8a54d0 100644 > --- a/src/box/sql/os_unix.c > +++ b/src/box/sql/os_unix.c > @@ -1011,10 +1011,6 @@ findInodeInfo(unixFile * pFile, /* Unix = file with file desc used in the key */ > rc =3D osFstat(fd, &statbuf); > if (rc !=3D 0) { > storeLastErrno(pFile, errno); > -#if defined(EOVERFLOW) && defined(SQLITE_DISABLE_LFS) > - if (pFile->lastErrno =3D=3D EOVERFLOW) > - return SQLITE_NOLFS; > -#endif > return SQLITE_IOERR; > } > #ifdef __APPLE__ > @@ -6011,10 +6007,7 @@ proxyFileControl(sqlite3_file * id, int op, = void *pArg) > int isProxyStyle =3D (pFile->pMethod =3D=3D = &proxyIoMethods); > if (pArg =3D=3D NULL || (const char *)pArg =3D=3D = 0) { > if (isProxyStyle) { > - /* turn off proxy locking - not = supported. */ > - rc =3D SQLITE_ERROR > - /*SQLITE_PROTOCOL? = SQLITE_MISUSE? */ > - ; > + rc =3D SQLITE_ERROR; > } else { > /* turn off proxy locking - = already off - NOOP */ > rc =3D SQLITE_OK; > diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h > index e786cf842..f78a93a71 100644 > --- a/src/box/sql/sqliteInt.h > +++ b/src/box/sql/sqliteInt.h > @@ -405,42 +405,56 @@ struct sqlite3_vfs { > #define SQLITE_LIMIT_TRIGGER_DEPTH 9 > #define SQLITE_LIMIT_WORKER_THREADS 10 >=20 > -#define SQLITE_OK 0 /* Successful result */ > -/* beginning-of-error-codes */ > -#define SQLITE_ERROR 1 /* SQL error or missing database = */ > -#define SQLITE_INTERNAL 2 /* Internal logic error in = SQLite */ > -#define SQLITE_PERM 3 /* Access permission denied */ > -#define SQLITE_ABORT 4 /* Callback routine requested an = abort */ > -#define SQLITE_BUSY 5 /* The database file is locked = */ > -#define SQLITE_LOCKED 6 /* A table in the database is = locked */ > -#define SQLITE_NOMEM 7 /* A malloc() failed */ > -#define SQLITE_READONLY 8 /* Attempt to write a readonly = database */ > -#define SQLITE_INTERRUPT 9 /* Operation terminated by = sqlite3_interrupt() */ > -#define SQLITE_IOERR 10 /* Some kind of disk I/O error = occurred */ > -#define SQLITE_CORRUPT 11 /* The database disk image is = malformed */ > -#define SQLITE_NOTFOUND 12 /* Unknown opcode in = sqlite3_file_control() */ > -#define SQLITE_FULL 13 /* Insertion failed because = database is full */ > -#define SQLITE_CANTOPEN 14 /* Unable to open the database = file */ > -#define SQLITE_PROTOCOL 15 /* Database lock protocol error = */ > -#define SQLITE_EMPTY 16 /* Database is empty */ > -#define SQLITE_SCHEMA 17 /* The database schema changed = */ > -#define SQLITE_TOOBIG 18 /* String or BLOB exceeds size = limit */ > -#define SQLITE_CONSTRAINT 19 /* Abort due to constraint = violation */ > -#define SQLITE_MISMATCH 20 /* Data type mismatch */ > -#define SQLITE_MISUSE 21 /* Library used incorrectly */ > -#define SQLITE_NOLFS 22 /* Uses OS features not = supported on host */ > -#define SQLITE_FORMAT 23 /* Auxiliary database format = error */ > -#define SQLITE_RANGE 24 /* 2nd parameter to sqlite3_bind = out of range */ > -#define SQLITE_NOTADB 25 /* File opened that is not a = database file */ > -#define SQL_TARANTOOL_ITERATOR_FAIL 26 > -#define SQL_TARANTOOL_INSERT_FAIL 27 > -#define SQL_TARANTOOL_DELETE_FAIL 28 > -#define SQL_TARANTOOL_ERROR 29 > -#define SQLITE_NOTICE 31 /* Notifications from = sqlite3_log() */ > -#define SQLITE_WARNING 32 /* Warnings from sqlite3_log() = */ > -#define SQLITE_ROW 100 /* sqlite3_step() has another = row ready */ > -#define SQLITE_DONE 101 /* sqlite3_step() has finished = executing */ > -/* end-of-error-codes */ > +enum sql_ret_code { > + /** Result of a routine is ok. */ > + SQLITE_OK =3D 0, > + /** Common error code. */ > + SQLITE_ERROR, > + /** Access permission denied. */ > + SQLITE_PERM, > + /** Callback routine requested an abort. */ > + SQLITE_ABORT, > + /** The database file is locked. */ > + SQLITE_BUSY, > + /** A table in the database is locked. */ > + SQLITE_LOCKED, > + /** A malloc() failed. */ > + SQLITE_NOMEM, > + /** Operation terminated by sqlite3_interrupt(). */ > + SQLITE_INTERRUPT, > + /** Some kind of disk I/O error occurred. */ > + SQLITE_IOERR, > + /** The database disk image is malformed. */ > + SQLITE_CORRUPT, > + /** Unknown opcode in sqlite3_file_control(). */ > + SQLITE_NOTFOUND, > + /** Insertion failed because database is full. */ > + SQLITE_FULL, > + /** Unable to open the database file. */ > + SQLITE_CANTOPEN, > + /** The database schema changed. */ > + SQLITE_SCHEMA, > + /** String or BLOB exceeds size limit. */ > + SQLITE_TOOBIG, > + /** Abort due to constraint violation. */ > + SQLITE_CONSTRAINT, > + /** Data type mismatch. */ > + SQLITE_MISMATCH, > + /** Library used incorrectly. */ > + SQLITE_MISUSE, > + /** 2nd parameter to sqlite3_bind out of range. */ > + SQLITE_RANGE, > + SQL_TARANTOOL_ITERATOR_FAIL, > + SQL_TARANTOOL_INSERT_FAIL, > + SQL_TARANTOOL_DELETE_FAIL, > + SQL_TARANTOOL_ERROR, > + /** Warnings from sqlite3_log(). */ > + SQLITE_WARNING, > + /** sqlite3_step() has another row ready. */ > + SQLITE_ROW, > + /** sqlite3_step() has finished executing. */ > + SQLITE_DONE, > +}; I do like move to enum, but what about removing this obsolete = =E2=80=99SQLITE=E2=80=99 prefix? Let it be just =E2=80=99SQL_=E2=80=99. Moreover, some error seems to be = SQLite specific. Great time to remove them, isn=E2=80=99t it? If you don=E2=80=99t want or don=E2=80=99t have time for this boring = routine, you can create issue with =E2=80=98good first issue=E2=80=99 label. >=20 > void * > sqlite3_malloc(int); > diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c > index bf8a27709..1685c27ce 100644 > --- a/src/box/sql/vdbe.c > +++ b/src/box/sql/vdbe.c > @@ -1419,7 +1419,6 @@ case OP_ResultRow: { > */ > if (SQLITE_OK!=3D(rc =3D sqlite3VdbeCheckFk(p, 0))) { > assert(user_session->sql_flags&SQLITE_CountRows); > - assert(p->usesStmtJournal); > goto abort_due_to_error; > } >=20 > diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h > index fcb45c8a8..f8f4e566b 100644 > --- a/src/box/sql/vdbeInt.h > +++ b/src/box/sql/vdbeInt.h > @@ -418,9 +418,6 @@ struct Vdbe { > i64 startTime; /* Time when query started - used for = profiling */ > #endif > int nOp; /* Number of instructions in the program = */ > -#ifdef SQLITE_DEBUG > - int rcApp; /* errcode set by = sqlite3_result_error_code() */ > -#endif > u16 nResColumn; /* Number of columns in one row of the = result set */ > u8 errorAction; /* Recovery action to do in case of an = error */ > bft expired:1; /* True if the VM needs to be recompiled = */ > @@ -428,7 +425,6 @@ struct Vdbe { > bft explain:2; /* True if EXPLAIN present on SQL = command */ > bft changeCntOn:1; /* True to update the change-counter */ > bft runOnlyOnce:1; /* Automatically expire on reset */ > - bft usesStmtJournal:1; /* True if uses a statement journal */ > bft isPrepareV2:1; /* True if prepared with prepare_v2() */ > u32 aCounter[5]; /* Counters used by = sqlite3_stmt_status() */ > char *zSql; /* Text of the SQL statement that = generated this */ > @@ -446,6 +442,7 @@ struct Vdbe { > int nScan; /* Entries in aScan[] */ > ScanStatus *aScan; /* Scan definitions for = sqlite3_stmt_scanstatus() */ > #endif > + Nitpicking: extra diff. > }; >=20 > /* > diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c > index dff1e1697..4c6e90c64 100644 > --- a/src/box/sql/vdbeapi.c > +++ b/src/box/sql/vdbeapi.c > @@ -469,10 +469,6 @@ sqlite3_result_error_code(sqlite3_context * pCtx, = int errCode) > { > pCtx->isError =3D errCode; > pCtx->fErrorOrAux =3D 1; > -#ifdef SQLITE_DEBUG > - if (pCtx->pVdbe) > - pCtx->pVdbe->rcApp =3D errCode; > -#endif > if (pCtx->pOut->flags & MEM_Null) { > sqlite3VdbeMemSetStr(pCtx->pOut, sqlite3ErrStr(errCode), = -1, 1, > SQLITE_STATIC); > @@ -580,9 +576,6 @@ sqlite3Step(Vdbe * p) > db->nVdbeActive++; > p->pc =3D 0; > } > -#ifdef SQLITE_DEBUG > - p->rcApp =3D SQLITE_OK; > -#endif > #ifndef SQLITE_OMIT_EXPLAIN > if (p->explain) { > rc =3D sqlite3VdbeList(p); > @@ -614,8 +607,6 @@ sqlite3Step(Vdbe * p) > */ > assert(rc =3D=3D SQLITE_ROW || rc =3D=3D SQLITE_DONE || rc =3D=3D = SQLITE_ERROR > || (rc & 0xff) =3D=3D SQLITE_BUSY || rc =3D=3D = SQLITE_MISUSE); > - assert((p->rc !=3D SQLITE_ROW && p->rc !=3D SQLITE_DONE) > - || p->rc =3D=3D p->rcApp); > if (p->isPrepareV2 && rc !=3D SQLITE_ROW && rc !=3D SQLITE_DONE) = { > /* If this statement was prepared using = sqlite3_prepare_v2(), and an > * error has occurred, then return the error code in = p->rc to the > diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c > index e4335524a..bb121a318 100644 > --- a/src/box/sql/vdbeaux.c > +++ b/src/box/sql/vdbeaux.c > @@ -2148,7 +2148,6 @@ sqlite3VdbeMakeReady(Vdbe * p, /* The VDBE */ > assert(EIGHT_BYTE_ALIGNMENT(&x.pSpace[x.nFree])); >=20 > resolveP2Values(p, &nArg); > - p->usesStmtJournal =3D (u8) (pParse->isMultiWrite && = pParse->mayAbort); > if (pParse->explain && nMem < 10) { > nMem =3D 10; > } > --=20 > 2.14.3 (Apple Git-98) >=20 >=20