From 2e54c809387fc0ced6d26602f42a14b0797552dc Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Fri, 11 Jan 2019 12:41:53 +0100 Subject: [PATCH 1/4] [sqlite] Use extended result codes Extended result codes can give more information about what kind of error occurred during an SQLite operation, which is desirable for improved logging and debugging. --- xbmc/dbwrappers/sqlitedataset.cpp | 195 ++++++++++++++++++++++-------- 1 file changed, 145 insertions(+), 50 deletions(-) diff --git a/xbmc/dbwrappers/sqlitedataset.cpp b/xbmc/dbwrappers/sqlitedataset.cpp index f0b11d4b79151..4643370de8a45 100644 --- a/xbmc/dbwrappers/sqlitedataset.cpp +++ b/xbmc/dbwrappers/sqlitedataset.cpp @@ -11,7 +11,9 @@ */ #include +#include #include +#include #include "sqlitedataset.h" #include "utils/log.h" @@ -21,6 +23,137 @@ #include "platform/linux/XTimeUtils.h" #endif +namespace { +#define X(VAL) std::make_pair(VAL, #VAL) +//!@todo Remove ifdefs when sqlite version requirement has been bumped to at least 3.26.0 +const std::map g_SqliteErrorStrings = +{ + X(SQLITE_OK), + X(SQLITE_ERROR), + X(SQLITE_INTERNAL), + X(SQLITE_PERM), + X(SQLITE_ABORT), + X(SQLITE_BUSY), + X(SQLITE_LOCKED), + X(SQLITE_NOMEM), + X(SQLITE_READONLY), + X(SQLITE_INTERRUPT), + X(SQLITE_IOERR), + X(SQLITE_CORRUPT), + X(SQLITE_NOTFOUND), + X(SQLITE_FULL), + X(SQLITE_CANTOPEN), + X(SQLITE_PROTOCOL), + X(SQLITE_EMPTY), + X(SQLITE_SCHEMA), + X(SQLITE_TOOBIG), + X(SQLITE_CONSTRAINT), + X(SQLITE_MISMATCH), + X(SQLITE_MISUSE), + X(SQLITE_NOLFS), + X(SQLITE_AUTH), + X(SQLITE_FORMAT), + X(SQLITE_RANGE), + X(SQLITE_NOTADB), + X(SQLITE_NOTICE), + X(SQLITE_WARNING), + X(SQLITE_ROW), + X(SQLITE_DONE), +#if defined(SQLITE_ERROR_MISSING_COLLSEQ) + X(SQLITE_ERROR_MISSING_COLLSEQ), +#endif +#if defined(SQLITE_ERROR_RETRY) + X(SQLITE_ERROR_RETRY), +#endif +#if defined(SQLITE_ERROR_SNAPSHOT) + X(SQLITE_ERROR_SNAPSHOT), +#endif + X(SQLITE_IOERR_READ), + X(SQLITE_IOERR_SHORT_READ), + X(SQLITE_IOERR_WRITE), + X(SQLITE_IOERR_FSYNC), + X(SQLITE_IOERR_DIR_FSYNC), + X(SQLITE_IOERR_TRUNCATE), + X(SQLITE_IOERR_FSTAT), + X(SQLITE_IOERR_UNLOCK), + X(SQLITE_IOERR_RDLOCK), + X(SQLITE_IOERR_DELETE), + X(SQLITE_IOERR_BLOCKED), + X(SQLITE_IOERR_NOMEM), + X(SQLITE_IOERR_ACCESS), + X(SQLITE_IOERR_CHECKRESERVEDLOCK), + X(SQLITE_IOERR_LOCK), + X(SQLITE_IOERR_CLOSE), + X(SQLITE_IOERR_DIR_CLOSE), + X(SQLITE_IOERR_SHMOPEN), + X(SQLITE_IOERR_SHMSIZE), + X(SQLITE_IOERR_SHMLOCK), + X(SQLITE_IOERR_SHMMAP), + X(SQLITE_IOERR_SEEK), + X(SQLITE_IOERR_DELETE_NOENT), + X(SQLITE_IOERR_MMAP), + X(SQLITE_IOERR_GETTEMPPATH), + X(SQLITE_IOERR_CONVPATH), + X(SQLITE_IOERR_VNODE), + X(SQLITE_IOERR_AUTH), +#if defined(SQLITE_IOERR_BEGIN_ATOMIC) + X(SQLITE_IOERR_BEGIN_ATOMIC), +#endif +#if defined(SQLITE_IOERR_COMMIT_ATOMIC) + X(SQLITE_IOERR_COMMIT_ATOMIC), +#endif +#if defined(SQLITE_IOERR_ROLLBACK_ATOMIC) + X(SQLITE_IOERR_ROLLBACK_ATOMIC), +#endif + X(SQLITE_LOCKED_SHAREDCACHE), +#if defined(SQLITE_LOCKED_VTAB) + X(SQLITE_LOCKED_VTAB), +#endif + X(SQLITE_BUSY_RECOVERY), + X(SQLITE_BUSY_SNAPSHOT), + X(SQLITE_CANTOPEN_NOTEMPDIR), + X(SQLITE_CANTOPEN_ISDIR), + X(SQLITE_CANTOPEN_FULLPATH), + X(SQLITE_CANTOPEN_CONVPATH), +#if defined(SQLITE_CANTOPEN_DIRTYWAL) + X(SQLITE_CANTOPEN_DIRTYWAL), +#endif + X(SQLITE_CORRUPT_VTAB), +#if defined(SQLITE_CORRUPT_SEQUENCE) + X(SQLITE_CORRUPT_SEQUENCE), +#endif + X(SQLITE_READONLY_RECOVERY), + X(SQLITE_READONLY_CANTLOCK), + X(SQLITE_READONLY_ROLLBACK), + X(SQLITE_READONLY_DBMOVED), +#if defined(SQLITE_READONLY_CANTINIT) + X(SQLITE_READONLY_CANTINIT), +#endif +#if defined(SQLITE_READONLY_DIRECTORY) + X(SQLITE_READONLY_DIRECTORY), +#endif + X(SQLITE_ABORT_ROLLBACK), + X(SQLITE_CONSTRAINT_CHECK), + X(SQLITE_CONSTRAINT_COMMITHOOK), + X(SQLITE_CONSTRAINT_FOREIGNKEY), + X(SQLITE_CONSTRAINT_FUNCTION), + X(SQLITE_CONSTRAINT_NOTNULL), + X(SQLITE_CONSTRAINT_PRIMARYKEY), + X(SQLITE_CONSTRAINT_TRIGGER), + X(SQLITE_CONSTRAINT_UNIQUE), + X(SQLITE_CONSTRAINT_VTAB), + X(SQLITE_CONSTRAINT_ROWID), + X(SQLITE_NOTICE_RECOVER_WAL), + X(SQLITE_NOTICE_RECOVER_ROLLBACK), + X(SQLITE_WARNING_AUTOINDEX), + X(SQLITE_AUTH_USER), +#if defined(SQLITE_OK_LOAD_PERMANENTLY) + X(SQLITE_OK_LOAD_PERMANENTLY), +#endif +}; +#undef X +} + namespace dbiplus { //************* Callback function *************************** @@ -129,56 +262,17 @@ int SqliteDatabase::status(void) { return DB_CONNECTION_OK; } -int SqliteDatabase::setErr(int err_code, const char * qry){ - switch (err_code) { - case SQLITE_OK: error ="Successful result"; - break; - case SQLITE_ERROR: error = "SQL error or missing database"; - break; - case SQLITE_INTERNAL: error = "An internal logic error in SQLite"; - break; - case SQLITE_PERM: error ="Access permission denied"; - break; - case SQLITE_ABORT: error = "Callback routine requested an abort"; - break; - case SQLITE_BUSY: error = "The database file is locked"; - break; - case SQLITE_LOCKED: error = "A table in the database is locked"; - break; - case SQLITE_NOMEM: error = "A malloc() failed"; - break; - case SQLITE_READONLY: error = "Attempt to write a readonly database"; - break; - case SQLITE_INTERRUPT: error = "Operation terminated by sqlite_interrupt()"; - break; - case SQLITE_IOERR: error = "Some kind of disk I/O error occurred"; - break; - case SQLITE_CORRUPT: error = "The database disk image is malformed"; - break; - case SQLITE_NOTFOUND: error = "(Internal Only) Table or record not found"; - break; - case SQLITE_FULL: error = "Insertion failed because database is full"; - break; - case SQLITE_CANTOPEN: error = "Unable to open the database file"; - break; - case SQLITE_PROTOCOL: error = "Database lock protocol error"; - break; - case SQLITE_EMPTY: error = "(Internal Only) Database table is empty"; - break; - case SQLITE_SCHEMA: error = "The database schema changed"; - break; - case SQLITE_TOOBIG: error = "Too much data for one row of a table"; - break; - case SQLITE_CONSTRAINT: error = "Abort due to constraint violation"; - break; - case SQLITE_MISMATCH: error = "Data type mismatch"; - break; - default : error = "Undefined SQLite error"; - } - error = "[" + db + "] " + error; - error += "\nQuery: "; - error += qry; - error += "\n"; +int SqliteDatabase::setErr(int err_code, const char * qry) { + std::stringstream ss; + ss << "[" << db << "] "; + auto errorIt = g_SqliteErrorStrings.find(err_code); + if (errorIt != g_SqliteErrorStrings.end()) { + ss << "SQLite error " << errorIt->second; + } else { + ss << "Undefined SQLite error " << err_code; + } + ss << "\nQuery: " << qry; + error = ss.str(); return err_code; } @@ -208,6 +302,7 @@ int SqliteDatabase::connect(bool create) { } else if (errorCode == SQLITE_OK) { + sqlite3_extended_result_codes(conn, 1); sqlite3_busy_handler(conn, busy_callback, NULL); char* err=NULL; if (setErr(sqlite3_exec(getHandle(),"PRAGMA empty_result_callbacks=ON",NULL,NULL,&err),"PRAGMA empty_result_callbacks=ON") != SQLITE_OK) From 7bb430522d7a40f38d76a698c121d3f2fa03a125 Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Fri, 11 Jan 2019 12:42:06 +0100 Subject: [PATCH 2/4] [sqlite] Include sqlite3_errmsg() result in error messages Improves debugging by including information about the actual SQL error in log messages (mostly relevant for sqlite3_prepare_v2 and sqlite3_finalize, but doesn't hurt to just print it always) --- xbmc/dbwrappers/sqlitedataset.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/xbmc/dbwrappers/sqlitedataset.cpp b/xbmc/dbwrappers/sqlitedataset.cpp index 4643370de8a45..5ad10c8813823 100644 --- a/xbmc/dbwrappers/sqlitedataset.cpp +++ b/xbmc/dbwrappers/sqlitedataset.cpp @@ -271,6 +271,8 @@ int SqliteDatabase::setErr(int err_code, const char * qry) { } else { ss << "Undefined SQLite error " << err_code; } + if (conn) + ss << " (" << sqlite3_errmsg(conn) << ")"; ss << "\nQuery: " << qry; error = ss.str(); return err_code; From fb53badaa5e7dc5560f37dd4c1ef0f7627d3c3a9 Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Fri, 11 Jan 2019 12:43:29 +0100 Subject: [PATCH 3/4] [sqlite] Use error message returned by sqlite3_exec() sqlite3_exec optionally gives us an error message when execution of an SQL statement fails, but we do not use it currently. To improve debugging, include the message in DbErrors exception message. Also fixes a memory leak in the error path of make_query (missing sqlite3_free) and a format string mistake (DbErrors constructor uses printf formatting) --- xbmc/dbwrappers/sqlitedataset.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/xbmc/dbwrappers/sqlitedataset.cpp b/xbmc/dbwrappers/sqlitedataset.cpp index 5ad10c8813823..eaa314dfc0988 100644 --- a/xbmc/dbwrappers/sqlitedataset.cpp +++ b/xbmc/dbwrappers/sqlitedataset.cpp @@ -620,7 +620,14 @@ void SqliteDataset::make_query(StringList &_sql) { char* err=NULL; Dataset::parse_sql(query); if (db->setErr(sqlite3_exec(this->handle(),query.c_str(),NULL,NULL,&err),query.c_str())!=SQLITE_OK) { - throw DbErrors(db->getErrorMsg()); + std::string message = db->getErrorMsg(); + if (err) { + message.append(" ("); + message.append(err); + message.append(")"); + sqlite3_free(err); + } + throw DbErrors("%s", message.c_str()); } } // end of for @@ -748,7 +755,10 @@ int SqliteDataset::exec(const std::string &sql) { return res; else { - throw DbErrors(db->getErrorMsg()); + if (errmsg) + throw DbErrors("%s (%s)", db->getErrorMsg(), errmsg); + else + throw DbErrors("%s", db->getErrorMsg()); } } From 0b8042940b9a6754e0040494b05217f5ec1a412f Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Fri, 11 Jan 2019 12:44:00 +0100 Subject: [PATCH 4/4] [sqlite] Fix DbErrors format strings Fixes format string mistake (DbErrors constructor uses printf formatting) --- xbmc/dbwrappers/sqlitedataset.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/xbmc/dbwrappers/sqlitedataset.cpp b/xbmc/dbwrappers/sqlitedataset.cpp index eaa314dfc0988..45592f320ab61 100644 --- a/xbmc/dbwrappers/sqlitedataset.cpp +++ b/xbmc/dbwrappers/sqlitedataset.cpp @@ -309,7 +309,7 @@ int SqliteDatabase::connect(bool create) { char* err=NULL; if (setErr(sqlite3_exec(getHandle(),"PRAGMA empty_result_callbacks=ON",NULL,NULL,&err),"PRAGMA empty_result_callbacks=ON") != SQLITE_OK) { - throw DbErrors(getErrorMsg()); + throw DbErrors("%s", getErrorMsg()); } else if (sqlite3_db_readonly(conn, nullptr) == 1) { @@ -783,7 +783,7 @@ bool SqliteDataset::query(const std::string &query) { sqlite3_stmt *stmt = NULL; if (db->setErr(sqlite3_prepare_v2(handle(),query.c_str(),-1,&stmt, NULL),query.c_str()) != SQLITE_OK) - throw DbErrors(db->getErrorMsg()); + throw DbErrors("%s", db->getErrorMsg()); // column headers const unsigned int numColumns = sqlite3_column_count(stmt); @@ -831,7 +831,7 @@ bool SqliteDataset::query(const std::string &query) { } else { - throw DbErrors(db->getErrorMsg()); + throw DbErrors("%s", db->getErrorMsg()); } }