From 026899e0c1479412b62dd42d6cc7c0e4fb051370 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 23 Feb 2012 18:28:30 +0000 Subject: [PATCH 01/36] Check errors when populating encodings map --- psycopg/psycopgmodule.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/psycopg/psycopgmodule.c b/psycopg/psycopgmodule.c index 438b018e..4177401f 100644 --- a/psycopg/psycopgmodule.c +++ b/psycopg/psycopgmodule.c @@ -326,15 +326,27 @@ static encodingPair encodings[] = { {NULL, NULL} }; -static void psyco_encodings_fill(PyObject *dict) +/* Initialize the encodings table. + * + * Return 0 on success, else -1 and set an exception. + */ +static int psyco_encodings_fill(PyObject *dict) { + PyObject *value = NULL; encodingPair *enc; + int rv = -1; for (enc = encodings; enc->pgenc != NULL; enc++) { - PyObject *value = Text_FromUTF8(enc->pyenc); - PyDict_SetItemString(dict, enc->pgenc, value); - Py_DECREF(value); + if (!(value = Text_FromUTF8(enc->pyenc))) { goto exit; } + if (0 != PyDict_SetItemString(dict, enc->pgenc, value)) { goto exit; } + Py_CLEAR(value); } + rv = 0; + +exit: + Py_XDECREF(value); + + return rv; } /* psyco_errors_init, psyco_errors_fill (callable from C) @@ -905,8 +917,8 @@ INIT_MODULE(_psycopg)(void) #endif /* other mixed initializations of module-level variables */ - psycoEncodings = PyDict_New(); - psyco_encodings_fill(psycoEncodings); + if (!(psycoEncodings = PyDict_New())) { goto exit; } + if (0 != psyco_encodings_fill(psycoEncodings)) { goto exit; } psyco_null = Bytes_FromString("NULL"); psyco_DescriptionType = psyco_make_description_type(); From ff61cf25b694f9c41d09d650ca28102cc2152ea0 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 23 Feb 2012 18:41:31 +0000 Subject: [PATCH 02/36] Fixed refcount of None if namedtuples are not available --- psycopg/psycopgmodule.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/psycopg/psycopgmodule.c b/psycopg/psycopgmodule.c index 4177401f..9f71d7e9 100644 --- a/psycopg/psycopgmodule.c +++ b/psycopg/psycopgmodule.c @@ -685,15 +685,11 @@ psyco_make_description_type(void) /* Try to import collections.namedtuple */ if (!(coll = PyImport_ImportModule("collections"))) { Dprintf("psyco_make_description_type: collections import failed"); - PyErr_Clear(); - rv = Py_None; - goto exit; + goto error; } if (!(nt = PyObject_GetAttrString(coll, "namedtuple"))) { Dprintf("psyco_make_description_type: no collections.namedtuple"); - PyErr_Clear(); - rv = Py_None; - goto exit; + goto error; } /* Build the namedtuple */ @@ -705,6 +701,13 @@ exit: Py_XDECREF(nt); return rv; + +error: + /* controlled error: we will fall back to regular tuples. Return None. */ + PyErr_Clear(); + rv = Py_None; + Py_INCREF(rv); + goto exit; } @@ -920,7 +923,7 @@ INIT_MODULE(_psycopg)(void) if (!(psycoEncodings = PyDict_New())) { goto exit; } if (0 != psyco_encodings_fill(psycoEncodings)) { goto exit; } psyco_null = Bytes_FromString("NULL"); - psyco_DescriptionType = psyco_make_description_type(); + if (!(psyco_DescriptionType = psyco_make_description_type())) { goto exit; } /* set some module's parameters */ PyModule_AddStringConstant(module, "__version__", PSYCOPG_VERSION); From 08fbd8649596680b7fa6b83857a390bdb9ca65a5 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 23 Feb 2012 19:20:51 +0000 Subject: [PATCH 03/36] Check errors in module typecasters init --- psycopg/microprotocols.c | 20 ++++++--- psycopg/psycopgmodule.c | 96 +++++++++++++++++++++++++++++----------- 2 files changed, 82 insertions(+), 34 deletions(-) diff --git a/psycopg/microprotocols.c b/psycopg/microprotocols.c index f48c0fbf..68b5f6c4 100644 --- a/psycopg/microprotocols.c +++ b/psycopg/microprotocols.c @@ -52,22 +52,28 @@ microprotocols_init(PyObject *dict) } -/* microprotocols_add - add a reverse type-caster to the dictionary */ - +/* microprotocols_add - add a reverse type-caster to the dictionary + * + * Return 0 on success, else -1 and set an exception. + */ int microprotocols_add(PyTypeObject *type, PyObject *proto, PyObject *cast) { - PyObject *key; + PyObject *key = NULL; + int rv = -1; if (proto == NULL) proto = (PyObject*)&isqlquoteType; Dprintf("microprotocols_add: cast %p for (%s, ?)", cast, type->tp_name); - key = PyTuple_Pack(2, (PyObject*)type, proto); - PyDict_SetItem(psyco_adapters, key, cast); - Py_DECREF(key); + if (!(key = PyTuple_Pack(2, (PyObject*)type, proto))) { goto exit; } + if (0 != PyDict_SetItem(psyco_adapters, key, cast)) { goto exit; } - return 0; + rv = 0; + +exit: + Py_XDECREF(key); + return rv; } /* Check if one of `obj` superclasses has an adapter for `proto`. diff --git a/psycopg/psycopgmodule.c b/psycopg/psycopgmodule.c index 9f71d7e9..577de15b 100644 --- a/psycopg/psycopgmodule.c +++ b/psycopg/psycopgmodule.c @@ -182,66 +182,108 @@ psyco_register_type(PyObject *self, PyObject *args) } -/* default adapters */ - -static void +/* Initialize the default adapters map + * + * Return 0 on success, else -1 and set an exception. + */ +static int psyco_adapters_init(PyObject *mod) { - PyObject *call; + PyObject *call = NULL; + int rv = -1; - microprotocols_add(&PyFloat_Type, NULL, (PyObject*)&pfloatType); + if (0 != microprotocols_add(&PyFloat_Type, NULL, (PyObject*)&pfloatType)) { + goto exit; + } #if PY_MAJOR_VERSION < 3 - microprotocols_add(&PyInt_Type, NULL, (PyObject*)&pintType); + if (0 != microprotocols_add(&PyInt_Type, NULL, (PyObject*)&pintType)) { + goto exit; + } #endif - microprotocols_add(&PyLong_Type, NULL, (PyObject*)&pintType); - microprotocols_add(&PyBool_Type, NULL, (PyObject*)&pbooleanType); + if (0 != microprotocols_add(&PyLong_Type, NULL, (PyObject*)&pintType)) { + goto exit; + } + if (0 != microprotocols_add(&PyBool_Type, NULL, (PyObject*)&pbooleanType)) { + goto exit; + } /* strings */ #if PY_MAJOR_VERSION < 3 - microprotocols_add(&PyString_Type, NULL, (PyObject*)&qstringType); + if (0 != microprotocols_add(&PyString_Type, NULL, (PyObject*)&qstringType)) { + goto exit; + } #endif - microprotocols_add(&PyUnicode_Type, NULL, (PyObject*)&qstringType); + if (0 != microprotocols_add(&PyUnicode_Type, NULL, (PyObject*)&qstringType)) { + goto exit; + } /* binary */ #if PY_MAJOR_VERSION < 3 - microprotocols_add(&PyBuffer_Type, NULL, (PyObject*)&binaryType); + if (0 != microprotocols_add(&PyBuffer_Type, NULL, (PyObject*)&binaryType)) { + goto exit; + } #else - microprotocols_add(&PyBytes_Type, NULL, (PyObject*)&binaryType); + if (0 != microprotocols_add(&PyBytes_Type, NULL, (PyObject*)&binaryType)) { + goto exit; + } #endif #if PY_MAJOR_VERSION >= 3 || PY_MINOR_VERSION >= 6 - microprotocols_add(&PyByteArray_Type, NULL, (PyObject*)&binaryType); + if (0 != microprotocols_add(&PyByteArray_Type, NULL, (PyObject*)&binaryType)) { + goto exit; + } #endif #if PY_MAJOR_VERSION >= 3 || PY_MINOR_VERSION >= 7 - microprotocols_add(&PyMemoryView_Type, NULL, (PyObject*)&binaryType); + if (0 != microprotocols_add(&PyMemoryView_Type, NULL, (PyObject*)&binaryType)) { + goto exit; + } #endif - microprotocols_add(&PyList_Type, NULL, (PyObject*)&listType); + if (0 != microprotocols_add(&PyList_Type, NULL, (PyObject*)&listType)) { + goto exit; + } /* the module has already been initialized, so we can obtain the callable objects directly from its dictionary :) */ - call = PyMapping_GetItemString(mod, "DateFromPy"); - microprotocols_add(PyDateTimeAPI->DateType, NULL, call); - call = PyMapping_GetItemString(mod, "TimeFromPy"); - microprotocols_add(PyDateTimeAPI->TimeType, NULL, call); - call = PyMapping_GetItemString(mod, "TimestampFromPy"); - microprotocols_add(PyDateTimeAPI->DateTimeType, NULL, call); - call = PyMapping_GetItemString(mod, "IntervalFromPy"); - microprotocols_add(PyDateTimeAPI->DeltaType, NULL, call); + if (!(call = PyMapping_GetItemString(mod, "DateFromPy"))) { goto exit; } + if (0 != microprotocols_add(PyDateTimeAPI->DateType, NULL, call)) { goto exit; } + Py_CLEAR(call); + + if (!(call = PyMapping_GetItemString(mod, "TimeFromPy"))) { goto exit; } + if (0 != microprotocols_add(PyDateTimeAPI->TimeType, NULL, call)) { goto exit; } + Py_CLEAR(call); + + if (!(call = PyMapping_GetItemString(mod, "TimestampFromPy"))) { goto exit; } + if (0 != microprotocols_add(PyDateTimeAPI->DateTimeType, NULL, call)) { goto exit; } + Py_CLEAR(call); + + if (!(call = PyMapping_GetItemString(mod, "IntervalFromPy"))) { goto exit; } + if (0 != microprotocols_add(PyDateTimeAPI->DeltaType, NULL, call)) { goto exit; } + Py_CLEAR(call); #ifdef HAVE_MXDATETIME /* as above, we use the callable objects from the psycopg module */ if (NULL != (call = PyMapping_GetItemString(mod, "TimestampFromMx"))) { - microprotocols_add(mxDateTime.DateTime_Type, NULL, call); + if (0 != microprotocols_add(mxDateTime.DateTime_Type, NULL, call)) { goto exit; } + Py_CLEAR(call); /* if we found the above, we have this too. */ - call = PyMapping_GetItemString(mod, "TimeFromMx"); - microprotocols_add(mxDateTime.DateTimeDelta_Type, NULL, call); + if (!(call = PyMapping_GetItemString(mod, "TimeFromMx"))) { goto exit; } + if (0 != microprotocols_add(mxDateTime.DateTimeDelta_Type, NULL, call)) { goto exit; } + Py_CLEAR(call); } else { PyErr_Clear(); } #endif + + /* Success! */ + rv = 0; + +exit: + Py_XDECREF(call); + + return rv; } /* psyco_encodings_fill @@ -960,7 +1002,7 @@ INIT_MODULE(_psycopg)(void) /* initialize microprotocols layer */ microprotocols_init(dict); - psyco_adapters_init(dict); + if (0 != psyco_adapters_init(dict)) { goto exit; } /* create a standard set of exceptions and add them to the module's dict */ if (0 != psyco_errors_init()) { goto exit; } From 1b27820389e304c68e5adbf02e906ac930485535 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 23 Feb 2012 19:33:29 +0000 Subject: [PATCH 04/36] Fixed refcount of exceptions dicts --- psycopg/psycopgmodule.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/psycopg/psycopgmodule.c b/psycopg/psycopgmodule.c index 577de15b..08125e6f 100644 --- a/psycopg/psycopgmodule.c +++ b/psycopg/psycopgmodule.c @@ -494,21 +494,22 @@ psyco_errors_init(void) live in _psycopg */ int i; - PyObject *dict; + PyObject *dict = NULL; PyObject *base; - PyObject *str; - PyObject *descr; + PyObject *str = NULL; + PyObject *descr = NULL; int rv = -1; static PyMethodDef psyco_error_reduce_ex_def = {"__reduce_ex__", psyco_error_reduce_ex, METH_VARARGS, "pickle helper"}; for (i=0; exctable[i].name; i++) { - dict = PyDict_New(); + if (!(dict = PyDict_New())) { goto exit; } if (exctable[i].docstr) { - str = Text_FromUTF8(exctable[i].docstr); - PyDict_SetItemString(dict, "__doc__", str); + if (!(str = Text_FromUTF8(exctable[i].docstr))) { goto exit; } + if (0 != PyDict_SetItemString(dict, "__doc__", str)) { goto exit; } + Py_CLEAR(str); } if (exctable[i].base == 0) { @@ -522,7 +523,11 @@ psyco_errors_init(void) else base = *exctable[i].base; - *exctable[i].exc = PyErr_NewException(exctable[i].name, base, dict); + if (!(*exctable[i].exc = PyErr_NewException( + exctable[i].name, base, dict))) { + goto exit; + } + Py_CLEAR(dict); } /* Make pgerror, pgcode and cursor default to None on psycopg @@ -546,12 +551,14 @@ psyco_errors_init(void) psyco_error_reduce_ex_def.ml_name, descr)) { goto exit; } - Py_DECREF(descr); #endif rv = 0; exit: + Py_XDECREF(descr); + Py_XDECREF(str); + Py_XDECREF(dict); return rv; } From 7d67ecbed3a6b5d2fbd78019fff571e906f985c6 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 23 Feb 2012 19:47:36 +0000 Subject: [PATCH 05/36] Fixed potential NULL incref --- psycopg/psycopgmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/psycopg/psycopgmodule.c b/psycopg/psycopgmodule.c index 08125e6f..eb8d55c9 100644 --- a/psycopg/psycopgmodule.c +++ b/psycopg/psycopgmodule.c @@ -710,7 +710,7 @@ psyco_GetDecimalType(void) } /* Store the object from future uses. */ - if (can_cache && !cachedType) { + if (can_cache && !cachedType && decimalType) { Py_INCREF(decimalType); cachedType = decimalType; } From 09be4dc5d15e64fb4dec7ef20c96168bcf860b49 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 23 Feb 2012 19:48:46 +0000 Subject: [PATCH 06/36] Fixed potential failures while setting exceptions attributes --- psycopg/psycopgmodule.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/psycopg/psycopgmodule.c b/psycopg/psycopgmodule.c index eb8d55c9..60cc8e2d 100644 --- a/psycopg/psycopgmodule.c +++ b/psycopg/psycopgmodule.c @@ -635,15 +635,17 @@ psyco_set_error(PyObject *exc, cursorObject *curs, const char *msg, } if (pgerror) { - t = conn_text_from_chars(conn, pgerror); - PyObject_SetAttrString(err, "pgerror", t); - Py_DECREF(t); + if ((t = conn_text_from_chars(conn, pgerror))) { + PyObject_SetAttrString(err, "pgerror", t); + Py_DECREF(t); + } } if (pgcode) { - t = conn_text_from_chars(conn, pgcode); - PyObject_SetAttrString(err, "pgcode", t); - Py_DECREF(t); + if ((t = conn_text_from_chars(conn, pgcode))) { + PyObject_SetAttrString(err, "pgcode", t); + Py_DECREF(t); + } } PyErr_SetObject(exc, err); From 3b36100ec1939f18030d6f78629d4c372ad525f8 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 23 Feb 2012 20:09:28 +0000 Subject: [PATCH 07/36] Dropped hardcoded list of exceptions in init functions Use the already available exctable array. This stops the gcc-python-plugin complaining about access to potentially uninitialized memory. --- psycopg/psycopgmodule.c | 54 ++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 30 deletions(-) diff --git a/psycopg/psycopgmodule.c b/psycopg/psycopgmodule.c index 60cc8e2d..60fec50b 100644 --- a/psycopg/psycopgmodule.c +++ b/psycopg/psycopgmodule.c @@ -565,41 +565,35 @@ exit: void psyco_errors_fill(PyObject *dict) { - PyDict_SetItemString(dict, "Error", Error); - PyDict_SetItemString(dict, "Warning", Warning); - PyDict_SetItemString(dict, "InterfaceError", InterfaceError); - PyDict_SetItemString(dict, "DatabaseError", DatabaseError); - PyDict_SetItemString(dict, "InternalError", InternalError); - PyDict_SetItemString(dict, "OperationalError", OperationalError); - PyDict_SetItemString(dict, "ProgrammingError", ProgrammingError); - PyDict_SetItemString(dict, "IntegrityError", IntegrityError); - PyDict_SetItemString(dict, "DataError", DataError); - PyDict_SetItemString(dict, "NotSupportedError", NotSupportedError); -#ifdef PSYCOPG_EXTENSIONS - PyDict_SetItemString(dict, "QueryCanceledError", QueryCanceledError); - PyDict_SetItemString(dict, "TransactionRollbackError", - TransactionRollbackError); -#endif + int i; + char *name; + + for (i = 0; exctable[i].name; i++) { + if (NULL == exctable[i].exc) { continue; } + + /* the name is the part after the last dot */ + name = strrchr(exctable[i].name, '.'); + name = name ? name + 1 : exctable[i].name; + + PyDict_SetItemString(dict, name, *exctable[i].exc); + } } void psyco_errors_set(PyObject *type) { - PyObject_SetAttrString(type, "Error", Error); - PyObject_SetAttrString(type, "Warning", Warning); - PyObject_SetAttrString(type, "InterfaceError", InterfaceError); - PyObject_SetAttrString(type, "DatabaseError", DatabaseError); - PyObject_SetAttrString(type, "InternalError", InternalError); - PyObject_SetAttrString(type, "OperationalError", OperationalError); - PyObject_SetAttrString(type, "ProgrammingError", ProgrammingError); - PyObject_SetAttrString(type, "IntegrityError", IntegrityError); - PyObject_SetAttrString(type, "DataError", DataError); - PyObject_SetAttrString(type, "NotSupportedError", NotSupportedError); -#ifdef PSYCOPG_EXTENSIONS - PyObject_SetAttrString(type, "QueryCanceledError", QueryCanceledError); - PyObject_SetAttrString(type, "TransactionRollbackError", - TransactionRollbackError); -#endif + int i; + char *name; + + for (i = 0; exctable[i].name; i++) { + if (NULL == exctable[i].exc) { continue; } + + /* the name is the part after the last dot */ + name = strrchr(exctable[i].name, '.'); + name = name ? name + 1 : exctable[i].name; + + PyObject_SetAttrString(type, name, *exctable[i].exc); + } } /* psyco_error_new From fc78fb09c05195b11b0af27fc9ae12a307f3f12b Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Fri, 24 Feb 2012 04:00:12 +0000 Subject: [PATCH 08/36] Dropped unused pq_resolve_critical() return value --- psycopg/pqpath.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/psycopg/pqpath.c b/psycopg/pqpath.c index 1734eceb..e3bba12b 100644 --- a/psycopg/pqpath.c +++ b/psycopg/pqpath.c @@ -249,7 +249,7 @@ pq_clear_critical(connectionObject *conn) } } -static PyObject * +static void pq_resolve_critical(connectionObject *conn, int close) { Dprintf("pq_resolve_critical: resolving %s", conn->critical); @@ -264,11 +264,10 @@ pq_resolve_critical(connectionObject *conn, int close) /* we don't want to destroy this connection but just close it */ if (close == 1) conn_close(conn); - + /* remember to clear the critical! */ - pq_clear_critical(conn); + pq_clear_critical(conn); } - return NULL; } /* pq_clear_async - clear the effects of a previous async query From 5f3f35a2c29bd91c52d0470ba5a6beed8b6e4bed Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Fri, 24 Feb 2012 04:25:08 +0000 Subject: [PATCH 09/36] Mark getnextarg function as returning a borrowed reference --- psycopg/bytes_format.c | 1 + psycopg/config.h | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/psycopg/bytes_format.c b/psycopg/bytes_format.c index 1e729e49..b22422b0 100644 --- a/psycopg/bytes_format.c +++ b/psycopg/bytes_format.c @@ -86,6 +86,7 @@ /* Helpers for formatstring */ +CPYCHECKER_RETURNS_BORROWED_REF Py_LOCAL_INLINE(PyObject *) getnextarg(PyObject *args, Py_ssize_t arglen, Py_ssize_t *p_argidx) { diff --git a/psycopg/config.h b/psycopg/config.h index 2112043b..321587a0 100644 --- a/psycopg/config.h +++ b/psycopg/config.h @@ -160,4 +160,12 @@ static double round(double num) #define isinf(x) (!finite((x)) && (x)==(x)) #endif +/* decorator for the gcc cpychecker plugin */ +#if defined(WITH_CPYCHECKER_RETURNS_BORROWED_REF_ATTRIBUTE) + #define CPYCHECKER_RETURNS_BORROWED_REF \ + __attribute__((cpychecker_returns_borrowed_ref)) +#else + #define CPYCHECKER_RETURNS_BORROWED_REF +#endif + #endif /* !defined(PSYCOPG_CONFIG_H) */ From a6df55f4e31a1a48886615098a21ccbf20782d76 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Fri, 24 Feb 2012 10:41:02 +0000 Subject: [PATCH 10/36] Flag the psycopg_ensure_*() functions as stealing a ref --- psycopg/config.h | 9 ++++++++- psycopg/psycopg.h | 8 ++++++-- psycopg/utils.c | 2 ++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/psycopg/config.h b/psycopg/config.h index 321587a0..2038ced7 100644 --- a/psycopg/config.h +++ b/psycopg/config.h @@ -160,7 +160,7 @@ static double round(double num) #define isinf(x) (!finite((x)) && (x)==(x)) #endif -/* decorator for the gcc cpychecker plugin */ +/* decorators for the gcc cpychecker plugin */ #if defined(WITH_CPYCHECKER_RETURNS_BORROWED_REF_ATTRIBUTE) #define CPYCHECKER_RETURNS_BORROWED_REF \ __attribute__((cpychecker_returns_borrowed_ref)) @@ -168,4 +168,11 @@ static double round(double num) #define CPYCHECKER_RETURNS_BORROWED_REF #endif +#if defined(WITH_CPYCHECKER_STEALS_REFERENCE_TO_ARG_ATTRIBUTE) + #define CPYCHECKER_STEALS_REFERENCE_TO_ARG(n) \ + __attribute__((cpychecker_steals_reference_to_arg(n))) +#else + #define CPYCHECKER_STEALS_REFERENCE_TO_ARG(n) +#endif + #endif /* !defined(PSYCOPG_CONFIG_H) */ diff --git a/psycopg/psycopg.h b/psycopg/psycopg.h index 2f06c378..01a68d92 100644 --- a/psycopg/psycopg.h +++ b/psycopg/psycopg.h @@ -127,10 +127,14 @@ HIDDEN char *psycopg_escape_string(PyObject *conn, const char *from, Py_ssize_t len, char *to, Py_ssize_t *tolen); HIDDEN char *psycopg_escape_identifier_easy(const char *from, Py_ssize_t len); HIDDEN char *psycopg_strdup(const char *from, Py_ssize_t len); -HIDDEN PyObject * psycopg_ensure_bytes(PyObject *obj); -HIDDEN PyObject * psycopg_ensure_text(PyObject *obj); HIDDEN int psycopg_is_text_file(PyObject *f); +CPYCHECKER_STEALS_REFERENCE_TO_ARG(1) +HIDDEN PyObject * psycopg_ensure_bytes(PyObject *obj); + +CPYCHECKER_STEALS_REFERENCE_TO_ARG(1) +HIDDEN PyObject * psycopg_ensure_text(PyObject *obj); + /* Exceptions docstrings */ #define Error_doc \ "Base class for error exceptions." diff --git a/psycopg/utils.c b/psycopg/utils.c index 2e81c113..4f40cc82 100644 --- a/psycopg/utils.c +++ b/psycopg/utils.c @@ -139,6 +139,7 @@ psycopg_strdup(const char *from, Py_ssize_t len) * * It is safe to call the function on NULL. */ +CPYCHECKER_STEALS_REFERENCE_TO_ARG(1) PyObject * psycopg_ensure_bytes(PyObject *obj) { @@ -169,6 +170,7 @@ psycopg_ensure_bytes(PyObject *obj) * The function is ref neutral: steals a ref from obj and adds one to the * return value. It is safe to call it on NULL. */ +CPYCHECKER_STEALS_REFERENCE_TO_ARG(1) PyObject * psycopg_ensure_text(PyObject *obj) { From 4ecfd48671b3caab869b72f95ba7c686321af6f1 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Fri, 24 Feb 2012 10:50:49 +0000 Subject: [PATCH 11/36] Fixed possible NULL dereferencing in notice process --- psycopg/connection_int.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/psycopg/connection_int.c b/psycopg/connection_int.c index 70465132..cb652249 100644 --- a/psycopg/connection_int.c +++ b/psycopg/connection_int.c @@ -120,8 +120,16 @@ conn_notice_process(connectionObject *self) /* Respect the order in which notices were produced, because in notice_list they are reversed (see ticket #9) */ - PyList_Insert(self->notice_list, nnotices, msg); - Py_DECREF(msg); + if (msg) { + PyList_Insert(self->notice_list, nnotices, msg); + Py_DECREF(msg); + } + else { + /* We don't really have a way to report errors, so gulp it. + * The function should only fail for out of memory, so we are + * likely going to die anyway. */ + PyErr_Clear(); + } notice = notice->next; } From 0ee641361bf44e6aacac87dd1c198f301cfcf038 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Fri, 24 Feb 2012 20:42:16 +0000 Subject: [PATCH 12/36] Flag a few other functions returning borrowed refs --- psycopg/connection_type.c | 1 + psycopg/cursor.h | 1 + psycopg/cursor_int.c | 1 + 3 files changed, 3 insertions(+) diff --git a/psycopg/connection_type.c b/psycopg/connection_type.c index 693de3ce..2f240917 100644 --- a/psycopg/connection_type.c +++ b/psycopg/connection_type.c @@ -550,6 +550,7 @@ psyco_conn_autocommit_get(connectionObject *self) return ret; } +CPYCHECKER_RETURNS_BORROWED_REF static PyObject * _psyco_conn_autocommit_set_checks(connectionObject *self) { diff --git a/psycopg/cursor.h b/psycopg/cursor.h index eecaa8ff..fe16b56c 100644 --- a/psycopg/cursor.h +++ b/psycopg/cursor.h @@ -85,6 +85,7 @@ struct cursorObject { /* C-callable functions in cursor_int.c and cursor_ext.c */ +CPYCHECKER_RETURNS_BORROWED_REF HIDDEN PyObject *curs_get_cast(cursorObject *self, PyObject *oid); HIDDEN void curs_reset(cursorObject *self); diff --git a/psycopg/cursor_int.c b/psycopg/cursor_int.c index 40e0bae3..8004bef9 100644 --- a/psycopg/cursor_int.c +++ b/psycopg/cursor_int.c @@ -38,6 +38,7 @@ * Return a borrowed reference. */ +CPYCHECKER_RETURNS_BORROWED_REF PyObject * curs_get_cast(cursorObject *self, PyObject *oid) { From efee0493380c12b17c39a76761a3c72220d44132 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Fri, 24 Feb 2012 21:44:29 +0000 Subject: [PATCH 13/36] Added error check in _mogrify for failed tuple creation --- psycopg/cursor_type.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index bb9db691..f6cf3a9f 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -217,7 +217,10 @@ _mogrify(PyObject *var, PyObject *fmt, cursorObject *curs, PyObject **new) } if (n == NULL) { - n = PyTuple_New(PyObject_Length(var)); + if (!(n = PyTuple_New(PyObject_Length(var)))) { + Py_DECREF(value); + return -1; + } } /* let's have d point just after the '%' */ From 94a53b48df088d0a686e42617db29d61c7cb5804 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Fri, 24 Feb 2012 21:52:36 +0000 Subject: [PATCH 14/36] Building rows simplified Dropped repeated checks for tuple_factory. Internal functions refactored a bit. --- psycopg/cursor_type.c | 99 +++++++++++++++++++------------------------ 1 file changed, 43 insertions(+), 56 deletions(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index f6cf3a9f..743b2d8f 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -664,13 +664,14 @@ _psyco_curs_prefetch(cursorObject *self) return i; } -static PyObject * +static int _psyco_curs_buildrow_fill(cursorObject *self, PyObject *res, int row, int n, int istuple) { int i, len, err; const char *str; PyObject *val; + int rv = -1; for (i=0; i < n; i++) { if (PQgetisnull(self->pgres, row, i)) { @@ -685,59 +686,59 @@ _psyco_curs_buildrow_fill(cursorObject *self, PyObject *res, Dprintf("_psyco_curs_buildrow: row %ld, element %d, len %d", self->row, i, len); - val = typecast_cast(PyTuple_GET_ITEM(self->casts, i), str, len, - (PyObject*)self); + if (!(val = typecast_cast(PyTuple_GET_ITEM(self->casts, i), str, len, + (PyObject*)self))) { + goto exit; + } - if (val) { - Dprintf("_psyco_curs_buildrow: val->refcnt = " - FORMAT_CODE_PY_SSIZE_T, - Py_REFCNT(val) - ); - if (istuple) { - PyTuple_SET_ITEM(res, i, val); - } - else { - err = PySequence_SetItem(res, i, val); - Py_DECREF(val); - if (err == -1) { - Py_DECREF(res); - res = NULL; - break; - } - } + Dprintf("_psyco_curs_buildrow: val->refcnt = " + FORMAT_CODE_PY_SSIZE_T, + Py_REFCNT(val) + ); + if (istuple) { + PyTuple_SET_ITEM(res, i, val); } else { - /* an error occurred in the type system, we return NULL to raise - an exception. the typecast code should already have set the - exception type and text */ - Py_DECREF(res); - res = NULL; - break; + err = PySequence_SetItem(res, i, val); + Py_DECREF(val); + if (err == -1) { goto exit; } } } - return res; + + rv = 0; + +exit: + return rv; } static PyObject * _psyco_curs_buildrow(cursorObject *self, int row) { int n; + int istuple; + PyObject *t = NULL; + PyObject *rv = NULL; n = PQnfields(self->pgres); - return _psyco_curs_buildrow_fill(self, PyTuple_New(n), row, n, 1); -} + istuple = (self->tuple_factory == Py_None); -static PyObject * -_psyco_curs_buildrow_with_factory(cursorObject *self, int row) -{ - int n; - PyObject *res; + if (istuple) { + t = PyTuple_New(n); + } + else { + t = PyObject_CallFunctionObjArgs(self->tuple_factory, self, NULL); + } + if (!t) { goto exit; } - n = PQnfields(self->pgres); - if (!(res = PyObject_CallFunctionObjArgs(self->tuple_factory, self, NULL))) - return NULL; + if (0 == _psyco_curs_buildrow_fill(self, t, row, n, istuple)) { + rv = t; + t = NULL; + } + +exit: + Py_XDECREF(t); + return rv; - return _psyco_curs_buildrow_fill(self, res, row, n, 0); } static PyObject * @@ -769,11 +770,7 @@ psyco_curs_fetchone(cursorObject *self, PyObject *args) return Py_None; } - if (self->tuple_factory == Py_None) - res = _psyco_curs_buildrow(self, self->row); - else - res = _psyco_curs_buildrow_with_factory(self, self->row); - + res = _psyco_curs_buildrow(self, self->row); self->row++; /* move the counter to next line */ /* if the query was async aggresively free pgres, to allow @@ -820,11 +817,7 @@ psyco_curs_next_named(cursorObject *self) return NULL; } - if (self->tuple_factory == Py_None) - res = _psyco_curs_buildrow(self, self->row); - else - res = _psyco_curs_buildrow_with_factory(self, self->row); - + res = _psyco_curs_buildrow(self, self->row); self->row++; /* move the counter to next line */ /* if the query was async aggresively free pgres, to allow @@ -900,10 +893,7 @@ psyco_curs_fetchmany(cursorObject *self, PyObject *args, PyObject *kwords) list = PyList_New(size); for (i = 0; i < size; i++) { - if (self->tuple_factory == Py_None) - res = _psyco_curs_buildrow(self, self->row); - else - res = _psyco_curs_buildrow_with_factory(self, self->row); + res = _psyco_curs_buildrow(self, self->row); self->row++; @@ -965,10 +955,7 @@ psyco_curs_fetchall(cursorObject *self, PyObject *args) list = PyList_New(size); for (i = 0; i < size; i++) { - if (self->tuple_factory == Py_None) - res = _psyco_curs_buildrow(self, self->row); - else - res = _psyco_curs_buildrow_with_factory(self, self->row); + res = _psyco_curs_buildrow(self, self->row); self->row++; From 6d76e81166a97fb1eb950b126979185125c5361b Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Fri, 24 Feb 2012 22:06:00 +0000 Subject: [PATCH 15/36] Fixed possible NULL dereferencing in callproc() --- psycopg/cursor_type.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index 743b2d8f..f515d6b8 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -996,7 +996,7 @@ psyco_curs_callproc(cursorObject *self, PyObject *args, PyObject *kwargs) if (!PyArg_ParseTuple(args, "s#|O", &procname, &procname_len, ¶meters )) - { return NULL; } + { goto exit; } EXC_IF_CURS_CLOSED(self); EXC_IF_ASYNC_IN_PROGRESS(self, callproc); @@ -1005,10 +1005,10 @@ psyco_curs_callproc(cursorObject *self, PyObject *args, PyObject *kwargs) if (self->name != NULL) { psyco_set_error(ProgrammingError, self, "can't call .callproc() on named cursors", NULL, NULL); - return NULL; + goto exit; } - if(parameters != Py_None) { + if (parameters != Py_None) { nparameters = PyObject_Length(parameters); if (nparameters < 0) nparameters = 0; } @@ -1017,7 +1017,8 @@ psyco_curs_callproc(cursorObject *self, PyObject *args, PyObject *kwargs) sl = procname_len + 17 + nparameters*3 - (nparameters ? 1 : 0); sql = (char*)PyMem_Malloc(sl); if (sql == NULL) { - return PyErr_NoMemory(); + PyErr_NoMemory(); + goto exit; } sprintf(sql, "SELECT * FROM %s(", procname); @@ -1027,15 +1028,16 @@ psyco_curs_callproc(cursorObject *self, PyObject *args, PyObject *kwargs) sql[sl-2] = ')'; sql[sl-1] = '\0'; - operation = Bytes_FromString(sql); - PyMem_Free((void*)sql); + if (!(operation = Bytes_FromString(sql))) { goto exit; } if (_psyco_curs_execute(self, operation, parameters, self->conn->async)) { Py_INCREF(parameters); res = parameters; } - Py_DECREF(operation); +exit: + Py_XDECREF(operation); + PyMem_Free((void*)sql); return res; } From 67712e42268176894a08462006020a0a3946a6ce Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Fri, 24 Feb 2012 22:24:54 +0000 Subject: [PATCH 16/36] Fixed possible NULL dereferencing in fetchmany()/fetchall() --- psycopg/cursor_type.c | 69 ++++++++++++++++++++++++++----------------- 1 file changed, 42 insertions(+), 27 deletions(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index f515d6b8..4c4d5af6 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -844,7 +844,9 @@ static PyObject * psyco_curs_fetchmany(cursorObject *self, PyObject *args, PyObject *kwords) { int i; - PyObject *list, *res; + PyObject *list = NULL; + PyObject *row = NULL; + PyObject *rv = NULL; PyObject *pysize = NULL; long int size = self->arraysize; @@ -875,8 +877,8 @@ psyco_curs_fetchmany(cursorObject *self, PyObject *args, PyObject *kwords) EXC_IF_TPC_PREPARED(self->conn, fetchone); PyOS_snprintf(buffer, 127, "FETCH FORWARD %d FROM \"%s\"", (int)size, self->name); - if (pq_execute(self, buffer, 0) == -1) return NULL; - if (_psyco_curs_prefetch(self) < 0) return NULL; + if (pq_execute(self, buffer, 0) == -1) { goto exit; } + if (_psyco_curs_prefetch(self) < 0) { goto exit; } } /* make sure size is not > than the available number of rows */ @@ -887,23 +889,21 @@ psyco_curs_fetchmany(cursorObject *self, PyObject *args, PyObject *kwords) Dprintf("psyco_curs_fetchmany: size = %ld", size); if (size <= 0) { - return PyList_New(0); + rv = PyList_New(0); + goto exit; } - list = PyList_New(size); + if (!(list = PyList_New(size))) { goto exit; } for (i = 0; i < size; i++) { - res = _psyco_curs_buildrow(self, self->row); - + row = _psyco_curs_buildrow(self, self->row); self->row++; - if (res == NULL) { - Py_DECREF(list); - return NULL; - } + if (row == NULL) { goto exit; } - PyList_SET_ITEM(list, i, res); + PyList_SET_ITEM(list, i, row); } + row = NULL; /* if the query was async aggresively free pgres, to allow successive requests to reallocate it */ @@ -912,7 +912,15 @@ psyco_curs_fetchmany(cursorObject *self, PyObject *args, PyObject *kwords) && PyWeakref_GetObject(self->conn->async_cursor) == (PyObject*)self) IFCLEARPGRES(self->pgres); - return list; + /* success */ + rv = list; + list = NULL; + +exit: + Py_XDECREF(list); + Py_XDECREF(row); + + return rv; } @@ -929,7 +937,9 @@ static PyObject * psyco_curs_fetchall(cursorObject *self, PyObject *args) { int i, size; - PyObject *list, *res; + PyObject *list = NULL; + PyObject *row = NULL; + PyObject *rv = NULL; EXC_IF_CURS_CLOSED(self); if (_psyco_curs_prefetch(self) < 0) return NULL; @@ -942,30 +952,27 @@ psyco_curs_fetchall(cursorObject *self, PyObject *args) EXC_IF_ASYNC_IN_PROGRESS(self, fetchall); EXC_IF_TPC_PREPARED(self->conn, fetchall); PyOS_snprintf(buffer, 127, "FETCH FORWARD ALL FROM \"%s\"", self->name); - if (pq_execute(self, buffer, 0) == -1) return NULL; - if (_psyco_curs_prefetch(self) < 0) return NULL; + if (pq_execute(self, buffer, 0) == -1) { goto exit; } + if (_psyco_curs_prefetch(self) < 0) { goto exit; } } size = self->rowcount - self->row; if (size <= 0) { - return PyList_New(0); + rv = PyList_New(0); + goto exit; } - list = PyList_New(size); + if (!(list = PyList_New(size))) { goto exit; } for (i = 0; i < size; i++) { - res = _psyco_curs_buildrow(self, self->row); - + row = _psyco_curs_buildrow(self, self->row); self->row++; + if (row == NULL) { goto exit; } - if (res == NULL) { - Py_DECREF(list); - return NULL; - } - - PyList_SET_ITEM(list, i, res); + PyList_SET_ITEM(list, i, row); } + row = NULL; /* if the query was async aggresively free pgres, to allow successive requests to reallocate it */ @@ -974,7 +981,15 @@ psyco_curs_fetchall(cursorObject *self, PyObject *args) && PyWeakref_GetObject(self->conn->async_cursor) == (PyObject*)self) IFCLEARPGRES(self->pgres); - return list; + /* success */ + rv = list; + list = NULL; + +exit: + Py_XDECREF(list); + Py_XDECREF(row); + + return rv; } From a167822e2692a8372d4cbe4570a3616cbd57ff6b Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Wed, 29 Feb 2012 23:51:51 +0000 Subject: [PATCH 17/36] Use the newly provided attributes to validate exceptions raising Be more consistent in using 0 for success, <0 for failure, and to check for values < 0 instead of specific -1. --- psycopg/config.h | 24 +++++++++++++---- psycopg/connection.h | 7 +++++ psycopg/connection_int.c | 9 ++++++- psycopg/connection_type.c | 2 +- psycopg/cursor_type.c | 56 +++++++++++++++++++-------------------- psycopg/pqpath.c | 34 ++++++++++++++---------- psycopg/pqpath.h | 6 +++++ psycopg/psycopg.h | 1 + psycopg/psycopgmodule.c | 3 ++- 9 files changed, 92 insertions(+), 50 deletions(-) diff --git a/psycopg/config.h b/psycopg/config.h index 2038ced7..4d3fa99b 100644 --- a/psycopg/config.h +++ b/psycopg/config.h @@ -162,17 +162,31 @@ static double round(double num) /* decorators for the gcc cpychecker plugin */ #if defined(WITH_CPYCHECKER_RETURNS_BORROWED_REF_ATTRIBUTE) - #define CPYCHECKER_RETURNS_BORROWED_REF \ +#define CPYCHECKER_RETURNS_BORROWED_REF \ __attribute__((cpychecker_returns_borrowed_ref)) #else - #define CPYCHECKER_RETURNS_BORROWED_REF +#define CPYCHECKER_RETURNS_BORROWED_REF #endif #if defined(WITH_CPYCHECKER_STEALS_REFERENCE_TO_ARG_ATTRIBUTE) - #define CPYCHECKER_STEALS_REFERENCE_TO_ARG(n) \ - __attribute__((cpychecker_steals_reference_to_arg(n))) +#define CPYCHECKER_STEALS_REFERENCE_TO_ARG(n) \ + __attribute__((cpychecker_steals_reference_to_arg(n))) #else - #define CPYCHECKER_STEALS_REFERENCE_TO_ARG(n) +#define CPYCHECKER_STEALS_REFERENCE_TO_ARG(n) +#endif + +#if defined(WITH_CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION_ATTRIBUTE) +#define CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION \ + __attribute__((cpychecker_negative_result_sets_exception)) +#else +#define CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION +#endif + +#if defined(WITH_CPYCHECKER_SETS_EXCEPTION_ATTRIBUTE) +#define CPYCHECKER_SETS_EXCEPTION \ + __attribute__((cpychecker_sets_exception)) +#else +#define CPYCHECKER_SETS_EXCEPTION #endif #endif /* !defined(PSYCOPG_CONFIG_H) */ diff --git a/psycopg/connection.h b/psycopg/connection.h index 9a9ea420..d4a83480 100644 --- a/psycopg/connection.h +++ b/psycopg/connection.h @@ -131,6 +131,7 @@ typedef struct { /* C-callable functions in connection_int.c and connection_ext.c */ HIDDEN PyObject *conn_text_from_chars(connectionObject *pgconn, const char *str); HIDDEN int conn_get_standard_conforming_strings(PGconn *pgconn); +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION HIDDEN int conn_get_isolation_level(connectionObject *self); HIDDEN int conn_get_protocol_version(PGconn *pgconn); HIDDEN int conn_get_server_version(PGconn *pgconn); @@ -141,16 +142,22 @@ HIDDEN void conn_notifies_process(connectionObject *self); HIDDEN int conn_setup(connectionObject *self, PGconn *pgconn); HIDDEN int conn_connect(connectionObject *self, long int async); HIDDEN void conn_close(connectionObject *self); +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION HIDDEN int conn_commit(connectionObject *self); +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION HIDDEN int conn_rollback(connectionObject *self); HIDDEN int conn_set_session(connectionObject *self, const char *isolevel, const char *readonly, const char *deferrable, int autocommit); HIDDEN int conn_set_autocommit(connectionObject *self, int value); +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION HIDDEN int conn_switch_isolation_level(connectionObject *self, int level); +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION HIDDEN int conn_set_client_encoding(connectionObject *self, const char *enc); HIDDEN int conn_poll(connectionObject *self); +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION HIDDEN int conn_tpc_begin(connectionObject *self, XidObject *xid); +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION HIDDEN int conn_tpc_command(connectionObject *self, const char *cmd, XidObject *xid); HIDDEN PyObject *conn_tpc_recover(connectionObject *self); diff --git a/psycopg/connection_int.c b/psycopg/connection_int.c index cb652249..4c12e1d7 100644 --- a/psycopg/connection_int.c +++ b/psycopg/connection_int.c @@ -370,6 +370,7 @@ exit: } +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION int conn_get_isolation_level(connectionObject *self) { @@ -947,6 +948,7 @@ conn_close(connectionObject *self) /* conn_commit - commit on a connection */ +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION int conn_commit(connectionObject *self) { @@ -958,6 +960,7 @@ conn_commit(connectionObject *self) /* conn_rollback - rollback a connection */ +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION int conn_rollback(connectionObject *self) { @@ -1040,6 +1043,7 @@ conn_set_autocommit(connectionObject *self, int value) /* conn_switch_isolation_level - switch isolation level on the connection */ +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION int conn_switch_isolation_level(connectionObject *self, int level) { @@ -1122,12 +1126,13 @@ endlock: /* conn_set_client_encoding - switch client encoding on connection */ +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION int conn_set_client_encoding(connectionObject *self, const char *enc) { PGresult *pgres = NULL; char *error = NULL; - int res = 1; + int res = -1; char *codec = NULL; char *clean_enc = NULL; @@ -1195,6 +1200,7 @@ exit: * in regular transactions, as PostgreSQL won't even know we are in a TPC * until PREPARE. */ +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION int conn_tpc_begin(connectionObject *self, XidObject *xid) { @@ -1229,6 +1235,7 @@ conn_tpc_begin(connectionObject *self, XidObject *xid) * The function doesn't change the connection state as it can be used * for many commands and for recovered transactions. */ +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION int conn_tpc_command(connectionObject *self, const char *cmd, XidObject *xid) { diff --git a/psycopg/connection_type.c b/psycopg/connection_type.c index 2f240917..d88126ba 100644 --- a/psycopg/connection_type.c +++ b/psycopg/connection_type.c @@ -638,7 +638,7 @@ psyco_conn_set_client_encoding(connectionObject *self, PyObject *args) if (!PyArg_ParseTuple(args, "s", &enc)) return NULL; - if (conn_set_client_encoding(self, enc) == 0) { + if (conn_set_client_encoding(self, enc) >= 0) { Py_INCREF(Py_None); rv = Py_None; } diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index 4c4d5af6..b60263b9 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -75,6 +75,7 @@ psyco_curs_close(cursorObject *self, PyObject *args) /* mogrify a query string and build argument array or dict */ +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION static int _mogrify(PyObject *var, PyObject *fmt, cursorObject *curs, PyObject **new) { @@ -359,11 +360,13 @@ _psyco_curs_merge_query_args(cursorObject *self, #define psyco_curs_execute_doc \ "execute(query, vars=None) -- Execute query with bound vars." +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION static int _psyco_curs_execute(cursorObject *self, PyObject *operation, PyObject *vars, long int async) { - int res = 0; + int res = -1; + int tmp; PyObject *fquery, *cvt = NULL; operation = _psyco_curs_validate_sql_basic(self, operation); @@ -371,7 +374,7 @@ _psyco_curs_execute(cursorObject *self, /* Any failure from here forward should 'goto fail' rather than 'return 0' directly. */ - if (operation == NULL) { goto fail; } + if (operation == NULL) { goto exit; } IFCLEARPGRES(self->pgres); @@ -388,12 +391,12 @@ _psyco_curs_execute(cursorObject *self, if (vars && vars != Py_None) { - if(_mogrify(vars, operation, self, &cvt) == -1) { goto fail; } + if (0 > _mogrify(vars, operation, self, &cvt)) { goto exit; } } if (vars && cvt) { if (!(fquery = _psyco_curs_merge_query_args(self, operation, cvt))) { - goto fail; + goto exit; } if (self->name != NULL) { @@ -427,25 +430,20 @@ _psyco_curs_execute(cursorObject *self, /* At this point, the SQL statement must be str, not unicode */ - res = pq_execute(self, Bytes_AS_STRING(self->query), async); - Dprintf("psyco_curs_execute: res = %d, pgres = %p", res, self->pgres); - if (res == -1) { goto fail; } + tmp = pq_execute(self, Bytes_AS_STRING(self->query), async); + Dprintf("psyco_curs_execute: res = %d, pgres = %p", tmp, self->pgres); + if (tmp < 0) { goto exit; } - res = 1; /* Success */ - goto cleanup; + res = 0; /* Success */ - fail: - res = 0; - /* Fall through to cleanup */ - cleanup: - /* Py_XDECREF(operation) is safe because the original reference passed - by the caller was overwritten with either NULL or a new - reference */ - Py_XDECREF(operation); +exit: + /* Py_XDECREF(operation) is safe because the original reference passed + by the caller was overwritten with either NULL or a new + reference */ + Py_XDECREF(operation); + Py_XDECREF(cvt); - Py_XDECREF(cvt); - - return res; + return res; } static PyObject * @@ -479,13 +477,13 @@ psyco_curs_execute(cursorObject *self, PyObject *args, PyObject *kwargs) EXC_IF_ASYNC_IN_PROGRESS(self, execute); EXC_IF_TPC_PREPARED(self->conn, execute); - if (_psyco_curs_execute(self, operation, vars, self->conn->async)) { - Py_INCREF(Py_None); - return Py_None; - } - else { + if (0 > _psyco_curs_execute(self, operation, vars, self->conn->async)) { return NULL; } + + /* success */ + Py_INCREF(Py_None); + return Py_None; } #define psyco_curs_executemany_doc \ @@ -524,7 +522,7 @@ psyco_curs_executemany(cursorObject *self, PyObject *args, PyObject *kwargs) } while ((v = PyIter_Next(vars)) != NULL) { - if (_psyco_curs_execute(self, operation, v, 0) == 0) { + if (0 > _psyco_curs_execute(self, operation, v, 0)) { Py_DECREF(v); Py_XDECREF(iter); return NULL; @@ -571,7 +569,7 @@ _psyco_curs_mogrify(cursorObject *self, if (vars && vars != Py_None) { - if (_mogrify(vars, operation, self, &cvt) == -1) { + if (0 > _mogrify(vars, operation, self, &cvt)) { goto cleanup; } } @@ -647,6 +645,7 @@ psyco_curs_cast(cursorObject *self, PyObject *args) "default) or using the sequence factory previously set in the\n" \ "`row_factory` attribute. Return `!None` when no more data is available.\n" +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION static int _psyco_curs_prefetch(cursorObject *self) { @@ -664,6 +663,7 @@ _psyco_curs_prefetch(cursorObject *self) return i; } +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION static int _psyco_curs_buildrow_fill(cursorObject *self, PyObject *res, int row, int n, int istuple) @@ -1045,7 +1045,7 @@ psyco_curs_callproc(cursorObject *self, PyObject *args, PyObject *kwargs) if (!(operation = Bytes_FromString(sql))) { goto exit; } - if (_psyco_curs_execute(self, operation, parameters, self->conn->async)) { + if (0 <= _psyco_curs_execute(self, operation, parameters, self->conn->async)) { Py_INCREF(parameters); res = parameters; } diff --git a/psycopg/pqpath.c b/psycopg/pqpath.c index e3bba12b..816adb24 100644 --- a/psycopg/pqpath.c +++ b/psycopg/pqpath.c @@ -64,6 +64,7 @@ strip_severity(const char *msg) code. A list of error codes can be found at: http://www.postgresql.org/docs/current/static/errcodes-appendix.html */ +CPYCHECKER_RETURNS_BORROWED_REF static PyObject * exception_from_sqlstate(const char *sqlstate) { @@ -151,6 +152,7 @@ exception_from_sqlstate(const char *sqlstate) This function should be called while holding the GIL. */ +CPYCHECKER_SETS_EXCEPTION static void pq_raise(connectionObject *conn, cursorObject *curs, PGresult *pgres) { @@ -164,7 +166,7 @@ pq_raise(connectionObject *conn, cursorObject *curs, PGresult *pgres) "psycopg went psycotic and raised a null error"); return; } - + /* if the connection has somehow beed broken, we mark the connection object as closed but requiring cleanup */ if (conn->pgconn != NULL && PQstatus(conn->pgconn) == CONNECTION_BAD) @@ -249,7 +251,10 @@ pq_clear_critical(connectionObject *conn) } } -static void +/* return -1 if the exception is set (i.e. if conn->critical is set), + * else 0 */ +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION +static int pq_resolve_critical(connectionObject *conn, int close) { Dprintf("pq_resolve_critical: resolving %s", conn->critical); @@ -267,7 +272,10 @@ pq_resolve_critical(connectionObject *conn, int close) /* remember to clear the critical! */ pq_clear_critical(conn); + + return -1; } + return 0; } /* pq_clear_async - clear the effects of a previous async query @@ -381,6 +389,7 @@ cleanup: This function should be called while holding the global interpreter lock. */ +CPYCHECKER_SETS_EXCEPTION void pq_complete_error(connectionObject *conn, PGresult **pgres, char **error) { @@ -474,6 +483,7 @@ pq_commit(connectionObject *conn) return retvalue; } +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION int pq_abort_locked(connectionObject *conn, PGresult **pgres, char **error, PyThreadState **tstate) @@ -501,6 +511,7 @@ pq_abort_locked(connectionObject *conn, PGresult **pgres, char **error, This function should be called while holding the global interpreter lock. */ +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION int pq_abort(connectionObject *conn) { @@ -543,6 +554,7 @@ pq_abort(connectionObject *conn) connection without holding the global interpreter lock. */ +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION int pq_reset_locked(connectionObject *conn, PGresult **pgres, char **error, PyThreadState **tstate) @@ -835,6 +847,7 @@ pq_flush(connectionObject *conn) this fucntion locks the connection object this function call Py_*_ALLOW_THREADS macros */ +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION int pq_execute(cursorObject *curs, const char *query, int async) { @@ -845,8 +858,7 @@ pq_execute(cursorObject *curs, const char *query, int async) /* if the status of the connection is critical raise an exception and definitely close the connection */ if (curs->conn->critical) { - pq_resolve_critical(curs->conn, 1); - return -1; + return pq_resolve_critical(curs->conn, 1); } /* check status of connection, raise error if not OK */ @@ -941,7 +953,7 @@ pq_execute(cursorObject *curs, const char *query, int async) to respect the old DBAPI-2.0 compatible behaviour */ if (async == 0) { Dprintf("pq_execute: entering syncronous DBAPI compatibility mode"); - if (pq_fetch(curs) == -1) return -1; + if (pq_fetch(curs) < 0) return -1; } else { curs->conn->async_status = async_status; @@ -1015,6 +1027,7 @@ pq_get_last_result(connectionObject *conn) 1 - result from backend (possibly data is ready) */ +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION static int _pq_fetch_tuples(cursorObject *curs) { @@ -1415,8 +1428,7 @@ pq_fetch(cursorObject *curs) Dprintf("pq_fetch: got a NULL pgres, checking for critical"); pq_set_critical(curs->conn); if (curs->conn->critical) { - pq_resolve_critical(curs->conn); - return -1; + return pq_resolve_critical(curs->conn); } else { return 0; @@ -1492,13 +1504,7 @@ pq_fetch(cursorObject *curs) raise the exception but we avoid to close the connection) */ Dprintf("pq_fetch: fetching done; check for critical errors"); if (curs->conn->critical) { - if (ex == -1) { - pq_resolve_critical(curs->conn, 1); - } - else { - pq_resolve_critical(curs->conn, 0); - } - return -1; + return pq_resolve_critical(curs->conn, ex == -1 ? 1 : 0); } return ex; diff --git a/psycopg/pqpath.h b/psycopg/pqpath.h index bf012ade..ef0e3252 100644 --- a/psycopg/pqpath.h +++ b/psycopg/pqpath.h @@ -35,17 +35,22 @@ /* exported functions */ HIDDEN PGresult *pq_get_last_result(connectionObject *conn); +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION HIDDEN int pq_fetch(cursorObject *curs); +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION HIDDEN int pq_execute(cursorObject *curs, const char *query, int async); HIDDEN int pq_send_query(connectionObject *conn, const char *query); HIDDEN int pq_begin_locked(connectionObject *conn, PGresult **pgres, char **error, PyThreadState **tstate); HIDDEN int pq_commit(connectionObject *conn); +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION HIDDEN int pq_abort_locked(connectionObject *conn, PGresult **pgres, char **error, PyThreadState **tstate); +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION HIDDEN int pq_abort(connectionObject *conn); HIDDEN int pq_reset_locked(connectionObject *conn, PGresult **pgres, char **error, PyThreadState **tstate); +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION HIDDEN int pq_reset(connectionObject *conn); HIDDEN char *pq_get_guc_locked(connectionObject *conn, const char *param, PGresult **pgres, @@ -69,6 +74,7 @@ HIDDEN int pq_execute_command_locked(connectionObject *conn, const char *query, PGresult **pgres, char **error, PyThreadState **tstate); +CPYCHECKER_SETS_EXCEPTION HIDDEN void pq_complete_error(connectionObject *conn, PGresult **pgres, char **error); diff --git a/psycopg/psycopg.h b/psycopg/psycopg.h index 01a68d92..60f01b12 100644 --- a/psycopg/psycopg.h +++ b/psycopg/psycopg.h @@ -120,6 +120,7 @@ HIDDEN PyObject *psyco_GetDecimalType(void); typedef struct cursorObject cursorObject; /* some utility functions */ +CPYCHECKER_SETS_EXCEPTION HIDDEN void psyco_set_error(PyObject *exc, cursorObject *curs, const char *msg, const char *pgerror, const char *pgcode); diff --git a/psycopg/psycopgmodule.c b/psycopg/psycopgmodule.c index 60fec50b..7d77ee17 100644 --- a/psycopg/psycopgmodule.c +++ b/psycopg/psycopgmodule.c @@ -596,10 +596,11 @@ psyco_errors_set(PyObject *type) } } -/* psyco_error_new +/* psyco_set_error Create a new error of the given type with extra attributes. */ +CPYCHECKER_SETS_EXCEPTION void psyco_set_error(PyObject *exc, cursorObject *curs, const char *msg, const char *pgerror, const char *pgcode) From 94327872790cc1649cfe08165d111aa25f045f03 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Wed, 29 Feb 2012 23:53:00 +0000 Subject: [PATCH 18/36] Work around a false positive returned by the static checker To be submitted to the author. --- psycopg/pqpath.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/psycopg/pqpath.c b/psycopg/pqpath.c index 816adb24..e8506990 100644 --- a/psycopg/pqpath.c +++ b/psycopg/pqpath.c @@ -956,9 +956,10 @@ pq_execute(cursorObject *curs, const char *query, int async) if (pq_fetch(curs) < 0) return -1; } else { + PyObject *tmp; curs->conn->async_status = async_status; - curs->conn->async_cursor = PyWeakref_NewRef((PyObject *)curs, NULL); - if (!curs->conn->async_cursor) { + curs->conn->async_cursor = tmp = PyWeakref_NewRef((PyObject *)curs, NULL); + if (!tmp) { /* weakref creation failed */ return -1; } From f2e4a8ed78973a0d14dbe0d463911436562683f3 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 1 Mar 2012 00:24:52 +0000 Subject: [PATCH 19/36] Functions setting exception return a negative value on error This works around another shortcoming of the static checker; also to be discussed with the author. --- psycopg/connection_int.c | 42 +++++++++++++++++++++++----------------- psycopg/psycopg.h | 2 +- psycopg/utils.c | 18 ++++++++--------- 3 files changed, 34 insertions(+), 28 deletions(-) diff --git a/psycopg/connection_int.c b/psycopg/connection_int.c index 4c12e1d7..7375da3b 100644 --- a/psycopg/connection_int.c +++ b/psycopg/connection_int.c @@ -250,19 +250,21 @@ conn_get_standard_conforming_strings(PGconn *pgconn) /* Remove irrelevant chars from encoding name and turn it uppercase. * - * Return a buffer allocated on Python heap, - * NULL and set an exception on error. + * Return a buffer allocated on Python heap into 'clean' and return 0 on + * success, otherwise return -1 and set an exception. */ -static char * -clean_encoding_name(const char *enc) +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION +static int +clear_encoding_name(const char *enc, char **clean) { const char *i = enc; - char *rv, *j; + char *j, *buf; + int rv = -1; /* convert to upper case and remove '-' and '_' from string */ - if (!(j = rv = PyMem_Malloc(strlen(enc) + 1))) { + if (!(j = buf = PyMem_Malloc(strlen(enc) + 1))) { PyErr_NoMemory(); - return NULL; + goto exit; } while (*i) { @@ -275,25 +277,29 @@ clean_encoding_name(const char *enc) } *j = '\0'; - Dprintf("clean_encoding_name: %s -> %s", enc, rv); + Dprintf("clear_encoding_name: %s -> %s", enc, buf); + *clean = buf; + rv = 0; +exit: return rv; } /* Convert a PostgreSQL encoding to a Python codec. * - * Return a new copy of the codec name allocated on the Python heap, - * NULL with exception in case of error. + * Set 'codec' to a new copy of the codec name allocated on the Python heap. + * Return 0 in case of success, else -1 and set an exception. * * 'enc' should be already normalized (uppercase, no - or _). */ -static char * -conn_encoding_to_codec(const char *enc) +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION +static int +conn_encoding_to_codec(const char *enc, char **codec) { char *tmp; Py_ssize_t size; PyObject *pyenc = NULL; - char *rv = NULL; + int rv = -1; /* Find the Py codec name from the PG encoding */ if (!(pyenc = PyDict_GetItemString(psycoEncodings, enc))) { @@ -313,7 +319,7 @@ conn_encoding_to_codec(const char *enc) } /* have our own copy of the python codec name */ - rv = psycopg_strdup(tmp, size); + rv = psycopg_strdup(codec, tmp, size); exit: Py_XDECREF(pyenc); @@ -343,12 +349,12 @@ conn_read_encoding(connectionObject *self, PGconn *pgconn) goto exit; } - if (!(enc = clean_encoding_name(tmp))) { + if (0 > clear_encoding_name(tmp, &enc)) { goto exit; } /* Look for this encoding in Python codecs. */ - if (!(codec = conn_encoding_to_codec(enc))) { + if (0 > conn_encoding_to_codec(enc, &codec)) { goto exit; } @@ -1141,8 +1147,8 @@ conn_set_client_encoding(connectionObject *self, const char *enc) if (strcmp(self->encoding, enc) == 0) return 0; /* We must know what python codec this encoding is. */ - if (!(clean_enc = clean_encoding_name(enc))) { goto exit; } - if (!(codec = conn_encoding_to_codec(clean_enc))) { goto exit; } + if (0 > clear_encoding_name(enc, &clean_enc)) { goto exit; } + if (0 > conn_encoding_to_codec(clean_enc, &codec)) { goto exit; } Py_BEGIN_ALLOW_THREADS; pthread_mutex_lock(&self->lock); diff --git a/psycopg/psycopg.h b/psycopg/psycopg.h index 60f01b12..0eec1385 100644 --- a/psycopg/psycopg.h +++ b/psycopg/psycopg.h @@ -127,7 +127,7 @@ HIDDEN void psyco_set_error(PyObject *exc, cursorObject *curs, const char *msg, HIDDEN char *psycopg_escape_string(PyObject *conn, const char *from, Py_ssize_t len, char *to, Py_ssize_t *tolen); HIDDEN char *psycopg_escape_identifier_easy(const char *from, Py_ssize_t len); -HIDDEN char *psycopg_strdup(const char *from, Py_ssize_t len); +HIDDEN int psycopg_strdup(char **to, const char *from, Py_ssize_t len); HIDDEN int psycopg_is_text_file(PyObject *f); CPYCHECKER_STEALS_REFERENCE_TO_ARG(1) diff --git a/psycopg/utils.c b/psycopg/utils.c index 4f40cc82..9ae710df 100644 --- a/psycopg/utils.c +++ b/psycopg/utils.c @@ -113,20 +113,20 @@ psycopg_escape_identifier_easy(const char *from, Py_ssize_t len) * Allocate a new buffer on the Python heap containing the new string. * 'len' is optional: if 0 the length is calculated. * - * Return NULL and set an exception in case of error. + * Store the return in 'to' and return 0 in case of success, else return -1 + * and raise an exception. */ -char * -psycopg_strdup(const char *from, Py_ssize_t len) +CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION +int +psycopg_strdup(char **to, const char *from, Py_ssize_t len) { - char *rv; - if (!len) { len = strlen(from); } - if (!(rv = PyMem_Malloc(len + 1))) { + if (!(*to = PyMem_Malloc(len + 1))) { PyErr_NoMemory(); - return NULL; + return -1; } - strcpy(rv, from); - return rv; + strcpy(*to, from); + return 0; } /* Ensure a Python object is a bytes string. From 5bfb6cdefed3754b2a15e1e3372630691392ca47 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 1 Mar 2012 00:57:52 +0000 Subject: [PATCH 20/36] Use more compact macros to annotate functions for the static checker --- psycopg/bytes_format.c | 3 +-- psycopg/config.h | 16 ++++++++-------- psycopg/connection.h | 21 +++++++-------------- psycopg/connection_int.c | 27 +++++++++------------------ psycopg/connection_type.c | 3 +-- psycopg/cursor.h | 3 +-- psycopg/cursor_int.c | 3 +-- psycopg/cursor_type.c | 12 ++++-------- psycopg/pqpath.c | 27 +++++++++------------------ psycopg/pqpath.h | 18 ++++++------------ psycopg/psycopg.h | 9 +++------ psycopg/psycopgmodule.c | 3 +-- psycopg/utils.c | 9 +++------ 13 files changed, 54 insertions(+), 100 deletions(-) diff --git a/psycopg/bytes_format.c b/psycopg/bytes_format.c index b22422b0..68662075 100644 --- a/psycopg/bytes_format.c +++ b/psycopg/bytes_format.c @@ -86,8 +86,7 @@ /* Helpers for formatstring */ -CPYCHECKER_RETURNS_BORROWED_REF -Py_LOCAL_INLINE(PyObject *) +BORROWED Py_LOCAL_INLINE(PyObject *) getnextarg(PyObject *args, Py_ssize_t arglen, Py_ssize_t *p_argidx) { Py_ssize_t argidx = *p_argidx; diff --git a/psycopg/config.h b/psycopg/config.h index 4d3fa99b..f3094493 100644 --- a/psycopg/config.h +++ b/psycopg/config.h @@ -162,31 +162,31 @@ static double round(double num) /* decorators for the gcc cpychecker plugin */ #if defined(WITH_CPYCHECKER_RETURNS_BORROWED_REF_ATTRIBUTE) -#define CPYCHECKER_RETURNS_BORROWED_REF \ +#define BORROWED \ __attribute__((cpychecker_returns_borrowed_ref)) #else -#define CPYCHECKER_RETURNS_BORROWED_REF +#define BORROWED #endif #if defined(WITH_CPYCHECKER_STEALS_REFERENCE_TO_ARG_ATTRIBUTE) -#define CPYCHECKER_STEALS_REFERENCE_TO_ARG(n) \ +#define STEALS(n) \ __attribute__((cpychecker_steals_reference_to_arg(n))) #else -#define CPYCHECKER_STEALS_REFERENCE_TO_ARG(n) +#define STEALS(n) #endif #if defined(WITH_CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION_ATTRIBUTE) -#define CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION \ +#define RAISES_NEG \ __attribute__((cpychecker_negative_result_sets_exception)) #else -#define CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION +#define RAISES_NEG #endif #if defined(WITH_CPYCHECKER_SETS_EXCEPTION_ATTRIBUTE) -#define CPYCHECKER_SETS_EXCEPTION \ +#define RAISES \ __attribute__((cpychecker_sets_exception)) #else -#define CPYCHECKER_SETS_EXCEPTION +#define RAISES #endif #endif /* !defined(PSYCOPG_CONFIG_H) */ diff --git a/psycopg/connection.h b/psycopg/connection.h index d4a83480..79f823e3 100644 --- a/psycopg/connection.h +++ b/psycopg/connection.h @@ -131,8 +131,7 @@ typedef struct { /* C-callable functions in connection_int.c and connection_ext.c */ HIDDEN PyObject *conn_text_from_chars(connectionObject *pgconn, const char *str); HIDDEN int conn_get_standard_conforming_strings(PGconn *pgconn); -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -HIDDEN int conn_get_isolation_level(connectionObject *self); +RAISES_NEG HIDDEN int conn_get_isolation_level(connectionObject *self); HIDDEN int conn_get_protocol_version(PGconn *pgconn); HIDDEN int conn_get_server_version(PGconn *pgconn); HIDDEN PGcancel *conn_get_cancel(PGconn *pgconn); @@ -142,23 +141,17 @@ HIDDEN void conn_notifies_process(connectionObject *self); HIDDEN int conn_setup(connectionObject *self, PGconn *pgconn); HIDDEN int conn_connect(connectionObject *self, long int async); HIDDEN void conn_close(connectionObject *self); -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -HIDDEN int conn_commit(connectionObject *self); -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -HIDDEN int conn_rollback(connectionObject *self); +RAISES_NEG HIDDEN int conn_commit(connectionObject *self); +RAISES_NEG HIDDEN int conn_rollback(connectionObject *self); HIDDEN int conn_set_session(connectionObject *self, const char *isolevel, const char *readonly, const char *deferrable, int autocommit); HIDDEN int conn_set_autocommit(connectionObject *self, int value); -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -HIDDEN int conn_switch_isolation_level(connectionObject *self, int level); -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -HIDDEN int conn_set_client_encoding(connectionObject *self, const char *enc); +RAISES_NEG HIDDEN int conn_switch_isolation_level(connectionObject *self, int level); +RAISES_NEG HIDDEN int conn_set_client_encoding(connectionObject *self, const char *enc); HIDDEN int conn_poll(connectionObject *self); -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -HIDDEN int conn_tpc_begin(connectionObject *self, XidObject *xid); -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -HIDDEN int conn_tpc_command(connectionObject *self, +RAISES_NEG HIDDEN int conn_tpc_begin(connectionObject *self, XidObject *xid); +RAISES_NEG HIDDEN int conn_tpc_command(connectionObject *self, const char *cmd, XidObject *xid); HIDDEN PyObject *conn_tpc_recover(connectionObject *self); diff --git a/psycopg/connection_int.c b/psycopg/connection_int.c index 7375da3b..4d1ffb9f 100644 --- a/psycopg/connection_int.c +++ b/psycopg/connection_int.c @@ -253,8 +253,7 @@ conn_get_standard_conforming_strings(PGconn *pgconn) * Return a buffer allocated on Python heap into 'clean' and return 0 on * success, otherwise return -1 and set an exception. */ -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -static int +RAISES_NEG static int clear_encoding_name(const char *enc, char **clean) { const char *i = enc; @@ -292,8 +291,7 @@ exit: * * 'enc' should be already normalized (uppercase, no - or _). */ -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -static int +RAISES_NEG static int conn_encoding_to_codec(const char *enc, char **codec) { char *tmp; @@ -376,8 +374,7 @@ exit: } -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -int +RAISES_NEG int conn_get_isolation_level(connectionObject *self) { PGresult *pgres = NULL; @@ -954,8 +951,7 @@ conn_close(connectionObject *self) /* conn_commit - commit on a connection */ -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -int +RAISES_NEG int conn_commit(connectionObject *self) { int res; @@ -966,8 +962,7 @@ conn_commit(connectionObject *self) /* conn_rollback - rollback a connection */ -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -int +RAISES_NEG int conn_rollback(connectionObject *self) { int res; @@ -1049,8 +1044,7 @@ conn_set_autocommit(connectionObject *self, int value) /* conn_switch_isolation_level - switch isolation level on the connection */ -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -int +RAISES_NEG int conn_switch_isolation_level(connectionObject *self, int level) { PGresult *pgres = NULL; @@ -1132,8 +1126,7 @@ endlock: /* conn_set_client_encoding - switch client encoding on connection */ -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -int +RAISES_NEG int conn_set_client_encoding(connectionObject *self, const char *enc) { PGresult *pgres = NULL; @@ -1206,8 +1199,7 @@ exit: * in regular transactions, as PostgreSQL won't even know we are in a TPC * until PREPARE. */ -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -int +RAISES_NEG int conn_tpc_begin(connectionObject *self, XidObject *xid) { PGresult *pgres = NULL; @@ -1241,8 +1233,7 @@ conn_tpc_begin(connectionObject *self, XidObject *xid) * The function doesn't change the connection state as it can be used * for many commands and for recovered transactions. */ -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -int +RAISES_NEG int conn_tpc_command(connectionObject *self, const char *cmd, XidObject *xid) { PGresult *pgres = NULL; diff --git a/psycopg/connection_type.c b/psycopg/connection_type.c index d88126ba..47afbd77 100644 --- a/psycopg/connection_type.c +++ b/psycopg/connection_type.c @@ -550,8 +550,7 @@ psyco_conn_autocommit_get(connectionObject *self) return ret; } -CPYCHECKER_RETURNS_BORROWED_REF -static PyObject * +BORROWED static PyObject * _psyco_conn_autocommit_set_checks(connectionObject *self) { /* wrapper to use the EXC_IF macros. diff --git a/psycopg/cursor.h b/psycopg/cursor.h index fe16b56c..723bd170 100644 --- a/psycopg/cursor.h +++ b/psycopg/cursor.h @@ -85,8 +85,7 @@ struct cursorObject { /* C-callable functions in cursor_int.c and cursor_ext.c */ -CPYCHECKER_RETURNS_BORROWED_REF -HIDDEN PyObject *curs_get_cast(cursorObject *self, PyObject *oid); +BORROWED HIDDEN PyObject *curs_get_cast(cursorObject *self, PyObject *oid); HIDDEN void curs_reset(cursorObject *self); /* exception-raising macros */ diff --git a/psycopg/cursor_int.c b/psycopg/cursor_int.c index 8004bef9..1ac3f550 100644 --- a/psycopg/cursor_int.c +++ b/psycopg/cursor_int.c @@ -38,8 +38,7 @@ * Return a borrowed reference. */ -CPYCHECKER_RETURNS_BORROWED_REF -PyObject * +BORROWED PyObject * curs_get_cast(cursorObject *self, PyObject *oid) { PyObject *cast; diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index b60263b9..3ce02a1d 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -75,8 +75,7 @@ psyco_curs_close(cursorObject *self, PyObject *args) /* mogrify a query string and build argument array or dict */ -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -static int +RAISES_NEG static int _mogrify(PyObject *var, PyObject *fmt, cursorObject *curs, PyObject **new) { PyObject *key, *value, *n; @@ -360,8 +359,7 @@ _psyco_curs_merge_query_args(cursorObject *self, #define psyco_curs_execute_doc \ "execute(query, vars=None) -- Execute query with bound vars." -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -static int +RAISES_NEG static int _psyco_curs_execute(cursorObject *self, PyObject *operation, PyObject *vars, long int async) { @@ -645,8 +643,7 @@ psyco_curs_cast(cursorObject *self, PyObject *args) "default) or using the sequence factory previously set in the\n" \ "`row_factory` attribute. Return `!None` when no more data is available.\n" -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -static int +RAISES_NEG static int _psyco_curs_prefetch(cursorObject *self) { int i = 0; @@ -663,8 +660,7 @@ _psyco_curs_prefetch(cursorObject *self) return i; } -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -static int +RAISES_NEG static int _psyco_curs_buildrow_fill(cursorObject *self, PyObject *res, int row, int n, int istuple) { diff --git a/psycopg/pqpath.c b/psycopg/pqpath.c index e8506990..899c3315 100644 --- a/psycopg/pqpath.c +++ b/psycopg/pqpath.c @@ -64,8 +64,7 @@ strip_severity(const char *msg) code. A list of error codes can be found at: http://www.postgresql.org/docs/current/static/errcodes-appendix.html */ -CPYCHECKER_RETURNS_BORROWED_REF -static PyObject * +BORROWED static PyObject * exception_from_sqlstate(const char *sqlstate) { switch (sqlstate[0]) { @@ -152,8 +151,7 @@ exception_from_sqlstate(const char *sqlstate) This function should be called while holding the GIL. */ -CPYCHECKER_SETS_EXCEPTION -static void +RAISES static void pq_raise(connectionObject *conn, cursorObject *curs, PGresult *pgres) { PyObject *exc = NULL; @@ -253,8 +251,7 @@ pq_clear_critical(connectionObject *conn) /* return -1 if the exception is set (i.e. if conn->critical is set), * else 0 */ -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -static int +RAISES_NEG static int pq_resolve_critical(connectionObject *conn, int close) { Dprintf("pq_resolve_critical: resolving %s", conn->critical); @@ -389,8 +386,7 @@ cleanup: This function should be called while holding the global interpreter lock. */ -CPYCHECKER_SETS_EXCEPTION -void +RAISES void pq_complete_error(connectionObject *conn, PGresult **pgres, char **error) { Dprintf("pq_complete_error: pgconn = %p, pgres = %p, error = %s", @@ -483,8 +479,7 @@ pq_commit(connectionObject *conn) return retvalue; } -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -int +RAISES_NEG int pq_abort_locked(connectionObject *conn, PGresult **pgres, char **error, PyThreadState **tstate) { @@ -511,8 +506,7 @@ pq_abort_locked(connectionObject *conn, PGresult **pgres, char **error, This function should be called while holding the global interpreter lock. */ -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -int +RAISES_NEG int pq_abort(connectionObject *conn) { int retvalue = -1; @@ -554,8 +548,7 @@ pq_abort(connectionObject *conn) connection without holding the global interpreter lock. */ -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -int +RAISES_NEG int pq_reset_locked(connectionObject *conn, PGresult **pgres, char **error, PyThreadState **tstate) { @@ -847,8 +840,7 @@ pq_flush(connectionObject *conn) this fucntion locks the connection object this function call Py_*_ALLOW_THREADS macros */ -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -int +RAISES_NEG int pq_execute(cursorObject *curs, const char *query, int async) { PGresult *pgres = NULL; @@ -1028,8 +1020,7 @@ pq_get_last_result(connectionObject *conn) 1 - result from backend (possibly data is ready) */ -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -static int +RAISES_NEG static int _pq_fetch_tuples(cursorObject *curs) { int i, *dsize = NULL; diff --git a/psycopg/pqpath.h b/psycopg/pqpath.h index ef0e3252..2fff85ff 100644 --- a/psycopg/pqpath.h +++ b/psycopg/pqpath.h @@ -35,23 +35,18 @@ /* exported functions */ HIDDEN PGresult *pq_get_last_result(connectionObject *conn); -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -HIDDEN int pq_fetch(cursorObject *curs); -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -HIDDEN int pq_execute(cursorObject *curs, const char *query, int async); +RAISES_NEG HIDDEN int pq_fetch(cursorObject *curs); +RAISES_NEG HIDDEN int pq_execute(cursorObject *curs, const char *query, int async); HIDDEN int pq_send_query(connectionObject *conn, const char *query); HIDDEN int pq_begin_locked(connectionObject *conn, PGresult **pgres, char **error, PyThreadState **tstate); HIDDEN int pq_commit(connectionObject *conn); -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -HIDDEN int pq_abort_locked(connectionObject *conn, PGresult **pgres, +RAISES_NEG HIDDEN int pq_abort_locked(connectionObject *conn, PGresult **pgres, char **error, PyThreadState **tstate); -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -HIDDEN int pq_abort(connectionObject *conn); +RAISES_NEG HIDDEN int pq_abort(connectionObject *conn); HIDDEN int pq_reset_locked(connectionObject *conn, PGresult **pgres, char **error, PyThreadState **tstate); -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -HIDDEN int pq_reset(connectionObject *conn); +RAISES_NEG HIDDEN int pq_reset(connectionObject *conn); HIDDEN char *pq_get_guc_locked(connectionObject *conn, const char *param, PGresult **pgres, char **error, PyThreadState **tstate); @@ -74,8 +69,7 @@ HIDDEN int pq_execute_command_locked(connectionObject *conn, const char *query, PGresult **pgres, char **error, PyThreadState **tstate); -CPYCHECKER_SETS_EXCEPTION -HIDDEN void pq_complete_error(connectionObject *conn, PGresult **pgres, +RAISES HIDDEN void pq_complete_error(connectionObject *conn, PGresult **pgres, char **error); #endif /* !defined(PSYCOPG_PQPATH_H) */ diff --git a/psycopg/psycopg.h b/psycopg/psycopg.h index 0eec1385..2e86dca8 100644 --- a/psycopg/psycopg.h +++ b/psycopg/psycopg.h @@ -120,8 +120,7 @@ HIDDEN PyObject *psyco_GetDecimalType(void); typedef struct cursorObject cursorObject; /* some utility functions */ -CPYCHECKER_SETS_EXCEPTION -HIDDEN void psyco_set_error(PyObject *exc, cursorObject *curs, const char *msg, +RAISES 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, @@ -130,11 +129,9 @@ HIDDEN char *psycopg_escape_identifier_easy(const char *from, Py_ssize_t len); HIDDEN int psycopg_strdup(char **to, const char *from, Py_ssize_t len); HIDDEN int psycopg_is_text_file(PyObject *f); -CPYCHECKER_STEALS_REFERENCE_TO_ARG(1) -HIDDEN PyObject * psycopg_ensure_bytes(PyObject *obj); +STEALS(1) HIDDEN PyObject * psycopg_ensure_bytes(PyObject *obj); -CPYCHECKER_STEALS_REFERENCE_TO_ARG(1) -HIDDEN PyObject * psycopg_ensure_text(PyObject *obj); +STEALS(1) HIDDEN PyObject * psycopg_ensure_text(PyObject *obj); /* Exceptions docstrings */ #define Error_doc \ diff --git a/psycopg/psycopgmodule.c b/psycopg/psycopgmodule.c index 7d77ee17..497c42ee 100644 --- a/psycopg/psycopgmodule.c +++ b/psycopg/psycopgmodule.c @@ -600,8 +600,7 @@ psyco_errors_set(PyObject *type) Create a new error of the given type with extra attributes. */ -CPYCHECKER_SETS_EXCEPTION -void +RAISES void psyco_set_error(PyObject *exc, cursorObject *curs, const char *msg, const char *pgerror, const char *pgcode) { diff --git a/psycopg/utils.c b/psycopg/utils.c index 9ae710df..57586c57 100644 --- a/psycopg/utils.c +++ b/psycopg/utils.c @@ -116,8 +116,7 @@ psycopg_escape_identifier_easy(const char *from, Py_ssize_t len) * Store the return in 'to' and return 0 in case of success, else return -1 * and raise an exception. */ -CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION -int +RAISES_NEG int psycopg_strdup(char **to, const char *from, Py_ssize_t len) { if (!len) { len = strlen(from); } @@ -139,8 +138,7 @@ psycopg_strdup(char **to, const char *from, Py_ssize_t len) * * It is safe to call the function on NULL. */ -CPYCHECKER_STEALS_REFERENCE_TO_ARG(1) -PyObject * +STEALS(1) PyObject * psycopg_ensure_bytes(PyObject *obj) { PyObject *rv = NULL; @@ -170,8 +168,7 @@ psycopg_ensure_bytes(PyObject *obj) * The function is ref neutral: steals a ref from obj and adds one to the * return value. It is safe to call it on NULL. */ -CPYCHECKER_STEALS_REFERENCE_TO_ARG(1) -PyObject * +STEALS(1) PyObject * psycopg_ensure_text(PyObject *obj) { #if PY_MAJOR_VERSION < 3 From e1266d52cd9fb48585fb7eb152389b314c1610a8 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 1 Mar 2012 02:45:48 +0000 Subject: [PATCH 21/36] More functions annotated for static analysis Also more return values checked for values < 0 for errors, instead of checking == 0 and leaving the positive side unchecked --- psycopg/connection.h | 4 ++-- psycopg/connection_int.c | 14 +++++++------- psycopg/connection_type.c | 2 +- psycopg/cursor_type.c | 8 ++++---- psycopg/pqpath.c | 13 +++++-------- psycopg/pqpath.h | 2 +- 6 files changed, 20 insertions(+), 23 deletions(-) diff --git a/psycopg/connection.h b/psycopg/connection.h index 79f823e3..9647ffd2 100644 --- a/psycopg/connection.h +++ b/psycopg/connection.h @@ -138,12 +138,12 @@ HIDDEN PGcancel *conn_get_cancel(PGconn *pgconn); HIDDEN void conn_notice_process(connectionObject *self); HIDDEN void conn_notice_clean(connectionObject *self); HIDDEN void conn_notifies_process(connectionObject *self); -HIDDEN int conn_setup(connectionObject *self, PGconn *pgconn); +RAISES_NEG HIDDEN int conn_setup(connectionObject *self, PGconn *pgconn); HIDDEN int conn_connect(connectionObject *self, long int async); HIDDEN void conn_close(connectionObject *self); RAISES_NEG HIDDEN int conn_commit(connectionObject *self); RAISES_NEG HIDDEN int conn_rollback(connectionObject *self); -HIDDEN int conn_set_session(connectionObject *self, const char *isolevel, +RAISES_NEG HIDDEN int conn_set_session(connectionObject *self, const char *isolevel, const char *readonly, const char *deferrable, int autocommit); HIDDEN int conn_set_autocommit(connectionObject *self, int value); diff --git a/psycopg/connection_int.c b/psycopg/connection_int.c index 4d1ffb9f..abc61e88 100644 --- a/psycopg/connection_int.c +++ b/psycopg/connection_int.c @@ -332,7 +332,7 @@ exit: * * Return 0 on success, else nonzero. */ -static int +RAISES_NEG static int conn_read_encoding(connectionObject *self, PGconn *pgconn) { char *enc = NULL, *codec = NULL; @@ -468,7 +468,7 @@ conn_is_datestyle_ok(PGconn *pgconn) /* conn_setup - setup and read basic information about the connection */ -int +RAISES_NEG int conn_setup(connectionObject *self, PGconn *pgconn) { PGresult *pgres = NULL; @@ -482,7 +482,7 @@ conn_setup(connectionObject *self, PGconn *pgconn) return -1; } - if (conn_read_encoding(self, pgconn)) { + if (0 > conn_read_encoding(self, pgconn)) { return -1; } @@ -496,7 +496,7 @@ conn_setup(connectionObject *self, PGconn *pgconn) pthread_mutex_lock(&self->lock); Py_BLOCK_THREADS; - if (psyco_green() && (pq_set_non_blocking(self, 1, 1) != 0)) { + if (psyco_green() && (0 > pq_set_non_blocking(self, 1))) { return -1; } @@ -774,7 +774,7 @@ _conn_poll_setup_async(connectionObject *self) switch (self->status) { case CONN_STATUS_CONNECTING: /* Set the connection to nonblocking now. */ - if (pq_set_non_blocking(self, 1, 1) != 0) { + if (pq_set_non_blocking(self, 1) != 0) { break; } @@ -785,7 +785,7 @@ _conn_poll_setup_async(connectionObject *self) PyErr_SetString(InterfaceError, "only protocol 3 supported"); break; } - if (conn_read_encoding(self, self->pgconn)) { + if (0 > conn_read_encoding(self, self->pgconn)) { break; } self->cancel = conn_get_cancel(self->pgconn); @@ -971,7 +971,7 @@ conn_rollback(connectionObject *self) return res; } -int +RAISES_NEG int conn_set_session(connectionObject *self, const char *isolevel, const char *readonly, const char *deferrable, int autocommit) diff --git a/psycopg/connection_type.c b/psycopg/connection_type.c index 47afbd77..9cdd640f 100644 --- a/psycopg/connection_type.c +++ b/psycopg/connection_type.c @@ -528,7 +528,7 @@ psyco_conn_set_session(connectionObject *self, PyObject *args, PyObject *kwargs) if (-1 == c_autocommit) { return NULL; } } - if (0 != conn_set_session(self, + if (0 > conn_set_session(self, c_isolevel, c_readonly, c_deferrable, c_autocommit)) { return NULL; } diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index 3ce02a1d..030147d6 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -726,7 +726,7 @@ _psyco_curs_buildrow(cursorObject *self, int row) } if (!t) { goto exit; } - if (0 == _psyco_curs_buildrow_fill(self, t, row, n, istuple)) { + if (0 <= _psyco_curs_buildrow_fill(self, t, row, n, istuple)) { rv = t; t = NULL; } @@ -1347,7 +1347,7 @@ psyco_curs_copy_from(cursorObject *self, PyObject *args, PyObject *kwargs) Py_INCREF(file); self->copyfile = file; - if (pq_execute(self, query, 0) == 1) { + if (pq_execute(self, query, 0) >= 0) { res = Py_None; Py_INCREF(Py_None); } @@ -1443,7 +1443,7 @@ psyco_curs_copy_to(cursorObject *self, PyObject *args, PyObject *kwargs) Py_INCREF(file); self->copyfile = file; - if (pq_execute(self, query, 0) == 1) { + if (pq_execute(self, query, 0) >= 0) { res = Py_None; Py_INCREF(Py_None); } @@ -1517,7 +1517,7 @@ psyco_curs_copy_expert(cursorObject *self, PyObject *args, PyObject *kwargs) self->copyfile = file; /* At this point, the SQL statement must be str, not unicode */ - if (pq_execute(self, Bytes_AS_STRING(sql), 0) == 1) { + if (pq_execute(self, Bytes_AS_STRING(sql), 0) >= 0) { res = Py_None; Py_INCREF(res); } diff --git a/psycopg/pqpath.c b/psycopg/pqpath.c index 899c3315..e8b8dc63 100644 --- a/psycopg/pqpath.c +++ b/psycopg/pqpath.c @@ -304,19 +304,16 @@ pq_clear_async(connectionObject *conn) Accepted arg values are 1 (nonblocking) and 0 (blocking). - Return 0 if everything ok, else nonzero. - - In case of error, if pyerr is nonzero, set a Python exception. + Return 0 if everything ok, else < 0 and set an exception. */ -int -pq_set_non_blocking(connectionObject *conn, int arg, int pyerr) +RAISES_NEG int +pq_set_non_blocking(connectionObject *conn, int arg) { int ret = PQsetnonblocking(conn->pgconn, arg); if (0 != ret) { Dprintf("PQsetnonblocking(%d) FAILED", arg); - if (pyerr) { - PyErr_SetString(OperationalError, "PQsetnonblocking() failed"); - } + PyErr_SetString(OperationalError, "PQsetnonblocking() failed"); + ret = -1; } return ret; } diff --git a/psycopg/pqpath.h b/psycopg/pqpath.h index 2fff85ff..a8f39c18 100644 --- a/psycopg/pqpath.h +++ b/psycopg/pqpath.h @@ -61,7 +61,7 @@ HIDDEN int pq_is_busy(connectionObject *conn); HIDDEN int pq_is_busy_locked(connectionObject *conn); HIDDEN int pq_flush(connectionObject *conn); HIDDEN void pq_clear_async(connectionObject *conn); -HIDDEN int pq_set_non_blocking(connectionObject *conn, int arg, int pyerr); +RAISES_NEG HIDDEN int pq_set_non_blocking(connectionObject *conn, int arg); HIDDEN void pq_set_critical(connectionObject *conn, const char *msg); From 4d15b973b0eb59558f4f1bcc7183e0d4cbc9fbe1 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 1 Mar 2012 02:47:30 +0000 Subject: [PATCH 22/36] Attempt to enforce signature for the "O&" converter functions It seems causing a traceback in the static checker. Enforcing it simplifies the code, but doesn't help the checker. --- psycopg/cursor_type.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index 030147d6..dbcc4788 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -1261,8 +1261,8 @@ exit: #define psyco_curs_copy_from_doc \ "copy_from(file, table, sep='\\t', null='\\\\N', size=8192, columns=None) -- Copy table from file." -static int -_psyco_curs_has_read_check(PyObject* o, void* var) +STEALS(1) static int +_psyco_curs_has_read_check(PyObject *o, PyObject **var) { if (PyObject_HasAttrString(o, "readline") && PyObject_HasAttrString(o, "read")) { @@ -1272,7 +1272,7 @@ _psyco_curs_has_read_check(PyObject* o, void* var) * which could invoke the garbage collector. We thus need an * INCREF/DECREF pair if we store this pointer in a GC object, such as * a cursorObject */ - *((PyObject**)var) = o; + *var = o; return 1; } else { @@ -1368,11 +1368,11 @@ exit: #define psyco_curs_copy_to_doc \ "copy_to(file, table, sep='\\t', null='\\\\N', columns=None) -- Copy table to file." -static int -_psyco_curs_has_write_check(PyObject* o, void* var) +STEALS(1) static int +_psyco_curs_has_write_check(PyObject *o, PyObject **var) { if (PyObject_HasAttrString(o, "write")) { - *((PyObject**)var) = o; + *var = o; return 1; } else { From d93732558da4679c802292284cd8c2324bebadda Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sun, 4 Mar 2012 04:17:03 +0000 Subject: [PATCH 23/36] Raise an exception in case of failed localtime_r call --- psycopg/adapter_datetime.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/psycopg/adapter_datetime.c b/psycopg/adapter_datetime.c index 5850be8f..bf31cfab 100644 --- a/psycopg/adapter_datetime.c +++ b/psycopg/adapter_datetime.c @@ -427,6 +427,10 @@ psyco_DateFromTicks(PyObject *self, PyObject *args) Py_DECREF(args); } } + else { + PyErr_SetString(InterfaceError, "failed localtime call"); + } + return res; } @@ -451,6 +455,10 @@ psyco_TimeFromTicks(PyObject *self, PyObject *args) Py_DECREF(args); } } + else { + PyErr_SetString(InterfaceError, "failed localtime call"); + } + return res; } @@ -473,6 +481,9 @@ psyco_TimestampFromTicks(PyObject *self, PyObject *args) tm.tm_hour, tm.tm_min, (double)tm.tm_sec + ticks, pyPsycopgTzLOCAL); } + else { + PyErr_SetString(InterfaceError, "failed localtime call"); + } return res; } From 31812c01e6b4ba5476902a19430254ef8a6ab445 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sun, 4 Mar 2012 04:38:44 +0000 Subject: [PATCH 24/36] Further modeling of exception raising --- psycopg/adapter_qstring.c | 2 +- psycopg/lobject.h | 22 +++++++++++----------- psycopg/lobject_int.c | 22 +++++++++++----------- psycopg/psycopgmodule.c | 22 ++++++++++------------ psycopg/typecast.c | 4 ++-- psycopg/typecast.h | 4 ++-- psycopg/typecast_array.c | 19 +++++++++---------- psycopg/typecast_datetime.c | 2 +- psycopg/xid_type.c | 2 +- 9 files changed, 48 insertions(+), 51 deletions(-) diff --git a/psycopg/adapter_qstring.c b/psycopg/adapter_qstring.c index a9d4ec6e..4240d382 100644 --- a/psycopg/adapter_qstring.c +++ b/psycopg/adapter_qstring.c @@ -35,7 +35,7 @@ /* qstring_quote - do the quote process on plain and unicode strings */ -static PyObject * +BORROWED static PyObject * qstring_quote(qstringObject *self) { PyObject *str; diff --git a/psycopg/lobject.h b/psycopg/lobject.h index f52d85cf..6587198c 100644 --- a/psycopg/lobject.h +++ b/psycopg/lobject.h @@ -51,19 +51,19 @@ typedef struct { /* functions exported from lobject_int.c */ -HIDDEN int lobject_open(lobjectObject *self, connectionObject *conn, - Oid oid, const char *smode, Oid new_oid, - const char *new_file); -HIDDEN int lobject_unlink(lobjectObject *self); -HIDDEN int lobject_export(lobjectObject *self, const char *filename); +RAISES_NEG HIDDEN int lobject_open(lobjectObject *self, connectionObject *conn, + Oid oid, const char *smode, Oid new_oid, + const char *new_file); +RAISES_NEG HIDDEN int lobject_unlink(lobjectObject *self); +RAISES_NEG HIDDEN int lobject_export(lobjectObject *self, const char *filename); -HIDDEN Py_ssize_t lobject_read(lobjectObject *self, char *buf, size_t len); -HIDDEN Py_ssize_t lobject_write(lobjectObject *self, const char *buf, +RAISES_NEG HIDDEN Py_ssize_t lobject_read(lobjectObject *self, char *buf, size_t len); +RAISES_NEG HIDDEN Py_ssize_t lobject_write(lobjectObject *self, const char *buf, size_t len); -HIDDEN int lobject_seek(lobjectObject *self, int pos, int whence); -HIDDEN int lobject_tell(lobjectObject *self); -HIDDEN int lobject_truncate(lobjectObject *self, size_t len); -HIDDEN int lobject_close(lobjectObject *self); +RAISES_NEG HIDDEN int lobject_seek(lobjectObject *self, int pos, int whence); +RAISES_NEG HIDDEN int lobject_tell(lobjectObject *self); +RAISES_NEG HIDDEN int lobject_truncate(lobjectObject *self, size_t len); +RAISES_NEG HIDDEN int lobject_close(lobjectObject *self); #define lobject_is_closed(self) \ ((self)->fd < 0 || !(self)->conn || (self)->conn->closed) diff --git a/psycopg/lobject_int.c b/psycopg/lobject_int.c index f180c8c2..5f23ca25 100644 --- a/psycopg/lobject_int.c +++ b/psycopg/lobject_int.c @@ -50,7 +50,7 @@ collect_error(connectionObject *conn, char **error) * * Valid mode are [r|w|rw|n][t|b] */ -static int +RAISES_NEG static int _lobject_parse_mode(const char *mode) { int rv = 0; @@ -147,7 +147,7 @@ _lobject_unparse_mode(int mode) /* lobject_open - create a new/open an existing lo */ -int +RAISES_NEG int lobject_open(lobjectObject *self, connectionObject *conn, Oid oid, const char *smode, Oid new_oid, const char *new_file) { @@ -237,7 +237,7 @@ lobject_open(lobjectObject *self, connectionObject *conn, /* lobject_close - close an existing lo */ -static int +RAISES_NEG static int lobject_close_locked(lobjectObject *self, char **error) { int retvalue; @@ -270,7 +270,7 @@ lobject_close_locked(lobjectObject *self, char **error) return retvalue; } -int +RAISES_NEG int lobject_close(lobjectObject *self) { PGresult *pgres = NULL; @@ -292,7 +292,7 @@ lobject_close(lobjectObject *self) /* lobject_unlink - remove an lo from database */ -int +RAISES_NEG int lobject_unlink(lobjectObject *self) { PGresult *pgres = NULL; @@ -326,7 +326,7 @@ lobject_unlink(lobjectObject *self) /* lobject_write - write bytes to a lo */ -Py_ssize_t +RAISES_NEG Py_ssize_t lobject_write(lobjectObject *self, const char *buf, size_t len) { Py_ssize_t written; @@ -353,7 +353,7 @@ lobject_write(lobjectObject *self, const char *buf, size_t len) /* lobject_read - read bytes from a lo */ -Py_ssize_t +RAISES_NEG Py_ssize_t lobject_read(lobjectObject *self, char *buf, size_t len) { Py_ssize_t n_read; @@ -377,7 +377,7 @@ lobject_read(lobjectObject *self, char *buf, size_t len) /* lobject_seek - move the current position in the lo */ -int +RAISES_NEG int lobject_seek(lobjectObject *self, int pos, int whence) { PGresult *pgres = NULL; @@ -405,7 +405,7 @@ lobject_seek(lobjectObject *self, int pos, int whence) /* lobject_tell - tell the current position in the lo */ -int +RAISES_NEG int lobject_tell(lobjectObject *self) { PGresult *pgres = NULL; @@ -432,7 +432,7 @@ lobject_tell(lobjectObject *self) /* lobject_export - export to a local file */ -int +RAISES_NEG int lobject_export(lobjectObject *self, const char *filename) { PGresult *pgres = NULL; @@ -461,7 +461,7 @@ lobject_export(lobjectObject *self, const char *filename) #if PG_VERSION_HEX >= 0x080300 -int +RAISES_NEG int lobject_truncate(lobjectObject *self, size_t len) { int retvalue; diff --git a/psycopg/psycopgmodule.c b/psycopg/psycopgmodule.c index 497c42ee..3328c609 100644 --- a/psycopg/psycopgmodule.c +++ b/psycopg/psycopgmodule.c @@ -143,14 +143,6 @@ psyco_connect(PyObject *self, PyObject *args, PyObject *keywds) " * `name`: Name for the new type\n" \ " * `baseobj`: Adapter to perform type conversion of a single array item." -static void -_psyco_register_type_set(PyObject **dict, PyObject *type) -{ - if (*dict == NULL) - *dict = PyDict_New(); - typecast_add(type, *dict, 0); -} - static PyObject * psyco_register_type(PyObject *self, PyObject *args) { @@ -162,10 +154,16 @@ psyco_register_type(PyObject *self, PyObject *args) if (obj != NULL && obj != Py_None) { if (PyObject_TypeCheck(obj, &cursorType)) { - _psyco_register_type_set(&(((cursorObject*)obj)->string_types), type); + PyObject **dict = &(((cursorObject*)obj)->string_types); + if (*dict == NULL) { + if (!(*dict = PyDict_New())) { return NULL; } + } + if (0 > typecast_add(type, *dict, 0)) { return NULL; } } else if (PyObject_TypeCheck(obj, &connectionType)) { - typecast_add(type, ((connectionObject*)obj)->string_types, 0); + if (0 > typecast_add(type, ((connectionObject*)obj)->string_types, 0)) { + return NULL; + } } else { PyErr_SetString(PyExc_TypeError, @@ -174,7 +172,7 @@ psyco_register_type(PyObject *self, PyObject *args) } } else { - typecast_add(type, NULL, 0); + if (0 > typecast_add(type, NULL, 0)) { return NULL; } } Py_INCREF(Py_None); @@ -1001,7 +999,7 @@ INIT_MODULE(_psycopg)(void) } #endif /* initialize default set of typecasters */ - typecast_init(dict); + if (0 != typecast_init(dict)) { goto exit; } /* initialize microprotocols layer */ microprotocols_init(dict); diff --git a/psycopg/typecast.c b/psycopg/typecast.c index 8ede351c..e4abd8f7 100644 --- a/psycopg/typecast.c +++ b/psycopg/typecast.c @@ -250,7 +250,7 @@ PyObject *psyco_default_binary_cast; /* typecast_init - initialize the dictionary and create default types */ -int +RAISES_NEG int typecast_init(PyObject *dict) { int i; @@ -316,7 +316,7 @@ typecast_init(PyObject *dict) } /* typecast_add - add a type object to the dictionary */ -int +RAISES_NEG int typecast_add(PyObject *obj, PyObject *dict, int binary) { PyObject *val; diff --git a/psycopg/typecast.h b/psycopg/typecast.h index 20def306..04976f31 100644 --- a/psycopg/typecast.h +++ b/psycopg/typecast.h @@ -71,8 +71,8 @@ extern HIDDEN PyObject *psyco_default_binary_cast; /** exported functions **/ /* used by module.c to init the type system and register types */ -HIDDEN int typecast_init(PyObject *dict); -HIDDEN int typecast_add(PyObject *obj, PyObject *dict, int binary); +RAISES_NEG HIDDEN int typecast_init(PyObject *dict); +RAISES_NEG HIDDEN int typecast_add(PyObject *obj, PyObject *dict, int binary); /* the C callable typecastObject creator function */ HIDDEN PyObject *typecast_from_c(typecastObject_initlist *type, PyObject *d); diff --git a/psycopg/typecast_array.c b/psycopg/typecast_array.c index 75b84b50..6d825f42 100644 --- a/psycopg/typecast_array.c +++ b/psycopg/typecast_array.c @@ -166,7 +166,7 @@ typecast_array_tokenize(const char *str, Py_ssize_t strlength, return res; } -static int +RAISES_NEG static int typecast_array_scan(const char *str, Py_ssize_t strlength, PyObject *curs, PyObject *base, PyObject *array) { @@ -199,7 +199,7 @@ typecast_array_scan(const char *str, Py_ssize_t strlength, /* before anything else we free the memory */ if (state == ASCAN_QUOTED) PyMem_Free(token); - if (obj == NULL) return 0; + if (obj == NULL) return -1; PyList_Append(array, obj); Py_DECREF(obj); @@ -207,25 +207,25 @@ typecast_array_scan(const char *str, Py_ssize_t strlength, else if (state == ASCAN_BEGIN) { PyObject *sub = PyList_New(0); - if (sub == NULL) return 0; + if (sub == NULL) return -1; PyList_Append(array, sub); Py_DECREF(sub); if (stack_index == MAX_DIMENSIONS) - return 0; + return -1; stack[stack_index++] = array; array = sub; } else if (state == ASCAN_ERROR) { - return 0; + return -1; } else if (state == ASCAN_END) { if (--stack_index < 0) - return 0; + return -1; array = stack[stack_index]; } @@ -233,7 +233,7 @@ typecast_array_scan(const char *str, Py_ssize_t strlength, break; } - return 1; + return 0; } @@ -263,9 +263,8 @@ typecast_GENERIC_ARRAY_cast(const char *str, Py_ssize_t len, PyObject *curs) obj = PyList_New(0); /* scan the array skipping the first level of {} */ - if (typecast_array_scan(&str[1], len-2, curs, base, obj) == 0) { - Py_DECREF(obj); - obj = NULL; + if (typecast_array_scan(&str[1], len-2, curs, base, obj) < 0) { + Py_CLEAR(obj); } return obj; diff --git a/psycopg/typecast_datetime.c b/psycopg/typecast_datetime.c index 29149c05..7c1cfa45 100644 --- a/psycopg/typecast_datetime.c +++ b/psycopg/typecast_datetime.c @@ -26,7 +26,7 @@ #include #include "datetime.h" -static int +RAISES_NEG static int psyco_typecast_datetime_init(void) { Dprintf("psyco_typecast_datetime_init: datetime init"); diff --git a/psycopg/xid_type.c b/psycopg/xid_type.c index 4de46b44..bc5e3ca5 100644 --- a/psycopg/xid_type.c +++ b/psycopg/xid_type.c @@ -486,7 +486,7 @@ exit: * * Return a borrowed reference. */ -static PyObject * +BORROWED static PyObject * _xid_get_parse_regex(void) { static PyObject *rv; From 18085201c852e636198d889269a42e7ab1844e80 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sun, 4 Mar 2012 04:39:41 +0000 Subject: [PATCH 25/36] Guard from NULL dereference if Xid allocation fails --- psycopg/xid_type.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/psycopg/xid_type.c b/psycopg/xid_type.c index bc5e3ca5..b28543ca 100644 --- a/psycopg/xid_type.c +++ b/psycopg/xid_type.c @@ -78,7 +78,9 @@ static PyMemberDef xid_members[] = { static PyObject * xid_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) { - XidObject *self = (XidObject *)type->tp_alloc(type, 0); + XidObject *self; + + if (!(self = (XidObject *)type->tp_alloc(type, 0))) { return NULL; } Py_INCREF(Py_None); self->format_id = Py_None; From dc4c3d3143787e9001f8a543ef6f8f588dc5f8a4 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sun, 4 Mar 2012 04:40:44 +0000 Subject: [PATCH 26/36] Guard from failed keys creation during adaptation --- psycopg/microprotocols.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/psycopg/microprotocols.c b/psycopg/microprotocols.c index 68b5f6c4..23f12794 100644 --- a/psycopg/microprotocols.c +++ b/psycopg/microprotocols.c @@ -78,9 +78,9 @@ exit: /* Check if one of `obj` superclasses has an adapter for `proto`. * - * If it does, return a *borrowed reference* to the adapter, else NULL. + * If it does, return a *borrowed reference* to the adapter, else to None. */ -static PyObject * +BORROWED static PyObject * _get_superclass_adapter(PyObject *obj, PyObject *proto) { PyTypeObject *type; @@ -95,14 +95,14 @@ _get_superclass_adapter(PyObject *obj, PyObject *proto) #endif type->tp_mro)) { /* has no mro */ - return NULL; + return Py_None; } /* Walk the mro from the most specific subclass. */ mro = type->tp_mro; for (i = 1, ii = PyTuple_GET_SIZE(mro); i < ii; ++i) { st = PyTuple_GET_ITEM(mro, i); - key = PyTuple_Pack(2, st, proto); + if (!(key = PyTuple_Pack(2, st, proto))) { return NULL; } adapter = PyDict_GetItem(psyco_adapters, key); Py_DECREF(key); @@ -125,7 +125,7 @@ _get_superclass_adapter(PyObject *obj, PyObject *proto) return adapter; } } - return NULL; + return Py_None; } @@ -145,7 +145,7 @@ microprotocols_adapt(PyObject *obj, PyObject *proto, PyObject *alt) Py_TYPE(obj)->tp_name); /* look for an adapter in the registry */ - key = PyTuple_Pack(2, Py_TYPE(obj), proto); + if (!(key = PyTuple_Pack(2, Py_TYPE(obj), proto))) { return NULL; } adapter = PyDict_GetItem(psyco_adapters, key); Py_DECREF(key); if (adapter) { @@ -154,7 +154,10 @@ microprotocols_adapt(PyObject *obj, PyObject *proto, PyObject *alt) } /* Check if a superclass can be adapted and use the same adapter. */ - if (NULL != (adapter = _get_superclass_adapter(obj, proto))) { + if (!(adapter = _get_superclass_adapter(obj, proto))) { + return NULL; + } + if (Py_None != adapter) { adapted = PyObject_CallFunctionObjArgs(adapter, obj, NULL); return adapted; } From be35df38184fe2446f3ed23a286aa207359adeb9 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sun, 4 Mar 2012 04:41:36 +0000 Subject: [PATCH 27/36] Fixed typecasters refcount --- psycopg/typecast.c | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/psycopg/typecast.c b/psycopg/typecast.c index e4abd8f7..9c36b02b 100644 --- a/psycopg/typecast.c +++ b/psycopg/typecast.c @@ -254,30 +254,24 @@ RAISES_NEG int typecast_init(PyObject *dict) { int i; + int rv = -1; + typecastObject *t = NULL; /* create type dictionary and put it in module namespace */ - psyco_types = PyDict_New(); - psyco_binary_types = PyDict_New(); - - if (!psyco_types || !psyco_binary_types) { - Py_XDECREF(psyco_types); - Py_XDECREF(psyco_binary_types); - return -1; - } - + if (!(psyco_types = PyDict_New())) { goto exit; } PyDict_SetItemString(dict, "string_types", psyco_types); + + if (!(psyco_binary_types = PyDict_New())) { goto exit; } PyDict_SetItemString(dict, "binary_types", psyco_binary_types); /* insert the cast types into the 'types' dictionary and register them in the module dictionary */ for (i = 0; typecast_builtins[i].name != NULL; i++) { - typecastObject *t; - Dprintf("typecast_init: initializing %s", typecast_builtins[i].name); t = (typecastObject *)typecast_from_c(&(typecast_builtins[i]), dict); - if (t == NULL) return -1; - if (typecast_add((PyObject *)t, NULL, 0) != 0) return -1; + if (t == NULL) { goto exit; } + if (typecast_add((PyObject *)t, NULL, 0) < 0) { goto exit; } PyDict_SetItem(dict, t->name, (PyObject *)t); @@ -285,6 +279,8 @@ typecast_init(PyObject *dict) if (typecast_builtins[i].values == typecast_BINARY_types) { psyco_default_binary_cast = (PyObject *)t; } + Py_DECREF((PyObject *)t); + t = NULL; } /* create and save a default cast object (but does not register it) */ @@ -294,25 +290,31 @@ typecast_init(PyObject *dict) #ifdef HAVE_MXDATETIME if (0 == psyco_typecast_mxdatetime_init()) { for (i = 0; typecast_mxdatetime[i].name != NULL; i++) { - typecastObject *t; Dprintf("typecast_init: initializing %s", typecast_mxdatetime[i].name); t = (typecastObject *)typecast_from_c(&(typecast_mxdatetime[i]), dict); - if (t == NULL) return -1; + if (t == NULL) { goto exit; } PyDict_SetItem(dict, t->name, (PyObject *)t); + Py_DECREF((PyObject *)t); + t = NULL; } } #endif - if (psyco_typecast_datetime_init()) { return -1; } + if (0 > psyco_typecast_datetime_init()) { goto exit; } for (i = 0; typecast_pydatetime[i].name != NULL; i++) { - typecastObject *t; Dprintf("typecast_init: initializing %s", typecast_pydatetime[i].name); t = (typecastObject *)typecast_from_c(&(typecast_pydatetime[i]), dict); - if (t == NULL) return -1; + if (t == NULL) { goto exit; } PyDict_SetItem(dict, t->name, (PyObject *)t); + Py_DECREF((PyObject *)t); + t = NULL; } - return 0; + rv = 0; + +exit: + Py_XDECREF((PyObject *)t); + return rv; } /* typecast_add - add a type object to the dictionary */ From 6cece00958875e7313ac36daf26fb4898fca66e2 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sun, 4 Mar 2012 04:42:44 +0000 Subject: [PATCH 28/36] Check failed list creation in array adaptation --- psycopg/typecast_array.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/psycopg/typecast_array.c b/psycopg/typecast_array.c index 6d825f42..b5f1062c 100644 --- a/psycopg/typecast_array.c +++ b/psycopg/typecast_array.c @@ -260,7 +260,7 @@ typecast_GENERIC_ARRAY_cast(const char *str, Py_ssize_t len, PyObject *curs) Dprintf("typecast_GENERIC_ARRAY_cast: str = '%s'," " len = " FORMAT_CODE_PY_SSIZE_T, str, len); - obj = PyList_New(0); + if (!(obj = PyList_New(0))) { return NULL; } /* scan the array skipping the first level of {} */ if (typecast_array_scan(&str[1], len-2, curs, base, obj) < 0) { From 5bbfd38dfb03655bc27a3114f729d80102fe38bd Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sun, 4 Mar 2012 04:43:14 +0000 Subject: [PATCH 29/36] Check for errors in float adaptation --- psycopg/typecast_basic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/psycopg/typecast_basic.c b/psycopg/typecast_basic.c index 20c7b81f..5e2a93ea 100644 --- a/psycopg/typecast_basic.c +++ b/psycopg/typecast_basic.c @@ -65,7 +65,7 @@ typecast_FLOAT_cast(const char *s, Py_ssize_t len, PyObject *curs) PyObject *str = NULL, *flo = NULL; if (s == NULL) {Py_INCREF(Py_None); return Py_None;} - str = Text_FromUTF8AndSize(s, len); + if (!(str = Text_FromUTF8AndSize(s, len))) { return NULL; } #if PY_MAJOR_VERSION < 3 flo = PyFloat_FromString(str, NULL); #else From 76cc838a93dc2e55c6ddd101a543bda61cba4550 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sun, 4 Mar 2012 04:47:21 +0000 Subject: [PATCH 30/36] Expressions rewritten in a more normal way (double)'0'? :) --- psycopg/typecast_datetime.c | 2 +- psycopg/typecast_mxdatetime.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/psycopg/typecast_datetime.c b/psycopg/typecast_datetime.c index 7c1cfa45..18e9d0d8 100644 --- a/psycopg/typecast_datetime.c +++ b/psycopg/typecast_datetime.c @@ -239,7 +239,7 @@ typecast_PYINTERVAL_cast(const char *str, Py_ssize_t len, PyObject *curs) case '0': case '1': case '2': case '3': case '4': case '5': case '6': case '7': case '8': case '9': - v = v*10 + (double)*str - (double)'0'; + v = v * 10.0 + (double)(*str - '0'); if (part == 6){ denominator *= 10; } diff --git a/psycopg/typecast_mxdatetime.c b/psycopg/typecast_mxdatetime.c index e61224df..bf69af5a 100644 --- a/psycopg/typecast_mxdatetime.c +++ b/psycopg/typecast_mxdatetime.c @@ -142,7 +142,7 @@ typecast_MXINTERVAL_cast(const char *str, Py_ssize_t len, PyObject *curs) case '0': case '1': case '2': case '3': case '4': case '5': case '6': case '7': case '8': case '9': - v = v*10 + (double)*str - (double)'0'; + v = v * 10.0 + (double)(*str - '0'); Dprintf("typecast_MXINTERVAL_cast: v = %f", v); if (part == 6){ denominator *= 10; From 0e832b97ea7a84220f2148db3746e10f85eadef2 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sun, 4 Mar 2012 17:59:51 +0000 Subject: [PATCH 31/36] Proper type check in prepare() methods for list, binary, qstring --- psycopg/adapter_binary.c | 10 ++++------ psycopg/adapter_list.c | 6 +++--- psycopg/adapter_qstring.c | 14 ++++++-------- 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/psycopg/adapter_binary.c b/psycopg/adapter_binary.c index 2574c603..da3bec60 100644 --- a/psycopg/adapter_binary.c +++ b/psycopg/adapter_binary.c @@ -149,16 +149,14 @@ binary_str(binaryObject *self) static PyObject * binary_prepare(binaryObject *self, PyObject *args) { - connectionObject *conn; + PyObject *conn; - if (!PyArg_ParseTuple(args, "O", &conn)) + if (!PyArg_ParseTuple(args, "O!", &connectionType, &conn)) return NULL; Py_XDECREF(self->conn); - if (conn) { - self->conn = (PyObject*)conn; - Py_INCREF(self->conn); - } + self->conn = conn; + Py_INCREF(self->conn); Py_INCREF(Py_None); return Py_None; diff --git a/psycopg/adapter_list.c b/psycopg/adapter_list.c index cbb75422..d97ecfb1 100644 --- a/psycopg/adapter_list.c +++ b/psycopg/adapter_list.c @@ -98,9 +98,9 @@ list_getquoted(listObject *self, PyObject *args) static PyObject * list_prepare(listObject *self, PyObject *args) { - connectionObject *conn; + PyObject *conn; - if (!PyArg_ParseTuple(args, "O", &conn)) + if (!PyArg_ParseTuple(args, "O!", &connectionType, &conn)) return NULL; /* note that we don't copy the encoding from the connection, but take a @@ -109,7 +109,7 @@ list_prepare(listObject *self, PyObject *args) work even without a connection to the backend. */ Py_CLEAR(self->connection); Py_INCREF(conn); - self->connection = (PyObject*)conn; + self->connection = conn; Py_INCREF(Py_None); return Py_None; diff --git a/psycopg/adapter_qstring.c b/psycopg/adapter_qstring.c index 4240d382..773cfa0d 100644 --- a/psycopg/adapter_qstring.c +++ b/psycopg/adapter_qstring.c @@ -124,24 +124,22 @@ qstring_str(qstringObject *self) static PyObject * qstring_prepare(qstringObject *self, PyObject *args) { - connectionObject *conn; + PyObject *conn; - if (!PyArg_ParseTuple(args, "O", &conn)) + if (!PyArg_ParseTuple(args, "O!", &connectionType, &conn)) return NULL; /* we bother copying the encoding only if the wrapped string is unicode, we don't need the encoding if that's not the case */ if (PyUnicode_Check(self->wrapped)) { if (self->encoding) free(self->encoding); - self->encoding = strdup(conn->codec); - Dprintf("qstring_prepare: set encoding to %s", conn->codec); + self->encoding = strdup(((connectionObject *)conn)->codec); + Dprintf("qstring_prepare: set encoding to %s", self->encoding); } Py_CLEAR(self->conn); - if (conn) { - Py_INCREF(conn); - self->conn = (PyObject*)conn; - } + Py_INCREF(conn); + self->conn = conn; Py_INCREF(Py_None); return Py_None; From 531084d561ee5aaaccbc892373f8551ae7ef3b1a Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sun, 4 Mar 2012 18:01:08 +0000 Subject: [PATCH 32/36] Stricter types usage in several PyArg_ParseTuple calls --- psycopg/connection_type.c | 4 ++-- psycopg/lobject_type.c | 4 ++-- psycopg/typecast.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/psycopg/connection_type.c b/psycopg/connection_type.c index 9cdd640f..374d34f8 100644 --- a/psycopg/connection_type.c +++ b/psycopg/connection_type.c @@ -701,8 +701,8 @@ psyco_conn_get_parameter_status(connectionObject *self, PyObject *args) static PyObject * psyco_conn_lobject(connectionObject *self, PyObject *args, PyObject *keywds) { - Oid oid=InvalidOid, new_oid=InvalidOid; - char *new_file = NULL; + int oid = (int)InvalidOid, new_oid = (int)InvalidOid; + const char *new_file = NULL; const char *smode = ""; PyObject *factory = (PyObject *)&lobjectType; PyObject *obj; diff --git a/psycopg/lobject_type.c b/psycopg/lobject_type.c index a55272ca..625a2939 100644 --- a/psycopg/lobject_type.c +++ b/psycopg/lobject_type.c @@ -373,7 +373,7 @@ lobject_dealloc(PyObject* obj) static int lobject_init(PyObject *obj, PyObject *args, PyObject *kwds) { - Oid oid=InvalidOid, new_oid=InvalidOid; + int oid = (int)InvalidOid, new_oid = (int)InvalidOid; const char *smode = ""; const char *new_file = NULL; PyObject *conn; @@ -383,7 +383,7 @@ lobject_init(PyObject *obj, PyObject *args, PyObject *kwds) return -1; return lobject_setup((lobjectObject *)obj, - (connectionObject *)conn, oid, smode, new_oid, new_file); + (connectionObject *)conn, (Oid)oid, smode, (Oid)new_oid, new_file); } static PyObject * diff --git a/psycopg/typecast.c b/psycopg/typecast.c index 9c36b02b..8504631b 100644 --- a/psycopg/typecast.c +++ b/psycopg/typecast.c @@ -468,7 +468,7 @@ typecast_repr(PyObject *self) static PyObject * typecast_call(PyObject *obj, PyObject *args, PyObject *kwargs) { - char *string; + const char *string; Py_ssize_t length; PyObject *cursor; From a9dc1b83ad96c336f2ff208ed01b591fb08081a0 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Mon, 5 Mar 2012 01:26:28 +0000 Subject: [PATCH 33/36] Methods callbacks signatures match the flags they are exported with --- psycopg/connection_type.c | 12 ++++++------ psycopg/cursor_type.c | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/psycopg/connection_type.c b/psycopg/connection_type.c index 374d34f8..56339167 100644 --- a/psycopg/connection_type.c +++ b/psycopg/connection_type.c @@ -754,7 +754,7 @@ psyco_conn_lobject(connectionObject *self, PyObject *args, PyObject *keywds) "get_backend_pid() -- Get backend process id." static PyObject * -psyco_conn_get_backend_pid(connectionObject *self) +psyco_conn_get_backend_pid(connectionObject *self, PyObject *args) { EXC_IF_CONN_CLOSED(self); @@ -767,7 +767,7 @@ psyco_conn_get_backend_pid(connectionObject *self) "reset() -- Reset current connection to defaults." static PyObject * -psyco_conn_reset(connectionObject *self) +psyco_conn_reset(connectionObject *self, PyObject *args) { int res; @@ -795,7 +795,7 @@ psyco_conn_get_exception(PyObject *self, void *closure) } static PyObject * -psyco_conn_poll(connectionObject *self) +psyco_conn_poll(connectionObject *self, PyObject *args) { int res; @@ -817,7 +817,7 @@ psyco_conn_poll(connectionObject *self) "fileno() -> int -- Return file descriptor associated to database connection." static PyObject * -psyco_conn_fileno(connectionObject *self) +psyco_conn_fileno(connectionObject *self, PyObject *args) { long int socket; @@ -836,7 +836,7 @@ psyco_conn_fileno(connectionObject *self) "executing an asynchronous operation." static PyObject * -psyco_conn_isexecuting(connectionObject *self) +psyco_conn_isexecuting(connectionObject *self, PyObject *args) { /* synchronous connections will always return False */ if (self->async == 0) { @@ -868,7 +868,7 @@ psyco_conn_isexecuting(connectionObject *self) "cancel() -- cancel the current operation" static PyObject * -psyco_conn_cancel(connectionObject *self) +psyco_conn_cancel(connectionObject *self, PyObject *args) { char errbuf[256]; diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index dbcc4788..533eb217 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -995,7 +995,7 @@ exit: "callproc(procname, parameters=None) -- Execute stored procedure." static PyObject * -psyco_curs_callproc(cursorObject *self, PyObject *args, PyObject *kwargs) +psyco_curs_callproc(cursorObject *self, PyObject *args) { const char *procname = NULL; char *sql = NULL; From 735d50c782b436d6807fd15e1b37e2231ce3e146 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Mon, 5 Mar 2012 02:08:45 +0000 Subject: [PATCH 34/36] Check if the object wrapped in binary is not None before trying the other types Otherwise it seems we clobber some result with NULL. --- psycopg/adapter_binary.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/psycopg/adapter_binary.c b/psycopg/adapter_binary.c index da3bec60..b08c1447 100644 --- a/psycopg/adapter_binary.c +++ b/psycopg/adapter_binary.c @@ -65,6 +65,13 @@ binary_quote(binaryObject *self) int got_view = 0; #endif + /* Allow Binary(None) to work */ + if (self->wrapped == Py_None) { + Py_INCREF(psyco_null); + rv = psyco_null; + goto exit; + } + /* if we got a plain string or a buffer we escape it and save the buffer */ #if HAS_MEMORYVIEW @@ -93,7 +100,7 @@ binary_quote(binaryObject *self) /* escape and build quoted buffer */ - to = (char *)binary_escape((unsigned char*)buffer, (size_t) buffer_len, + to = (char *)binary_escape((unsigned char*)buffer, (size_t)buffer_len, &len, self->conn ? ((connectionObject*)self->conn)->pgconn : NULL); if (to == NULL) { PyErr_NoMemory(); @@ -113,12 +120,6 @@ exit: if (got_view) { PyBuffer_Release(&view); } #endif - /* Allow Binary(None) to work */ - if (self->wrapped == Py_None) { - Py_INCREF(psyco_null); - rv = psyco_null; - } - /* if the wrapped object is not bytes or a buffer, this is an error */ if (!rv && !PyErr_Occurred()) { PyErr_Format(PyExc_TypeError, "can't escape %s to binary", From 8707d8c3991d2f6511876ce48089a46dd1f97ae7 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Mon, 5 Mar 2012 02:09:20 +0000 Subject: [PATCH 35/36] Fixed iterator refcount in case of memory error during COPY --- psycopg/cursor_type.c | 1 + 1 file changed, 1 insertion(+) diff --git a/psycopg/cursor_type.c b/psycopg/cursor_type.c index 533eb217..103c1db3 100644 --- a/psycopg/cursor_type.c +++ b/psycopg/cursor_type.c @@ -1205,6 +1205,7 @@ static char *_psyco_curs_copy_columns(PyObject *columns) } if (NULL == (columnlist = PyMem_Malloc(bufsize))) { + Py_DECREF(coliter); PyErr_NoMemory(); goto error; } From 2c309dfdb46cce51ff17688c97b391e39f2a392e Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Mon, 5 Mar 2012 02:29:02 +0000 Subject: [PATCH 36/36] Mention the static analysis cleanup in the news --- NEWS | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/NEWS b/NEWS index 3e687ab8..40787b59 100644 --- a/NEWS +++ b/NEWS @@ -9,6 +9,10 @@ What's new in psycopg 2.4.5 interaction (ticket #90). - Better efficiency and formatting of timezone offset objects thanks to Menno Smits (tickets #94, #95). + - Codebase cleaned up using the GCC Python plugin's static analysis + tool, which has revealed several unchecked return values, possible + NULL dereferences, reference counting problems. Many thanks to David + Malcolm for the useful tool and the assistance provided using it. What's new in psycopg 2.4.4