From: Thomas Markwalder Date: Wed, 18 May 2016 13:32:30 +0000 (-0400) Subject: [4276] Replace use of PGresult with PgSqlResult throughout X-Git-Tag: trac4106_update_base~12^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a7a134e786562156073a6c2544ff08285aaf6170;p=thirdparty%2Fkea.git [4276] Replace use of PGresult with PgSqlResult throughout --- diff --git a/src/lib/dhcpsrv/pgsql_connection.cc b/src/lib/dhcpsrv/pgsql_connection.cc index 1d565a2e6c..c820fb4188 100644 --- a/src/lib/dhcpsrv/pgsql_connection.cc +++ b/src/lib/dhcpsrv/pgsql_connection.cc @@ -22,15 +22,15 @@ // code is 5 decimal, ASCII, digits, the first two define the category of // error, the last three are the specific error. PostgreSQL makes the state // code as a char[5]. Macros for each code are defined in PostgreSQL's -// server/utils/errcodes.h, although they require a second macro, +// server/utils/errcodes.h, although they require a second macro, // MAKE_SQLSTATE for completion. For example, duplicate key error as: // // #define ERRCODE_UNIQUE_VIOLATION MAKE_SQLSTATE('2','3','5','0','5') -// -// PostgreSQL deliberately omits the MAKE_SQLSTATE macro so callers can/must +// +// PostgreSQL deliberately omits the MAKE_SQLSTATE macro so callers can/must // supply their own. We'll define it as an initlizer_list: #define MAKE_SQLSTATE(ch1,ch2,ch3,ch4,ch5) {ch1,ch2,ch3,ch4,ch5} -// So we can use it like this: const char some_error[] = ERRCODE_xxxx; +// So we can use it like this: const char some_error[] = ERRCODE_xxxx; #define PGSQL_STATECODE_LEN 5 #include @@ -101,7 +101,7 @@ PgSqlConnection::openDatabase() { isc_throw(NoDatabaseName, "must specify a name for the database"); } - // Connect to Postgres, saving the low level connection pointer + // Connect to Postgres, saving the low level connection pointer // in the holder object PGconn* new_conn = PQconnectdb(dbconnparameters.c_str()); if (!new_conn) { @@ -120,8 +120,8 @@ PgSqlConnection::openDatabase() { conn_.setConnection(new_conn); } -bool -PgSqlConnection::compareError(PGresult*& r, const char* error_state) { +bool +PgSqlConnection::compareError(const PgSqlResult& r, const char* error_state) { const char* sqlstate = PQresultErrorField(r, PG_DIAG_SQLSTATE); // PostgreSQL garuantees it will always be 5 characters long return ((sqlstate != NULL) && @@ -129,7 +129,7 @@ PgSqlConnection::compareError(PGresult*& r, const char* error_state) { } void -PgSqlConnection::checkStatementError(PGresult*& r, +PgSqlConnection::checkStatementError(const PgSqlResult& r, PgSqlTaggedStatement& statement) const { int s = PQresultStatus(r); if (s != PGRES_COMMAND_OK && s != PGRES_TUPLES_OK) { diff --git a/src/lib/dhcpsrv/pgsql_connection.h b/src/lib/dhcpsrv/pgsql_connection.h index 00405ae3a7..d20cfb2545 100755 --- a/src/lib/dhcpsrv/pgsql_connection.h +++ b/src/lib/dhcpsrv/pgsql_connection.h @@ -18,8 +18,9 @@ namespace isc { namespace dhcp { // Maximum number of parameters that can be used a statement -// @todo This allows us to use an initializer list (since we don't -// require C++11). It's unlikely we'd go past in a single statement. +// @todo This allows us to use an initializer list (since we can't +// require C++11). It's unlikely we'd go past this many a single +// statement. const size_t PGSQL_MAX_PARAMETERS_IN_QUERY = 32; /// @brief Defines a Postgresql SQL statement @@ -95,7 +96,7 @@ public: /// @brief Conversion Operator /// - /// Allows the PgSqlResult object to be passed as the context argument to + /// Allows the PgSqlResult object to be passed as the result set argument to /// PQxxxx functions. operator PGresult*() const { return (result_); @@ -108,13 +109,12 @@ public: return (result_); } - private: PGresult* result_; ///< Result set to be freed }; -/// @brief PgSql Handle Holder +/// @brief Postgresql connection handle Holder /// /// Small RAII object for safer initialization, will close the database /// connection upon destruction. This means that if an exception is thrown @@ -241,7 +241,7 @@ public: /// /// @return True if the result set's SQL state equals the error_state, /// false otherwise. - bool compareError(PGresult*& r, const char* error_state); + bool compareError(const PgSqlResult& r, const char* error_state); /// @brief Checks result of the r object /// @@ -263,7 +263,8 @@ public: /// @param statement - tagged statement that was executed /// /// @throw isc::dhcp::DbOperationError Detailed PostgreSQL failure - void checkStatementError(PGresult*& r, PgSqlTaggedStatement& statement) const; + void checkStatementError(const PgSqlResult& r, + PgSqlTaggedStatement& statement) const; /// @brief PgSql connection handle /// diff --git a/src/lib/dhcpsrv/pgsql_exchange.cc b/src/lib/dhcpsrv/pgsql_exchange.cc index 69f8bb8cb3..f1a90ee217 100644 --- a/src/lib/dhcpsrv/pgsql_exchange.cc +++ b/src/lib/dhcpsrv/pgsql_exchange.cc @@ -55,8 +55,8 @@ std::string PsqlBindArray::toText() { } else { stream << "0x"; for (int i = 0; i < lengths_[i]; ++i) { - stream << std::setfill('0') << std::setw(2) - << std::setbase(16) + stream << std::setfill('0') << std::setw(2) + << std::setbase(16) << static_cast(data[i]); } stream << std::endl; @@ -67,7 +67,7 @@ std::string PsqlBindArray::toText() { return (stream.str()); } -std::string +std::string PgSqlExchange::convertToDatabaseTime(const time_t input_time) { struct tm tinfo; char buffer[20]; @@ -77,9 +77,9 @@ PgSqlExchange::convertToDatabaseTime(const time_t input_time) { } std::string -PgSqlExchange::convertToDatabaseTime(const time_t cltt, +PgSqlExchange::convertToDatabaseTime(const time_t cltt, const uint32_t valid_lifetime) { - // Calculate expiry time. Store it in the 64-bit value so as we can + // Calculate expiry time. Store it in the 64-bit value so as we can // detect overflows. int64_t expire_time_64 = static_cast(cltt) + static_cast(valid_lifetime); @@ -96,7 +96,7 @@ PgSqlExchange::convertToDatabaseTime(const time_t cltt, return (convertToDatabaseTime(static_cast(expire_time_64))); } -time_t +time_t PgSqlExchange::convertFromDatabaseTime(const std::string& db_time_val) { // Convert string time value to time_t time_t new_time; @@ -110,7 +110,7 @@ PgSqlExchange::convertFromDatabaseTime(const std::string& db_time_val) { } const char* -PgSqlExchange::getRawColumnValue(PGresult*& r, const int row, +PgSqlExchange::getRawColumnValue(const PgSqlResult& r, const int row, const size_t col) const { const char* value = PQgetvalue(r, row, col); if (!value) { @@ -120,9 +120,9 @@ PgSqlExchange::getRawColumnValue(PGresult*& r, const int row, return (value); } -void -PgSqlExchange::getColumnValue(PGresult*& r, const int row, const size_t col, - bool &value) const { +void +PgSqlExchange::getColumnValue(const PgSqlResult& r, const int row, + const size_t col, bool &value) const { const char* data = getRawColumnValue(r, row, col); if (!strlen(data) || *data == 'f') { value = false; @@ -135,9 +135,9 @@ PgSqlExchange::getColumnValue(PGresult*& r, const int row, const size_t col, } } -void -PgSqlExchange::getColumnValue(PGresult*& r, const int row, const size_t col, - uint32_t &value) const { +void +PgSqlExchange::getColumnValue(const PgSqlResult& r, const int row, + const size_t col, uint32_t &value) const { const char* data = getRawColumnValue(r, row, col); try { value = boost::lexical_cast(data); @@ -148,9 +148,9 @@ PgSqlExchange::getColumnValue(PGresult*& r, const int row, const size_t col, } } -void -PgSqlExchange::getColumnValue(PGresult*& r, const int row, const size_t col, - int32_t &value) const { +void +PgSqlExchange::getColumnValue(const PgSqlResult& r, const int row, + const size_t col, int32_t &value) const { const char* data = getRawColumnValue(r, row, col); try { value = boost::lexical_cast(data); @@ -161,9 +161,9 @@ PgSqlExchange::getColumnValue(PGresult*& r, const int row, const size_t col, } } -void -PgSqlExchange::getColumnValue(PGresult*& r, const int row, const size_t col, - uint8_t &value) const { +void +PgSqlExchange::getColumnValue(const PgSqlResult& r, const int row, + const size_t col, uint8_t &value) const { const char* data = getRawColumnValue(r, row, col); try { // lexically casting as uint8_t doesn't convert from char @@ -176,9 +176,10 @@ PgSqlExchange::getColumnValue(PGresult*& r, const int row, const size_t col, } } -void -PgSqlExchange::convertFromBytea(PGresult*& r, const int row, const size_t col, - uint8_t* buffer, const size_t buffer_size, +void +PgSqlExchange::convertFromBytea(const PgSqlResult& r, const int row, + const size_t col, uint8_t* buffer, + const size_t buffer_size, size_t &bytes_converted) const { // Returns converted bytes in a dynamically allocated buffer, and // sets bytes_converted. diff --git a/src/lib/dhcpsrv/pgsql_exchange.h b/src/lib/dhcpsrv/pgsql_exchange.h index ec36d370dd..a5af449b53 100644 --- a/src/lib/dhcpsrv/pgsql_exchange.h +++ b/src/lib/dhcpsrv/pgsql_exchange.h @@ -157,7 +157,7 @@ public: /// /// @return a const char* pointer to the column's raw data /// @throw DbOperationError if the value cannot be fetched. - const char* getRawColumnValue(PGresult*& r, const int row, + const char* getRawColumnValue(const PgSqlResult& r, const int row, const size_t col) const; /// @brief Fetches boolean text ('t' or 'f') as a bool. @@ -169,7 +169,7 @@ public: /// /// @throw DbOperationError if the value cannot be fetched or is /// invalid. - void getColumnValue(PGresult*& r, const int row, const size_t col, + void getColumnValue(const PgSqlResult& r, const int row, const size_t col, bool &value) const; /// @brief Fetches an integer text column as a uint32_t. @@ -181,7 +181,7 @@ public: /// /// @throw DbOperationError if the value cannot be fetched or is /// invalid. - void getColumnValue(PGresult*& r, const int row, const size_t col, + void getColumnValue(const PgSqlResult& r, const int row, const size_t col, uint32_t &value) const; /// @brief Fetches an integer text column as a int32_t. @@ -193,7 +193,7 @@ public: /// /// @throw DbOperationError if the value cannot be fetched or is /// invalid. - void getColumnValue(PGresult*& r, const int row, const size_t col, + void getColumnValue(const PgSqlResult& r, const int row, const size_t col, int32_t &value) const; /// @brief Fetches an integer text column as a uint8_t. @@ -205,7 +205,7 @@ public: /// /// @throw DbOperationError if the value cannot be fetched or is /// invalid. - void getColumnValue(PGresult*& r, const int row, const size_t col, + void getColumnValue(const PgSqlResult& r, const int row, const size_t col, uint8_t &value) const; /// @brief Converts a column in a row in a result set to a binary bytes @@ -224,7 +224,7 @@ public: /// /// @throw DbOperationError if the value cannot be fetched or is /// invalid. - void convertFromBytea(PGresult*& r, const int row, const size_t col, + void convertFromBytea(const PgSqlResult& r, const int row, const size_t col, uint8_t* buffer, const size_t buffer_size, size_t &bytes_converted) const; diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.cc b/src/lib/dhcpsrv/pgsql_lease_mgr.cc index 3bae6d9e7f..f8ca8277ba 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.cc +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.cc @@ -371,7 +371,7 @@ public: /// /// @return Lease4Ptr to the newly created Lease4 object /// @throw DbOperationError if the lease cannot be created. - Lease4Ptr convertFromDatabase(PGresult*& r, int row) { + Lease4Ptr convertFromDatabase(const PgSqlResult& r, int row) { try { getColumnValue(r, row, ADDRESS_COL, addr4_); @@ -559,7 +559,7 @@ public: /// /// @return Lease6Ptr to the newly created Lease4 object /// @throw DbOperationError if the lease cannot be created. - Lease6Ptr convertFromDatabase(PGresult*& r, int row) { + Lease6Ptr convertFromDatabase(const PgSqlResult& r, int row) { try { isc::asiolink::IOAddress addr(getIPv6Value(r, row, ADDRESS_COL)); @@ -618,8 +618,8 @@ public: /// /// @throw DbOperationError if the value cannot be fetched or is /// invalid. - void getLeaseTypeColumnValue(PGresult*& r, const int row, const size_t col, - Lease6::Type& value) const { + void getLeaseTypeColumnValue(const PgSqlResult& r, const int row, + const size_t col, Lease6::Type& value) const { uint32_t raw_value = 0; getColumnValue(r, row , col, raw_value); switch (raw_value) { @@ -644,7 +644,7 @@ public: /// @return isc::asiolink::IOAddress containing the IPv6 address. /// @throw DbOperationError if the value cannot be fetched or is /// invalid. - isc::asiolink::IOAddress getIPv6Value(PGresult*& r, const int row, + isc::asiolink::IOAddress getIPv6Value(const PgSqlResult& r, const int row, const size_t col) const { const char* data = getRawColumnValue(r, row, col); try { @@ -722,11 +722,11 @@ PgSqlLeaseMgr::getDBVersion() { bool PgSqlLeaseMgr::addLeaseCommon(StatementIndex stindex, PsqlBindArray& bind_array) { - PGresult* r = PQexecPrepared(conn_, tagged_statements[stindex].name, - tagged_statements[stindex].nbparams, - &bind_array.values_[0], - &bind_array.lengths_[0], - &bind_array.formats_[0], 0); + PgSqlResult r(PQexecPrepared(conn_, tagged_statements[stindex].name, + tagged_statements[stindex].nbparams, + &bind_array.values_[0], + &bind_array.lengths_[0], + &bind_array.formats_[0], 0)); int s = PQresultStatus(r); @@ -735,15 +735,12 @@ PgSqlLeaseMgr::addLeaseCommon(StatementIndex stindex, // the case, we return false to indicate that the row was not added. // Otherwise we throw an exception. if (conn_.compareError(r, PgSqlConnection::DUPLICATE_KEY)) { - PQclear(r); return (false); } conn_.checkStatementError(r, tagged_statements[stindex]); } - PQclear(r); - return (true); } @@ -776,17 +773,16 @@ void PgSqlLeaseMgr::getLeaseCollection(StatementIndex stindex, LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_PGSQL_GET_ADDR4).arg(tagged_statements[stindex].name); - PGresult* r = PQexecPrepared(conn_, tagged_statements[stindex].name, - tagged_statements[stindex].nbparams, - &bind_array.values_[0], - &bind_array.lengths_[0], - &bind_array.formats_[0], 0); + PgSqlResult r(PQexecPrepared(conn_, tagged_statements[stindex].name, + tagged_statements[stindex].nbparams, + &bind_array.values_[0], + &bind_array.lengths_[0], + &bind_array.formats_[0], 0)); conn_.checkStatementError(r, tagged_statements[stindex]); int rows = PQntuples(r); if (single && rows > 1) { - PQclear(r); isc_throw(MultipleRecords, "multiple records were found in the " "database where only one was expected for query " << tagged_statements[stindex].name); @@ -795,8 +791,6 @@ void PgSqlLeaseMgr::getLeaseCollection(StatementIndex stindex, for(int i = 0; i < rows; ++ i) { result.push_back(exchange->convertFromDatabase(r, i)); } - - PQclear(r); } @@ -1094,16 +1088,15 @@ PgSqlLeaseMgr::updateLeaseCommon(StatementIndex stindex, LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_PGSQL_ADD_ADDR4).arg(tagged_statements[stindex].name); - PGresult* r = PQexecPrepared(conn_, tagged_statements[stindex].name, - tagged_statements[stindex].nbparams, - &bind_array.values_[0], - &bind_array.lengths_[0], - &bind_array.formats_[0], 0); + PgSqlResult r(PQexecPrepared(conn_, tagged_statements[stindex].name, + tagged_statements[stindex].nbparams, + &bind_array.values_[0], + &bind_array.lengths_[0], + &bind_array.formats_[0], 0)); conn_.checkStatementError(r, tagged_statements[stindex]); int affected_rows = boost::lexical_cast(PQcmdTuples(r)); - PQclear(r); // Check success case first as it is the most likely outcome. if (affected_rows == 1) { @@ -1165,15 +1158,14 @@ PgSqlLeaseMgr::updateLease6(const Lease6Ptr& lease) { uint64_t PgSqlLeaseMgr::deleteLeaseCommon(StatementIndex stindex, PsqlBindArray& bind_array) { - PGresult* r = PQexecPrepared(conn_, tagged_statements[stindex].name, - tagged_statements[stindex].nbparams, - &bind_array.values_[0], - &bind_array.lengths_[0], - &bind_array.formats_[0], 0); + PgSqlResult r(PQexecPrepared(conn_, tagged_statements[stindex].name, + tagged_statements[stindex].nbparams, + &bind_array.values_[0], + &bind_array.lengths_[0], + &bind_array.formats_[0], 0)); conn_.checkStatementError(r, tagged_statements[stindex]); int affected_rows = boost::lexical_cast(PQcmdTuples(r)); - PQclear(r); return (affected_rows); } @@ -1253,7 +1245,7 @@ PgSqlLeaseMgr::getVersion() const { LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_PGSQL_GET_VERSION); - PGresult* r = PQexecPrepared(conn_, "get_version", 0, NULL, NULL, NULL, 0); + PgSqlResult r(PQexecPrepared(conn_, "get_version", 0, NULL, NULL, NULL, 0)); conn_.checkStatementError(r, tagged_statements[GET_VERSION]); istringstream tmp; @@ -1267,8 +1259,6 @@ PgSqlLeaseMgr::getVersion() const { tmp.str(PQgetvalue(r, 0, 1)); tmp >> minor; - PQclear(r); - return make_pair(version, minor); } diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.h b/src/lib/dhcpsrv/pgsql_lease_mgr.h index 2457b1c1f0..890517f039 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.h +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.h @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2016 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -473,28 +473,6 @@ private: getLeaseCollection(stindex, bind_array, exchange6_, result); } - /// @brief Checks result of the r object - /// - /// This function is used to determine whether or not the SQL statement - /// execution succeeded, and in the event of failures, decide whether or - /// not the failures are recoverable. - /// - /// If the error is recoverable, the method will throw a DbOperationError. - /// In the error is deemed unrecoverable, such as a loss of connectivity - /// with the server, this method will log the error and call exit(-1); - /// - /// @todo Calling exit() is viewed as a short term solution for Kea 1.0. - /// Two tickets are likely to alter this behavior, first is #3639, which - /// calls for the ability to attempt to reconnect to the database. The - /// second ticket, #4087 which calls for the implementation of a generic, - /// FatalException class which will propagate outward. - /// - /// @param r result of the last PostgreSQL operation - /// @param index will be used to print out compiled statement name - /// - /// @throw isc::dhcp::DbOperationError Detailed PostgreSQL failure - void checkStatementError(PGresult*& r, StatementIndex index) const; - /// @brief Get Lease4 Common Code /// /// This method performs the common actions for the various getLease4() @@ -571,7 +549,8 @@ private: /// /// @throw isc::dhcp::DbOperationError An operation on the open database has /// failed. - uint64_t deleteLeaseCommon(StatementIndex stindex, PsqlBindArray& bind_array); + uint64_t deleteLeaseCommon(StatementIndex stindex, + PsqlBindArray& bind_array); /// @brief Delete expired-reclaimed leases. ///