From 00287aa6625012068203539e7dc64c9319339469 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 7 Jun 2021 00:00:38 +0200 Subject: [PATCH 1/3] Refactor sqlite3 statement create Call SQLite API's first, and return early in case of error. At the end, allocate the object and initialise members. We now avoid unneeded alloc/dealloc's in case the statement creation fails. --- Modules/_sqlite/statement.c | 83 ++++++++++++++++--------------------- 1 file changed, 36 insertions(+), 47 deletions(-) diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index 5c38b4607b428f..17cdb36a139bf9 100644 --- a/Modules/_sqlite/statement.c +++ b/Modules/_sqlite/statement.c @@ -51,15 +51,9 @@ typedef enum { pysqlite_Statement * pysqlite_statement_create(pysqlite_Connection *connection, PyObject *sql) { - const char* tail; - int rc; - const char* sql_cstr; - Py_ssize_t sql_cstr_len; - const char* p; - assert(PyUnicode_Check(sql)); - - sql_cstr = PyUnicode_AsUTF8AndSize(sql, &sql_cstr_len); + Py_ssize_t size; + const char *sql_cstr = PyUnicode_AsUTF8AndSize(sql, &size); if (sql_cstr == NULL) { PyErr_Format(pysqlite_Warning, "SQL is of wrong type ('%s'). Must be string.", @@ -67,31 +61,41 @@ pysqlite_statement_create(pysqlite_Connection *connection, PyObject *sql) return NULL; } - int max_length = sqlite3_limit(connection->db, SQLITE_LIMIT_LENGTH, -1); - if (sql_cstr_len >= max_length) { + sqlite3 *db = connection->db; + int max_length = sqlite3_limit(db, SQLITE_LIMIT_LENGTH, -1); + if (size >= max_length) { PyErr_SetString(pysqlite_DataError, "query string is too large"); return NULL; } - if (strlen(sql_cstr) != (size_t)sql_cstr_len) { + if (strlen(sql_cstr) != (size_t)size) { PyErr_SetString(PyExc_ValueError, "the query contains a null character"); return NULL; } - pysqlite_Statement *self = PyObject_GC_New(pysqlite_Statement, - pysqlite_StatementType); - if (self == NULL) { + int rc; + const char *tail; + sqlite3_stmt *stmt; + Py_BEGIN_ALLOW_THREADS + rc = sqlite3_prepare_v2(db, sql_cstr, (int)size + 1, &stmt, &tail); + Py_END_ALLOW_THREADS + + if (rc != SQLITE_OK) { + _pysqlite_seterror(db); return NULL; } - self->st = NULL; - self->in_use = 0; - self->is_dml = 0; - self->in_weakreflist = NULL; + if (pysqlite_check_remaining_sql(tail)) { + (void)sqlite3_finalize(stmt); + PyErr_SetString(pysqlite_Warning, + "You can only execute one statement at a time."); + return NULL; + } /* Determine if the statement is a DML statement. SELECT is the only exception. See #9924. */ - for (p = sql_cstr; *p != 0; p++) { + int is_dml = 0; + for (const char *p = sql_cstr; *p != 0; p++) { switch (*p) { case ' ': case '\r': @@ -100,41 +104,26 @@ pysqlite_statement_create(pysqlite_Connection *connection, PyObject *sql) continue; } - self->is_dml = (PyOS_strnicmp(p, "insert", 6) == 0) - || (PyOS_strnicmp(p, "update", 6) == 0) - || (PyOS_strnicmp(p, "delete", 6) == 0) - || (PyOS_strnicmp(p, "replace", 7) == 0); + is_dml = (PyOS_strnicmp(p, "insert", 6) == 0) + || (PyOS_strnicmp(p, "update", 6) == 0) + || (PyOS_strnicmp(p, "delete", 6) == 0) + || (PyOS_strnicmp(p, "replace", 7) == 0); break; } - Py_BEGIN_ALLOW_THREADS - rc = sqlite3_prepare_v2(connection->db, - sql_cstr, - (int)sql_cstr_len + 1, - &self->st, - &tail); - Py_END_ALLOW_THREADS - - PyObject_GC_Track(self); - - if (rc != SQLITE_OK) { - _pysqlite_seterror(connection->db); - goto error; + pysqlite_Statement *self = PyObject_GC_New(pysqlite_Statement, + pysqlite_StatementType); + if (self == NULL) { + return NULL; } - if (rc == SQLITE_OK && pysqlite_check_remaining_sql(tail)) { - (void)sqlite3_finalize(self->st); - self->st = NULL; - PyErr_SetString(pysqlite_Warning, - "You can only execute one statement at a time."); - goto error; - } + self->st = stmt; + self->in_use = 0; + self->is_dml = is_dml; + self->in_weakreflist = NULL; + PyObject_GC_Track(self); return self; - -error: - Py_DECREF(self); - return NULL; } int pysqlite_statement_bind_parameter(pysqlite_Statement* self, int pos, PyObject* parameter) From 171e99d01c36ee901368697b6c14736ed43dded7 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 7 Jun 2021 00:33:26 +0200 Subject: [PATCH 2/3] Always finalize on error --- Modules/_sqlite/statement.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index 17cdb36a139bf9..57fa2f36e8e1de 100644 --- a/Modules/_sqlite/statement.c +++ b/Modules/_sqlite/statement.c @@ -86,10 +86,9 @@ pysqlite_statement_create(pysqlite_Connection *connection, PyObject *sql) } if (pysqlite_check_remaining_sql(tail)) { - (void)sqlite3_finalize(stmt); PyErr_SetString(pysqlite_Warning, "You can only execute one statement at a time."); - return NULL; + goto error; } /* Determine if the statement is a DML statement. @@ -114,7 +113,7 @@ pysqlite_statement_create(pysqlite_Connection *connection, PyObject *sql) pysqlite_Statement *self = PyObject_GC_New(pysqlite_Statement, pysqlite_StatementType); if (self == NULL) { - return NULL; + goto error; } self->st = stmt; @@ -124,6 +123,10 @@ pysqlite_statement_create(pysqlite_Connection *connection, PyObject *sql) PyObject_GC_Track(self); return self; + +error: + (void)sqlite3_finalize(stmt); + return NULL; } int pysqlite_statement_bind_parameter(pysqlite_Statement* self, int pos, PyObject* parameter) From 78668b6093e624b69e5e3c18773f52088d50f857 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 7 Jun 2021 23:43:54 +0200 Subject: [PATCH 3/3] Reorder variables used with sqlite3_prepare_v2 --- Modules/_sqlite/statement.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c index 57fa2f36e8e1de..eca225820cd75e 100644 --- a/Modules/_sqlite/statement.c +++ b/Modules/_sqlite/statement.c @@ -73,9 +73,9 @@ pysqlite_statement_create(pysqlite_Connection *connection, PyObject *sql) return NULL; } - int rc; - const char *tail; sqlite3_stmt *stmt; + const char *tail; + int rc; Py_BEGIN_ALLOW_THREADS rc = sqlite3_prepare_v2(db, sql_cstr, (int)size + 1, &stmt, &tail); Py_END_ALLOW_THREADS