Allocate and track statement objects in pysqlite_statement_create

By allocating and tracking creation of statement object in pysqlite_statement_create(), the caller does not need to worry about GC syncronization, and eliminates the possibility of getting a badly created object. All related fault handling is moved to pysqlite_statement_create().
python · vstinner · Jun 1, 2021 · May 31, 2021 · Jun 1, 2021 · Jun 1, 2021
commit 1b0f5839b51c4e92c2e83aeddd61c130167f255d
@@ -1337,7 +1337,6 @@ pysqlite_connection_call(pysqlite_Connection *self, PyObject *args,
     PyObject* sql;
     pysqlite_Statement* statement;
     PyObject* weakref;
-    int rc;
 
     if (!pysqlite_check_thread(self) || !pysqlite_check_connection(self)) {
         return NULL;
@@ -1351,32 +1350,11 @@ pysqlite_connection_call(pysqlite_Connection *self, PyObject *args,
 
     _pysqlite_drop_unused_statement_references(self);
 
-    statement = PyObject_GC_New(pysqlite_Statement, pysqlite_StatementType);
-    if (!statement) {
+    statement = pysqlite_statement_create(self, sql);
+    if (statement == NULL) {
         return NULL;
     }
 
-    statement->db = NULL;
-    statement->st = NULL;
-    statement->sql = NULL;
-    statement->in_use = 0;
-    statement->in_weakreflist = NULL;
-
-    rc = pysqlite_statement_create(statement, self, sql);
-    PyObject_GC_Track(statement);
-    if (rc != SQLITE_OK) {
-        if (rc == PYSQLITE_TOO_MUCH_SQL) {
-            PyErr_SetString(pysqlite_Warning, "You can only execute one statement at a time.");
-        } else if (rc == PYSQLITE_SQL_WRONG_TYPE) {
-            if (PyErr_ExceptionMatches(PyExc_TypeError))
-                PyErr_SetString(pysqlite_Warning, "SQL is of wrong type. Must be string.");
-        } else {
-            (void)pysqlite_statement_reset(statement);
-            _pysqlite_seterror(self->db);
-        }
-        goto error;
-    }
-
     weakref = PyWeakref_NewRef((PyObject*)statement, NULL);
     if (weakref == NULL)
         goto error;

@@ -507,14 +507,8 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation
 
     if (self->statement->in_use) {
         Py_SETREF(self->statement,
-                  PyObject_GC_New(pysqlite_Statement, pysqlite_StatementType));
-        if (!self->statement) {
-            goto error;
-        }
-        rc = pysqlite_statement_create(self->statement, self->connection, operation);
-        PyObject_GC_Track(self->statement);
-        if (rc != SQLITE_OK) {
-            Py_CLEAR(self->statement);
+                  pysqlite_statement_create(self->connection, operation));
+        if (self->statement == NULL) {
             goto error;
         }
     }

@@ -48,29 +48,37 @@ typedef enum {
     TYPE_UNKNOWN
 } parameter_type;
 
-int pysqlite_statement_create(pysqlite_Statement* self, pysqlite_Connection* connection, PyObject* sql)
+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;
 
-    self->st = NULL;
-    self->in_use = 0;
-
     assert(PyUnicode_Check(sql));
 
     sql_cstr = PyUnicode_AsUTF8AndSize(sql, &sql_cstr_len);
     if (sql_cstr == NULL) {
-        rc = PYSQLITE_SQL_WRONG_TYPE;
-        return rc;
+        PyErr_SetString(pysqlite_Warning,
+                        "SQL is of wrong type. Must be string.");
+        return NULL;
     }
     if (strlen(sql_cstr) != (size_t)sql_cstr_len) {
-        PyErr_SetString(PyExc_ValueError, "the query contains a null character");
-        return PYSQLITE_SQL_WRONG_TYPE;
+        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) {
+        return NULL;
+    }
+
+    self->st = NULL;
+    self->in_use = 0;
     self->in_weakreflist = NULL;
     self->sql = Py_NewRef(sql);
 
@@ -102,14 +110,26 @@ int pysqlite_statement_create(pysqlite_Statement* self, pysqlite_Connection* con
     Py_END_ALLOW_THREADS
 
     self->db = connection->db;
+    PyObject_GC_Track(self);
+
+    if (rc != SQLITE_OK) {
+        _pysqlite_seterror(self->db);
+        goto error;
+    }
 
     if (rc == SQLITE_OK && pysqlite_check_remaining_sql(tail)) {
         (void)sqlite3_finalize(self->st);
         self->st = NULL;
-        rc = PYSQLITE_TOO_MUCH_SQL;
+        PyErr_SetString(pysqlite_Warning,
+                        "You can only execute one statement at a time.");
+        goto error;
     }
 
-    return rc;
+    return self;
+
+error:
+    Py_DECREF(self);
+    return NULL;
 }
 
 int pysqlite_statement_bind_parameter(pysqlite_Statement* self, int pos, PyObject* parameter)

@@ -45,7 +45,7 @@ typedef struct
 
 extern PyTypeObject *pysqlite_StatementType;
 
-int pysqlite_statement_create(pysqlite_Statement* self, pysqlite_Connection* connection, PyObject* sql);
+pysqlite_Statement *pysqlite_statement_create(pysqlite_Connection *connection, PyObject *sql);
 
 int pysqlite_statement_bind_parameter(pysqlite_Statement* self, int pos, PyObject* parameter);
 void pysqlite_statement_bind_parameters(pysqlite_Statement* self, PyObject* parameters);