From beffb02d564df8684137a5678d2a0a946e2a42d7 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sun, 20 Feb 2011 23:43:25 +0000 Subject: [PATCH] Stricter declaration and correct use of psyco_set_error It has long been used in wrong ways, with the function receiving a connection or lobject instead of a cursor. It has always been unnoticed (nobody has noticed the wrong object attached to the exception in the wrong cases) but it started crashing the interpreter with Python 3.2 on Windows. Thanks to Jason Erickson for finding the problem and helping fixing it. --- psycopg/cursor.h | 6 ++++-- psycopg/cursor_type.c | 34 +++++++++++++++++----------------- psycopg/lobject.h | 4 ++-- psycopg/lobject_type.c | 2 +- psycopg/pqpath.c | 3 +-- psycopg/psycopg.h | 5 ++++- psycopg/psycopgmodule.c | 4 ++-- 7 files changed, 31 insertions(+), 27 deletions(-) diff --git a/psycopg/cursor.h b/psycopg/cursor.h index 395c561a..09ac12a8 100644 --- a/psycopg/cursor.h +++ b/psycopg/cursor.h @@ -34,7 +34,8 @@ extern "C" { extern HIDDEN PyTypeObject cursorType; -typedef struct { +/* the typedef is forward-declared in psycopg.h */ +struct cursorObject { PyObject_HEAD connectionObject *conn; /* connection owning the cursor */ @@ -79,7 +80,8 @@ typedef struct { PyObject *weakreflist; /* list of weak references */ -} cursorObject; +}; + /* C-callable functions in cursor_int.c and cursor_ext.c */ HIDDEN PyObject *curs_get_cast(cursorObject *self, PyObject *oid); diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index be4e56c4..5bb9095a 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -76,7 +76,7 @@ psyco_curs_close(cursorObject *self, PyObject *args) /* mogrify a query string and build argument array or dict */ static int -_mogrify(PyObject *var, PyObject *fmt, connectionObject *conn, PyObject **new) +_mogrify(PyObject *var, PyObject *fmt, cursorObject *curs, PyObject **new) { PyObject *key, *value, *n; const char *d, *c; @@ -113,7 +113,7 @@ _mogrify(PyObject *var, PyObject *fmt, connectionObject *conn, PyObject **new) /* check if some crazy guy mixed formats */ if (kind == 2) { Py_XDECREF(n); - psyco_set_error(ProgrammingError, (PyObject*)conn, + psyco_set_error(ProgrammingError, curs, "argument formats can't be mixed", NULL, NULL); return -1; } @@ -155,7 +155,7 @@ _mogrify(PyObject *var, PyObject *fmt, connectionObject *conn, PyObject **new) /* t is a new object, refcnt = 1, key is at 2 */ } else { - t = microprotocol_getquoted(value, conn); + t = microprotocol_getquoted(value, curs->conn); if (t != NULL) { PyDict_SetItem(n, key, t); @@ -181,7 +181,7 @@ _mogrify(PyObject *var, PyObject *fmt, connectionObject *conn, PyObject **new) else { /* we found %( but not a ) */ Py_XDECREF(n); - psyco_set_error(ProgrammingError, (PyObject*)conn, + psyco_set_error(ProgrammingError, curs, "incomplete placeholder: '%(' without ')'", NULL, NULL); return -1; } @@ -196,7 +196,7 @@ _mogrify(PyObject *var, PyObject *fmt, connectionObject *conn, PyObject **new) /* check if some crazy guy mixed formats */ if (kind == 1) { Py_XDECREF(n); - psyco_set_error(ProgrammingError, (PyObject*)conn, + psyco_set_error(ProgrammingError, curs, "argument formats can't be mixed", NULL, NULL); return -1; } @@ -223,7 +223,7 @@ _mogrify(PyObject *var, PyObject *fmt, connectionObject *conn, PyObject **new) Py_DECREF(value); } else { - PyObject *t = microprotocol_getquoted(value, conn); + PyObject *t = microprotocol_getquoted(value, curs->conn); if (t != NULL) { PyTuple_SET_ITEM(n, index, t); @@ -255,7 +255,7 @@ static PyObject *_psyco_curs_validate_sql_basic( after having set an exception. */ if (!sql || !PyObject_IsTrue(sql)) { - psyco_set_error(ProgrammingError, (PyObject*)self, + psyco_set_error(ProgrammingError, self, "can't execute an empty query", NULL, NULL); goto fail; } @@ -327,7 +327,7 @@ _psyco_curs_merge_query_args(cursorObject *self, if (!strcmp(s, "not enough arguments for format string") || !strcmp(s, "not all arguments converted")) { Dprintf("psyco_curs_execute: -> got a match"); - psyco_set_error(ProgrammingError, (PyObject*)self, + psyco_set_error(ProgrammingError, self, s, NULL, NULL); pe = 1; } @@ -381,7 +381,7 @@ _psyco_curs_execute(cursorObject *self, if (vars && vars != Py_None) { - if(_mogrify(vars, operation, self->conn, &cvt) == -1) { goto fail; } + if(_mogrify(vars, operation, self, &cvt) == -1) { goto fail; } } if (vars && cvt) { @@ -451,18 +451,18 @@ psyco_curs_execute(cursorObject *self, PyObject *args, PyObject *kwargs) if (self->name != NULL) { if (self->query != Py_None) { - psyco_set_error(ProgrammingError, (PyObject*)self, + psyco_set_error(ProgrammingError, self, "can't call .execute() on named cursors more than once", NULL, NULL); return NULL; } if (self->conn->isolation_level == ISOLATION_LEVEL_AUTOCOMMIT) { - psyco_set_error(ProgrammingError, (PyObject*)self, + psyco_set_error(ProgrammingError, self, "can't use a named cursor outside of transactions", NULL, NULL); return NULL; } if (self->conn->mark != self->mark) { - psyco_set_error(ProgrammingError, (PyObject*)self, + psyco_set_error(ProgrammingError, self, "named cursor isn't valid anymore", NULL, NULL); return NULL; } @@ -506,7 +506,7 @@ psyco_curs_executemany(cursorObject *self, PyObject *args, PyObject *kwargs) EXC_IF_TPC_PREPARED(self->conn, executemany); if (self->name != NULL) { - psyco_set_error(ProgrammingError, (PyObject*)self, + psyco_set_error(ProgrammingError, self, "can't call .executemany() on named cursors", NULL, NULL); return NULL; } @@ -564,7 +564,7 @@ _psyco_curs_mogrify(cursorObject *self, if (vars && vars != Py_None) { - if (_mogrify(vars, operation, self->conn, &cvt) == -1) { + if (_mogrify(vars, operation, self, &cvt) == -1) { goto cleanup; } } @@ -998,7 +998,7 @@ psyco_curs_callproc(cursorObject *self, PyObject *args, PyObject *kwargs) EXC_IF_TPC_PREPARED(self->conn, callproc); if (self->name != NULL) { - psyco_set_error(ProgrammingError, (PyObject*)self, + psyco_set_error(ProgrammingError, self, "can't call .callproc() on named cursors", NULL, NULL); return NULL; } @@ -1121,13 +1121,13 @@ psyco_curs_scroll(cursorObject *self, PyObject *args, PyObject *kwargs) } else if (strcmp( mode, "absolute") == 0) { newpos = value; } else { - psyco_set_error(ProgrammingError, (PyObject*)self, + psyco_set_error(ProgrammingError, self, "scroll mode must be 'relative' or 'absolute'", NULL, NULL); return NULL; } if (newpos < 0 || newpos >= self->rowcount ) { - psyco_set_error(ProgrammingError, (PyObject*)self, + psyco_set_error(ProgrammingError, self, "scroll destination out of bounds", NULL, NULL); return NULL; } diff --git a/psycopg/lobject.h b/psycopg/lobject.h index 293f608f..84fd3b52 100644 --- a/psycopg/lobject.h +++ b/psycopg/lobject.h @@ -77,13 +77,13 @@ HIDDEN int lobject_close(lobjectObject *self); #define EXC_IF_LOBJ_LEVEL0(self) \ if (self->conn->isolation_level == 0) { \ - psyco_set_error(ProgrammingError, (PyObject*)self, \ + psyco_set_error(ProgrammingError, NULL, \ "can't use a lobject outside of transactions", NULL, NULL); \ return NULL; \ } #define EXC_IF_LOBJ_UNMARKED(self) \ if (self->conn->mark != self->mark) { \ - psyco_set_error(ProgrammingError, (PyObject*)self, \ + psyco_set_error(ProgrammingError, NULL, \ "lobject isn't valid anymore", NULL, NULL); \ return NULL; \ } diff --git a/psycopg/lobject_type.c b/psycopg/lobject_type.c index 49f64bab..39ff6ead 100644 --- a/psycopg/lobject_type.c +++ b/psycopg/lobject_type.c @@ -331,7 +331,7 @@ lobject_setup(lobjectObject *self, connectionObject *conn, Dprintf("lobject_setup: init lobject object at %p", self); if (conn->isolation_level == ISOLATION_LEVEL_AUTOCOMMIT) { - psyco_set_error(ProgrammingError, (PyObject*)self, + psyco_set_error(ProgrammingError, NULL, "can't use a lobject outside of transactions", NULL, NULL); return -1; } diff --git a/psycopg/pqpath.c b/psycopg/pqpath.c index d9f0aa7b..5bfa4436 100644 --- a/psycopg/pqpath.c +++ b/psycopg/pqpath.c @@ -151,7 +151,6 @@ exception_from_sqlstate(const char *sqlstate) static void pq_raise(connectionObject *conn, cursorObject *curs, PGresult *pgres) { - PyObject *pgc = (PyObject*)curs; PyObject *exc = NULL; const char *err = NULL; const char *err2 = NULL; @@ -196,7 +195,7 @@ pq_raise(connectionObject *conn, cursorObject *curs, PGresult *pgres) /* try to remove the initial "ERROR: " part from the postgresql error */ err2 = strip_severity(err); - psyco_set_error(exc, pgc, err2, err, code); + psyco_set_error(exc, curs, err2, err, code); } /* pq_set_critical, pq_resolve_critical - manage critical errors diff --git a/psycopg/psycopg.h b/psycopg/psycopg.h index 710a51bc..d40a2399 100644 --- a/psycopg/psycopg.h +++ b/psycopg/psycopg.h @@ -116,8 +116,11 @@ typedef struct { /* the Decimal type, used by the DECIMAL typecaster */ HIDDEN PyObject *psyco_GetDecimalType(void); +/* forward declaration */ +typedef struct cursorObject cursorObject; + /* some utility functions */ -HIDDEN void psyco_set_error(PyObject *exc, PyObject *curs, const char *msg, +HIDDEN void psyco_set_error(PyObject *exc, cursorObject *curs, const char *msg, const char *pgerror, const char *pgcode); HIDDEN char *psycopg_escape_string(PyObject *conn, diff --git a/psycopg/psycopgmodule.c b/psycopg/psycopgmodule.c index 2a98ed79..01069414 100644 --- a/psycopg/psycopgmodule.c +++ b/psycopg/psycopgmodule.c @@ -589,7 +589,7 @@ psyco_errors_set(PyObject *type) Create a new error of the given type with extra attributes. */ void -psyco_set_error(PyObject *exc, PyObject *curs, const char *msg, +psyco_set_error(PyObject *exc, cursorObject *curs, const char *msg, const char *pgerror, const char *pgcode) { PyObject *t; @@ -613,7 +613,7 @@ psyco_set_error(PyObject *exc, PyObject *curs, const char *msg, if (err) { if (curs) { - PyObject_SetAttrString(err, "cursor", curs); + PyObject_SetAttrString(err, "cursor", (PyObject *)curs); } if (pgerror) {