From 7d715dee47a07202b84ba5f0d9c919fa7e5bb067 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Fri, 23 Jan 2015 21:27:57 +0200 Subject: [PATCH] Fix miscellaneous little issues spotted by Coverity. A few were real bugs, although highly unlikely ones. Others were false positives like dead code, but let's be tidy. --- connection.c | 90 ++++++++++++++++++++++++---------------------------- convert.c | 14 +++----- drvconn.c | 2 +- info.c | 4 +-- multibyte.c | 2 ++ options.c | 3 +- pgapi30.c | 1 + results.c | 45 ++++++++++++++++---------- statement.c | 7 +--- 9 files changed, 82 insertions(+), 86 deletions(-) diff --git a/connection.c b/connection.c index 1d933d6..8a924eb 100644 --- a/connection.c +++ b/connection.c @@ -381,7 +381,6 @@ CC_lockinit(ConnectionClass *self) static ConnectionClass * CC_initialize(ConnectionClass *rv, BOOL lockinit) { - ConnectionClass *retrv = NULL; size_t clear_size; #if defined(WIN_MULTITHREAD_SUPPORT) || defined(POSIX_THREADMUTEX_SUPPORT) @@ -428,11 +427,12 @@ CC_initialize(ConnectionClass *rv, BOOL lockinit) #endif /* _HANDLE_ENLIST_IN_DTC_ */ if (lockinit) CC_lockinit(rv); - retrv = rv; + + return rv; + cleanup: - if (rv && !retrv) - CC_Destructor(rv); - return retrv; + CC_Destructor(rv); + return NULL; } ConnectionClass * @@ -872,7 +872,7 @@ handle_pgres_error(ConnectionClass *self, const PGresult *pgres, if (res) { QR_set_rstatus(res, PORES_FATAL_ERROR); - if (errmsg && errmsg[0]) + if (errmsg[0]) QR_set_message(res, errmsg); QR_set_aborted(res, TRUE); } @@ -1880,7 +1880,7 @@ CC_send_query_append(ConnectionClass *self, const char *query, QueryInfo *qi, UD * this should not happen, and if it does, we've already overrun * the buffer and possibly corrupted memory. */ - SC_set_error(stmt, STMT_INTERNAL_ERROR, "query buffer overrun", func); + CC_set_error(self, CONNECTION_COULD_NOT_SEND, "query buffer overrun", func); goto cleanup; } @@ -2629,12 +2629,12 @@ LIBPQ_update_transaction_status(ConnectionClass *self) static int LIBPQ_connect(ConnectionClass *self) { - CSTR func = "LIBPQ_connect"; - char ret = 0; - char *conninfo = NULL; - void *pqconn = NULL; - int pqret; - int pversion; + CSTR func = "LIBPQ_connect"; + char ret = 0; + char *conninfo = NULL; + void *pqconn = NULL; + int pqret; + int pversion; mylog("connecting to the database using %s as the server\n",self->connInfo.server); @@ -2642,52 +2642,43 @@ LIBPQ_connect(ConnectionClass *self) { if (CC_get_errornumber(self) <= 0) CC_set_error(self, CONN_OPENDB_ERROR, "Couldn't allcate conninfo", func); - goto cleanup1; + goto cleanup; } pqconn = PQconnectdb(conninfo); - free(conninfo); - if (!pqconn) { CC_set_error(self, CONN_OPENDB_ERROR, "PQconnectdb error", func); - goto cleanup1; + goto cleanup; } self->pqconn = pqconn; + pqret = PQstatus(pqconn); + if (pqret == CONNECTION_BAD && PQconnectionNeedsPassword(pqconn)) + { + mylog("password retry\n"); + PQfinish(pqconn); + self->pqconn = NULL; + self->connInfo.password_required = TRUE; + ret = -1; + goto cleanup; + } + if (CONNECTION_OK != pqret) { const char *errmsg; inolog("status=%d\n", pqret); errmsg = PQerrorMessage(pqconn); CC_set_error(self, CONNECTION_SERVER_NOT_REACHED, errmsg, func); - if (CONNECTION_BAD == pqret && PQconnectionNeedsPassword(pqconn)) - { - mylog("password retry\n"); - PQfinish(pqconn); - self->pqconn = NULL; - self->connInfo.password_required = TRUE; - return -1; - } mylog("Could not establish connection to the database; LIBPQ returned -> %s\n", errmsg); - goto cleanup1; + goto cleanup; } - ret = 1; -cleanup1: - if (!ret) - { - if (self->pqconn) - PQfinish(self->pqconn); - self->pqconn = NULL; - return ret; - } - mylog("libpq connection to the database succeeded.\n"); - ret = 0; + mylog("libpq connection to the database established.\n"); pversion = PQprotocolVersion(pqconn); if (pversion < 3) { mylog("Protocol version %d is not supported\n", pversion); - goto cleanup1; + goto cleanup; } mylog("protocol=%d\n", pversion); @@ -2697,23 +2688,24 @@ cleanup1: sprintf(self->pg_version, "%d.%d.%d", self->pg_version_major, self->pg_version_minor, pversion % 100); mylog("Server version=%s\n", self->pg_version); - ret = 1; - if (ret) + + if (!CC_get_username(self)[0]) { - if (!CC_get_username(self)[0]) - { - mylog("PQuser=%s\n", PQuser(pqconn)); - strcpy(self->connInfo.username, PQuser(pqconn)); - } + mylog("PQuser=%s\n", PQuser(pqconn)); + strncpy_null(self->connInfo.username, PQuser(pqconn), sizeof(self->connInfo.username)); } - else + + ret = 1; + +cleanup: + if (ret != 1) { if (self->pqconn) - { PQfinish(self->pqconn); - self->pqconn = NULL; - } + self->pqconn = NULL; } + if (conninfo) + free(conninfo); mylog("%s: retuning %d\n", func, ret); return ret; diff --git a/convert.c b/convert.c index 3ed6ee3..c9f78d5 100644 --- a/convert.c +++ b/convert.c @@ -2647,7 +2647,7 @@ inolog("prepareParametersNoDesc\n"); if (NAMED_PARSE_REQUEST == SC_get_prepare_method(stmt)) sprintf(plan_name, "_PLAN%p", stmt); else - strcpy(plan_name, NULL_STRING); + plan_name[0] = '\0'; stmt->current_exec_param = 0; multi = stmt->multi_statement; @@ -2897,7 +2897,7 @@ inolog("type=%d concur=%d\n", stmt->options.cursor_type, stmt->options.scroll_co if (NAMED_PARSE_REQUEST == SC_get_prepare_method(stmt)) sprintf(plan_name, "_PLAN%p", stmt); else - strcpy(plan_name, NULL_STRING); + plan_name[0] = '\0'; SC_set_planname(stmt, plan_name); SC_set_prepared(stmt, plan_name[0] ? PREPARING_PERMANENTLY : PREPARING_TEMPORARILY); @@ -3596,7 +3596,7 @@ build_libpq_bind_params(StatementClass *stmt, SQLSMALLINT num_p; int i, num_params; ConnectionClass *conn = SC_get_conn(stmt); - BOOL ret = FALSE, sockerr = FALSE, discard_output; + BOOL ret = FALSE, discard_output; RETCODE retval; const IPDFields *ipdopts = SC_get_IPDF(stmt); @@ -3628,7 +3628,7 @@ build_libpq_bind_params(StatementClass *stmt, goto cleanup; memset(*paramValues, 0, sizeof(char *) * num_params); *paramLengths = malloc(sizeof(int) * num_params); - if (paramLengths == NULL) + if (*paramLengths == NULL) goto cleanup; *paramFormats = malloc(sizeof(int) * num_params); if (*paramFormats == NULL) @@ -3707,12 +3707,6 @@ inolog("num_p=%d\n", num_p); cleanup: QB_Destructor(&qb); - if (sockerr) - { - CC_set_error(conn, CONNECTION_COULD_NOT_SEND, "Could not send D Request to backend", func); - CC_on_abort(conn, CONN_DEAD); - ret = FALSE; - } return ret; } diff --git a/drvconn.c b/drvconn.c index 22a7fbd..69553d0 100644 --- a/drvconn.c +++ b/drvconn.c @@ -518,7 +518,7 @@ dconn_get_attributes(copyfunc func, const char *connect_string, ConnInfo *ci) #endif /* FORCE_PASSWORD_DISPLAY */ mylog("attribute = '%s', value = '%s'\n", attribute, value); - if (!attribute || !value) + if (!attribute) continue; /* Copy the appropriate value to the conninfo */ diff --git a/info.c b/info.c index 9946792..31fd684 100644 --- a/info.c +++ b/info.c @@ -3534,8 +3534,8 @@ PGAPI_ColumnPrivileges(HSTMT hstmt, /* Neither Access or Borland care about this. */ - if (result = SC_initialize_and_recycle(stmt), SQL_SUCCESS != result) - return result; + if (SC_initialize_and_recycle(stmt) != SQL_SUCCESS) + return SQL_ERROR; escSchemaName = simpleCatalogEscape(szTableOwner, cbTableOwner, conn); escTableName = simpleCatalogEscape(szTableName, cbTableName, conn); search_pattern = (0 == (flag & PODBC_NOT_SEARCH_PATTERN)); diff --git a/multibyte.c b/multibyte.c index 6a30dba..641243a 100644 --- a/multibyte.c +++ b/multibyte.c @@ -121,11 +121,13 @@ check_client_encoding(const pgNAME conn_settings) for (cptr = SAFE_NAME(conn_settings); *cptr; cptr++) { if (in_quote) + { if (LITERAL_QUOTE == *cptr) { in_quote = FALSE; continue; } + } if (';' == *cptr) { allowed_cmd = TRUE; diff --git a/options.c b/options.c index ad77a08..efdad32 100644 --- a/options.c +++ b/options.c @@ -241,8 +241,9 @@ set_statement_option(ConnectionClass *conn, if (0 != vParam) changed = TRUE; break; -#endif /* NOT_USED */ +#else SC_set_error(stmt, STMT_OPTION_NOT_FOR_THE_DRIVER, "The option may be for MS SQL Server(Set)", func); +#endif /* NOT_USED */ } else if (conn) { diff --git a/pgapi30.c b/pgapi30.c index 8c5155d..6f8900b 100644 --- a/pgapi30.c +++ b/pgapi30.c @@ -1819,6 +1819,7 @@ PGAPI_SetDescField(SQLHDESC DescriptorHandle, { case DESC_INVALID_DESCRIPTOR_IDENTIFIER: DC_set_errormsg(desc, "can't SQLSetDescField for this descriptor identifier"); + break; case DESC_INVALID_COLUMN_NUMBER_ERROR: DC_set_errormsg(desc, "can't SQLSetDescField for this column number"); break; diff --git a/results.c b/results.c index 3925f1b..e9138c9 100644 --- a/results.c +++ b/results.c @@ -2140,6 +2140,7 @@ static void RemoveAdded(QResultClass *, SQLLEN); static void RemoveUpdated(QResultClass *, SQLLEN); static void RemoveUpdatedAfterTheKey(QResultClass *, SQLLEN, const KeySet*); static void RemoveDeleted(QResultClass *, SQLLEN); + static void RemoveAdded(QResultClass *res, SQLLEN index) { SQLLEN rmidx, mv_count; @@ -2169,7 +2170,8 @@ static void RemoveAdded(QResultClass *res, SQLLEN index) mylog("RemoveAdded removed=1 count=%d\n", res->ad_count); } -static void CommitAdded(QResultClass *res) +static void +CommitAdded(QResultClass *res) { KeySet *added_keyset; int i; @@ -2204,8 +2206,8 @@ inolog("!!Commit Added=%d(%d)\n", QR_get_num_total_read(res) + i, i); } } - -int AddDeleted(QResultClass *res, SQLULEN index, KeySet *keyset) +static int +AddDeleted(QResultClass *res, SQLULEN index, KeySet *keyset) { int i; Int2 dl_count, new_alloc; @@ -2215,7 +2217,6 @@ int AddDeleted(QResultClass *res, SQLULEN index, KeySet *keyset) Int2 num_fields = res->num_fields; inolog("AddDeleted %d\n", index); - if (!res) return FALSE; dl_count = res->dl_count; res->dl_count++; if (!QR_get_cursor(res)) @@ -2272,7 +2273,8 @@ inolog("AddDeleted %d\n", index); return TRUE; } -static void RemoveDeleted(QResultClass *res, SQLLEN index) +static void +RemoveDeleted(QResultClass *res, SQLLEN index) { int i, mv_count, rm_count = 0; SQLLEN pidx, midx; @@ -2313,7 +2315,8 @@ static void RemoveDeleted(QResultClass *res, SQLLEN index) mylog("RemoveDeleted removed count=%d,%d\n", rm_count, res->dl_count); } -static void CommitDeleted(QResultClass *res) +static void +CommitDeleted(QResultClass *res) { int i; SQLLEN *deleted; @@ -2349,7 +2352,8 @@ inolog("!!Commit Deleted=%d(%d)\n", *deleted, i); } } -static BOOL enlargeUpdated(QResultClass *res, Int4 number, const StatementClass *stmt) +static BOOL +enlargeUpdated(QResultClass *res, Int4 number, const StatementClass *stmt) { Int2 alloc; @@ -2373,7 +2377,8 @@ static BOOL enlargeUpdated(QResultClass *res, Int4 number, const StatementClass return TRUE; } -static void AddUpdated(StatementClass *stmt, SQLLEN index) +static void +AddUpdated(StatementClass *stmt, SQLLEN index) { QResultClass *res; SQLLEN *updated; @@ -2479,13 +2484,15 @@ inolog("AddUpdated index=%d\n", index); mylog("up_count=%d\n", res->up_count); } -static void RemoveUpdated(QResultClass *res, SQLLEN index) +static void +RemoveUpdated(QResultClass *res, SQLLEN index) { mylog("RemoveUpdated index=%d\n", index); RemoveUpdatedAfterTheKey(res, index, NULL); } -static void RemoveUpdatedAfterTheKey(QResultClass *res, SQLLEN index, const KeySet *keyset) +static void +RemoveUpdatedAfterTheKey(QResultClass *res, SQLLEN index, const KeySet *keyset) { SQLLEN *updated, num_read = QR_get_num_total_read(res); KeySet *updated_keyset; @@ -2539,7 +2546,8 @@ static void RemoveUpdatedAfterTheKey(QResultClass *res, SQLLEN index, const KeyS mylog("RemoveUpdatedAfter removed count=%d,%d\n", rm_count, res->up_count); } -static void CommitUpdated(QResultClass *res) +static void +CommitUpdated(QResultClass *res) { KeySet *updated_keyset; int i; @@ -2580,7 +2588,8 @@ inolog("!!Commit Updated=%d(%d)\n", res->updated[i], i); } -static void DiscardRollback(StatementClass *stmt, QResultClass *res) +static void +DiscardRollback(StatementClass *stmt, QResultClass *res) { int i; SQLLEN index, kres_ridx; @@ -2628,7 +2637,9 @@ inolog("DiscardRollback"); } static QResultClass *positioned_load(StatementClass *stmt, UInt4 flag, const UInt4 *oidint, const char *tid); -static void UndoRollback(StatementClass *stmt, QResultClass *res, BOOL partial) + +static void +UndoRollback(StatementClass *stmt, QResultClass *res, BOOL partial) { Int4 i, rollbp; SQLLEN index, ridx, kres_ridx; @@ -2646,7 +2657,8 @@ static void UndoRollback(StatementClass *stmt, QResultClass *res, BOOL partial) if (partial) { SQLLEN pidx, midx; - Int2 doubtp, rollbps; + int rollbps; + int doubtp; int j; for (i = 0, doubtp = 0; i < res->rb_count; i++) @@ -2676,8 +2688,6 @@ inolog(" doubtp=%d\n", doubtp); } rollbp = i; inolog(" doubtp=%d,rollbp=%d\n", doubtp, rollbp); - if (doubtp < 0) - doubtp = 0; do { rollbps = rollbp; @@ -2812,7 +2822,8 @@ inolog("->(%u, %u)\n", wkey->blocknum, wkey->offset); } } -void ProcessRollback(ConnectionClass *conn, BOOL undo, BOOL partial) +void +ProcessRollback(ConnectionClass *conn, BOOL undo, BOOL partial) { int i; StatementClass *stmt; diff --git a/statement.c b/statement.c index a141f35..adcbca3 100644 --- a/statement.c +++ b/statement.c @@ -469,10 +469,9 @@ SC_Constructor(ConnectionClass *conn) char SC_Destructor(StatementClass *self) { - CSTR func = "SC_Destrcutor"; + CSTR func = "SC_Destructor"; QResultClass *res = SC_get_Result(self); - if (!self) return FALSE; mylog("SC_Destructor: self=%p, self->result=%p, self->hdbc=%p\n", self, res, self->hdbc); SC_clear_error(self); if (STMT_EXECUTING == self->status) @@ -2772,7 +2771,6 @@ ParseAndDescribeWithLibpq(StatementClass *stmt, const char *plan_name, { CSTR func = "ParseAndDescribeWithLibpq"; ConnectionClass *conn = SC_get_conn(stmt); - Oid *paramTypes = NULL; PGresult *pgres = NULL; int num_p; Int2 num_discard_params; @@ -2904,9 +2902,6 @@ inolog("num_params=%d info=%d\n", stmt->num_params, num_p); } cleanup: - if (paramTypes) - free(paramTypes); - if (pgres) PQclear(pgres); -- 2.39.5