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.

9 files changed, 92 insertions, 50 deletions

diff --git a/psycopg/config.h b/psycopg/config.h
index 2038ced..4d3fa99 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 9a9ea42..d4a8348 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 cb65224..4c12e1d 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 2f24091..d88126b 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 4c4d5af..b60263b 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; }
-
-    res = 1; /* Success */
-    goto cleanup;
+    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; }
 
-    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);
+    res = 0; /* Success */
 
-        Py_XDECREF(cvt);
+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);
 
-        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 e3bba12..816adb2 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 bf012ad..ef0e325 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 01a68d9..60f01b1 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 60fec50..7d77ee1 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)