BIND 10 trac3080, updated. 6adb8b731be3a72e9a6e407ca076a7ee1810d826 [3080] Fix build and unit test errors
BIND 10 source code commits
bind10-changes at lists.isc.org
Fri Mar 28 12:54:51 UTC 2014
The branch, trac3080 has been updated
via 6adb8b731be3a72e9a6e407ca076a7ee1810d826 (commit)
via efbde0adb7a7dcff8438fc7a5209afe77b4b1b32 (commit)
from f496ea9a139b6d48277bddc1d63cbab652f40b0e (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 6adb8b731be3a72e9a6e407ca076a7ee1810d826
Author: Thomas Markwalder <tmark at isc.org>
Date: Fri Mar 28 08:31:58 2014 -0400
[3080] Fix build and unit test errors
Initial review and testing revealed build issues, unit test failures,
and memory leaks. With these changes it should build and pass unit testing.
There are still runtime issues to address.
Changed expire column type in lease tables to be "TIMESTAMP WITH TIME ZONE",
and added methods to convert to and from such fields to LeaseExchange. This
corrects mismatched time conversion to and from database which was causing unit tests to fail.
Added constructors to PgSqlParam to eliminate use of ".value" initializers and
to provide a safe, uniform way to create parameters for binary data. Prior to
this valgrind was reporting invalid reads when vectors were statically cast
to char*.
Removed superflous BOOST_STATIC_ASSERT and corrected values tested in remaining
check.
Removed use of "SET AUTOCOMMIT TO" as it is no longer supported in PostgreSQL.
Altered failure logic in PgSqlLeaseMgr::openDatabase() to release connection
if it is not NULL. This was causing memory leak in unit tests.
Added PQfinish call to createSchema() function to release the connection to fix
memory leaks during unit testing.
Cleaned most cppcheck complaints.
commit efbde0adb7a7dcff8438fc7a5209afe77b4b1b32
Author: Thomas Markwalder <tmark at isc.org>
Date: Fri Mar 28 07:17:51 2014 -0400
[3080] Fixed link flags for Kea postgresSQL backend build
Changed the definition of PGSQL_LIBS for building Kea with PostgreSQL
backend to use pg_config value for LIBDIR rather than LDFLAGS. The latter
did not build with PostgreSQL 9.3.4 on OS-X or Centos.
Added OS-X version numbers 10.9.1 and 10.9.2 to the test for setting the value
of bind10_undefined_pthread_behavior. Without this the death test for
conditional variables fails as the problem introduced in 10.9 is still there
as of 10.9.2. This is unrelated to PostgreSQL.
-----------------------------------------------------------------------
Summary of changes:
configure.ac | 8 +-
src/lib/dhcpsrv/dhcpdb_create.pgsql | 4 +-
src/lib/dhcpsrv/pgsql_lease_mgr.cc | 322 ++++++++-------------
src/lib/dhcpsrv/pgsql_lease_mgr.h | 24 +-
src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc | 7 +-
src/lib/dhcpsrv/tests/schema_pgsql_copy.h | 4 +-
6 files changed, 162 insertions(+), 207 deletions(-)
-----------------------------------------------------------------------
diff --git a/configure.ac b/configure.ac
index e70d5bb..8151420 100644
--- a/configure.ac
+++ b/configure.ac
@@ -294,7 +294,9 @@ case "$host" in
# it should be avoided to rely on 'osx_version' unless there's no
# viable alternative.
osx_version=`/usr/bin/sw_vers -productVersion`
- if [ test $osx_version = "10.9" ]; then
+ if [ test $osx_version = "10.9" \
+ -o $osx_version = "10.9.1" \
+ -o $osx_version = "10.9.2" ]; then
bind10_undefined_pthread_behavior=yes
fi
@@ -989,8 +991,8 @@ if test "$PG_CONFIG" != "" ; then
PGSQL_CPPFLAGS=`$PG_CONFIG --cppflags`
PGSQL_INCLUDEDIR=`$PG_CONFIG --includedir`
PGSQL_CPPFLAGS="$PGSQL_CPPFLAGS -I$PGSQL_INCLUDEDIR"
- PGSQL_LIBS=`$PG_CONFIG --ldflags`
- PGSQL_LIBS="$PGSQL_LIBS -lpq"
+ PGSQL_LIBS=`$PG_CONFIG --libdir`
+ PGSQL_LIBS="-L$PGSQL_LIBS -lpq"
PGSQL_VERSION=`$PG_CONFIG --version`
AC_SUBST(PGSQL_CPPFLAGS)
diff --git a/src/lib/dhcpsrv/dhcpdb_create.pgsql b/src/lib/dhcpsrv/dhcpdb_create.pgsql
index db4dc62..55485e8 100644
--- a/src/lib/dhcpsrv/dhcpdb_create.pgsql
+++ b/src/lib/dhcpsrv/dhcpdb_create.pgsql
@@ -35,7 +35,7 @@ CREATE TABLE lease4 (
hwaddr BYTEA, -- Hardware address
client_id BYTEA, -- Client ID
valid_lifetime BIGINT, -- Length of the lease (seconds)
- expire TIMESTAMP, -- Expiration time of the lease
+ expire TIMESTAMP WITH TIME ZONE, -- Expiration time of the lease
subnet_id BIGINT, -- Subnet identification
fqdn_fwd BOOLEAN, -- Has forward DNS update been performed by a server
fqdn_rev BOOLEAN, -- Has reverse DNS update been performed by a server
@@ -57,7 +57,7 @@ CREATE TABLE lease6 (
address VARCHAR(39) PRIMARY KEY NOT NULL, -- IPv6 address
duid BYTEA, -- DUID
valid_lifetime BIGINT, -- Length of the lease (seconds)
- expire TIMESTAMP, -- Expiration time of the lease
+ expire TIMESTAMP WITH TIME ZONE, -- Expiration time of the lease
subnet_id BIGINT, -- Subnet identification
pref_lifetime BIGINT, -- Preferred lifetime
lease_type SMALLINT, -- Lease type (see lease6_types
diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.cc b/src/lib/dhcpsrv/pgsql_lease_mgr.cc
index 2702236..a2215ef 100644
--- a/src/lib/dhcpsrv/pgsql_lease_mgr.cc
+++ b/src/lib/dhcpsrv/pgsql_lease_mgr.cc
@@ -34,16 +34,14 @@ using namespace std;
namespace {
-// Maximum text representation of an IPv6 address
-const size_t ADDRESS6_TEXT_MAX_LEN = 39;
-
// Maximum number of parameters used in any signle query
-const size_t MAX_PARAMETERS_IN_QUERY = 12;
+const size_t MAX_PARAMETERS_IN_QUERY = 13;
// Defines a single query
struct TaggedStatement {
/// Query index
+ /// @todo cppcheck flags index as unused
PgSqlLeaseMgr::StatementIndex index;
/// Number of parameters for a given query
@@ -54,7 +52,7 @@ struct TaggedStatement {
/// Sspecify parameter types. See /usr/include/postgresql/catalog/pg_type.h.
/// For some reason that header does not export those parameters.
/// Those OIDs must match both input and output parameters.
- const Oid types[MAX_PARAMETERS_IN_QUERY + 1];
+ const Oid types[MAX_PARAMETERS_IN_QUERY];
/// Short name of the query.
const char* name;
@@ -186,23 +184,47 @@ namespace dhcp {
class PgSqlLeaseExchange {
protected:
- /// Converts time_t structure to a text representation
- /// @param expire timestamp to be converted
- /// @param buffer text version will be written here
- void
- convertToTimestamp(const time_t& expire, char buffer[20]) {
+ /// Converts time_t structure to a text representation in local time.
+ ///
+ /// The format of the output string is "%Y-%m-%d %H:%M:%S". Database
+ /// table columns using this value should be typed as TIMESTAMP WITH
+ /// TIME ZONE. For such columns Postgres assumes input strings without
+ /// timezones should be treated as in local time and are converted to UTC
+ /// when stored. Likewise, these columns are automatically adjusted
+ /// upon retrieval unless fetched via "extract(epoch from <column>))".
+ ///
+ /// @param time_val timestamp to be converted
+ /// @return std::string containing the stringified time
+ std::string
+ convertToDatabaseTime(const time_t& time_val) {
struct tm tinfo;
- localtime_r(&expire, &tinfo);
- strftime(buffer, 20, "%Y-%m-%d %H:%M:%S", &tinfo);
+ char buffer[20];
+ localtime_r(&time_val, &tinfo);
+ strftime(buffer, sizeof(buffer), "%Y-%m-%d %H:%M:%S", &tinfo);
+ return (std::string(buffer));
}
- /// Converts available text representations to bool
+ /// Converts time stamp from the database to a time_t
+ /// @param db_time_val timestamp to be converted. This value
+ /// is expected to be the number of seconds since the epoch
+ /// expressed as base-10 integer string.
+ time_t convertFromDatabaseTime(const std::string& db_time_val) {
+ // Convert string time value to time_t
+ istringstream tmp;
+ time_t db_time_t;
+ tmp.str(db_time_val);
+ tmp >> db_time_t;
+ return (db_time_t);
+ }
+
+ /// Converts Postgres text boolean representations to bool
///
- /// Allowed values are "t" or "f". Any other will throw.
+ /// Allowed values are "t" or "f", or "" which is false.
+ // Any other will throw.
/// @param value text value to be converted
/// @throw BadValue if passed any value other than "t" or "f"
bool stringToBool(char* value) {
- if (!strlen(value)) {
+ if (!value || !strlen(value)) {
return (false);
}
switch (value[0]) {
@@ -226,7 +248,7 @@ class PgSqlLease4Exchange : public PgSqlLeaseExchange {
public:
/// Default constructor
- PgSqlLease4Exchange() : addr4_(0) {
+ PgSqlLease4Exchange() : addr4_(0), hwaddr_length_(0), client_id_length_(0) {
memset(hwaddr_buffer_, 0, sizeof(hwaddr_buffer_));
memset(client_id_buffer_, 0, sizeof(client_id_buffer_));
@@ -253,11 +275,8 @@ public:
ostringstream tmp;
tmp << static_cast<uint32_t>(lease_->addr_);
- PgSqlParam paddr4 = { .value = tmp.str() };
-
- params.push_back(paddr4);
+ params.push_back(PgSqlParam(tmp.str()));
tmp.str("");
- tmp.clear();
// Although HWADDR object will always be there, it may be just an empty vector
if (!lease_->hwaddr_.empty()) {
@@ -267,50 +286,30 @@ public:
<< HWAddr::MAX_HWADDR_LEN);
}
- PgSqlParam pdest = { .value = string(lease_->hwaddr_.begin(),
- lease_->hwaddr_.end()),
- .isbinary = 1,
- .binarylen = static_cast<int>(lease_->hwaddr_.size()) };
- params.push_back(pdest);
+ params.push_back(PgSqlParam(lease_->hwaddr_));
} else {
params.push_back(PgSqlParam());
}
- if(lease_->client_id_) {
- vector<uint8_t> client_data = lease_->client_id_->getClientId();
- PgSqlParam pclient_dest = { .value = reinterpret_cast<char *>(&client_data[0]),
- .isbinary = 1,
- .binarylen = static_cast<int>(lease_->client_id_->getClientId().size()) };
- params.push_back(pclient_dest);
+ if (lease_->client_id_) {
+ params.push_back(PgSqlParam(lease_->client_id_->getClientId()));
} else {
params.push_back(PgSqlParam());
}
- string valid_lft_str;
tmp << static_cast<unsigned long>(lease_->valid_lft_);
- PgSqlParam pvalid_lft = { .value = tmp.str() };
- params.push_back(pvalid_lft);
+ params.push_back(PgSqlParam(tmp.str()));
tmp.str("");
- tmp.clear();
- time_t expire_ = lease_->valid_lft_ + lease_->cltt_;
- char buffer_[20] = { 0 };
- convertToTimestamp(expire_, buffer_);
- PgSqlParam pbuffer = { .value = buffer_ };
- params.push_back(pbuffer);
- string subnet_id_str;
- tmp << static_cast<unsigned long>(lease_->subnet_id_);
- PgSqlParam psubnet_id = { .value = tmp.str() };
- params.push_back(psubnet_id);
- PgSqlParam fqdn_fwd = { .value = lease_->fqdn_fwd_?"TRUE":"FALSE" };
- PgSqlParam fqdn_rev = { .value = lease_->fqdn_fwd_?"TRUE":"FALSE" };
- PgSqlParam hostname = { .value = lease_->hostname_ };
+ time_t expire = lease_->valid_lft_ + lease_->cltt_;
+ params.push_back(PgSqlParam(convertToDatabaseTime(expire)));
- params.push_back(fqdn_fwd);
- params.push_back(fqdn_rev);
- params.push_back(hostname);
+ tmp << static_cast<unsigned long>(lease_->subnet_id_);
+ params.push_back(PgSqlParam(tmp.str()));
- BOOST_STATIC_ASSERT(8 < LEASE_COLUMNS);
+ params.push_back(PgSqlParam(lease_->fqdn_fwd_ ? "TRUE" : "FALSE"));
+ params.push_back(PgSqlParam(lease_->fqdn_rev_ ? "TRUE" : "FALSE"));
+ params.push_back(PgSqlParam(lease_->hostname_));
return (params);
}
@@ -327,7 +326,7 @@ public:
const char* valid_lifetime_str = PQgetvalue(r, line, 3);
const char* expire_str = PQgetvalue(r, line, 4);
const char* subnet_id_str = PQgetvalue(r, line, 5);
- unsigned long valid_lifetime, expire, subnet_id;
+ unsigned long valid_lifetime, subnet_id;
istringstream tmp;
tmp.str(addr4_str);
@@ -347,11 +346,7 @@ public:
tmp.clear();
valid_lifetime_ = static_cast<uint32_t>(valid_lifetime);
- tmp.str(expire_str);
- tmp >> expire;
- tmp.str("");
- tmp.clear();
- expire_ = static_cast<uint32_t>(expire);
+ expire_ = convertFromDatabaseTime(expire_str);
tmp.str(subnet_id_str);
tmp >> subnet_id;
@@ -392,7 +387,7 @@ private:
class PgSqlLease6Exchange : public PgSqlLeaseExchange {
static const size_t LEASE_COLUMNS = 12;
public:
- PgSqlLease6Exchange() {
+ PgSqlLease6Exchange() : duid_length_(0) {
memset(duid_buffer_, 0, sizeof(duid_buffer_));
// Set the column names (for error messages)
columns_[0] = "address";
@@ -407,7 +402,7 @@ public:
columns_[9] = "fqdn_fwd";
columns_[10]= "fqdn_rev";
columns_[11]= "hostname";
- BOOST_STATIC_ASSERT(8 < LEASE_COLUMNS);
+ BOOST_STATIC_ASSERT(11 < LEASE_COLUMNS);
params.reserve(LEASE_COLUMNS);
}
@@ -418,66 +413,44 @@ public:
params.clear();
ostringstream tmp;
- PgSqlParam paddr6 = { .value = lease_->addr_.toText() };
+ params.push_back(PgSqlParam(lease_->addr_.toText()));
- params.push_back(paddr6);
- vector<uint8_t> duid_data = lease_->duid_->getDuid();
- PgSqlParam pdest = { .value = string(duid_data.begin(), duid_data.end()),
- .isbinary = 1,
- .binarylen = static_cast<int>(duid_data.size()) };
+ params.push_back(PgSqlParam(lease_->duid_->getDuid()));
- params.push_back(pdest);
-
- string valid_lft_str;
tmp << static_cast<unsigned long>(lease_->valid_lft_);
- PgSqlParam pvalid_lft = { .value = tmp.str() };
- params.push_back(pvalid_lft);
+ params.push_back(PgSqlParam(tmp.str()));
tmp.str("");
tmp.clear();
- time_t expire_ = lease_->valid_lft_ + lease_->cltt_;
- char buffer_[20] = { 0 };
- convertToTimestamp(expire_, buffer_);
- PgSqlParam pbuffer = { .value = buffer_ };
- params.push_back(pbuffer);
+ time_t expire = lease_->valid_lft_ + lease_->cltt_;
+ params.push_back(PgSqlParam(convertToDatabaseTime(expire)));
tmp << static_cast<unsigned long>(lease_->subnet_id_);
- PgSqlParam psubnet_id = { .value = tmp.str() };
- params.push_back(psubnet_id);
+ params.push_back(PgSqlParam(tmp.str()));
tmp.str("");
tmp.clear();
tmp << static_cast<unsigned long>(lease_->preferred_lft_);
- PgSqlParam preferred_lft = { .value = tmp.str() };
- params.push_back(preferred_lft);
+ params.push_back(PgSqlParam(tmp.str()));
tmp.str("");
tmp.clear();
tmp << static_cast<unsigned int>(lease_->type_);
- PgSqlParam type = { .value = tmp.str() };
- params.push_back(type);
+ params.push_back(PgSqlParam(tmp.str()));
tmp.str("");
tmp.clear();
tmp << static_cast<unsigned long>(lease_->iaid_);
- PgSqlParam iaid = { .value = tmp.str() };
- params.push_back(iaid);
+ params.push_back(PgSqlParam(tmp.str()));
tmp.str("");
tmp.clear();
tmp << static_cast<unsigned int>(lease_->prefixlen_);
- PgSqlParam prefixlen = { .value = tmp.str() };
- params.push_back(prefixlen);
-
- PgSqlParam fqdn_fwd = { .value = lease_->fqdn_fwd_?"TRUE":"FALSE" };
- PgSqlParam fqdn_rev = { .value = lease_->fqdn_rev_?"TRUE":"FALSE" };
- PgSqlParam hostname = { .value = lease_->hostname_ };
+ params.push_back(PgSqlParam(tmp.str()));
- params.push_back(fqdn_fwd);
- params.push_back(fqdn_rev);
- params.push_back(hostname);
-
- BOOST_STATIC_ASSERT(11 < LEASE_COLUMNS);
+ params.push_back(PgSqlParam(lease_->fqdn_fwd_ ? "TRUE" : "FALSE"));
+ params.push_back(PgSqlParam(lease_->fqdn_rev_ ? "TRUE" : "FALSE"));
+ params.push_back(PgSqlParam(lease_->hostname_));
return (params);
}
@@ -496,7 +469,7 @@ public:
const char* iaid_str = PQgetvalue(r, line, 7);
const char* prefixlen_str = PQgetvalue(r, line, 8);
unsigned int lease_type, prefixlen;
- unsigned long valid_lifetime, expire, subnet_id, pref_lifetime, iaid;
+ unsigned long valid_lifetime, subnet_id, pref_lifetime, iaid;
istringstream tmp;
@@ -513,11 +486,7 @@ public:
tmp.clear();
valid_lifetime_ = static_cast<uint32_t>(valid_lifetime);
- tmp.str(expire_str);
- tmp >> expire;
- tmp.str("");
- tmp.clear();
- expire_ = static_cast<uint32_t>(expire);
+ expire_ = convertFromDatabaseTime(expire_str);
tmp.str(subnet_id_str);
tmp >> subnet_id;
@@ -618,12 +587,12 @@ PgSqlLeaseMgr::~PgSqlLeaseMgr() {
if (status) {
// Attempt to deallocate prepared queries set previously with DEALLOCATE query
// No internal libpq function for that, no errors checking as well
- PGresult* r = NULL;
+ /// @todo Can't this be done as a single string with list of statements?
for(int i = 0; tagged_statements[i].text != NULL; ++ i) {
string deallocate = "DEALLOCATE \"";
deallocate += tagged_statements[i].name;
deallocate += "\"";
- r = PQexec(status, deallocate.c_str());
+ PGresult* r = PQexec(status, deallocate.c_str());
PQclear(r);
}
@@ -635,16 +604,16 @@ void PgSqlLeaseMgr::prepareStatements() {
statements_.clear();
statements_.resize(NUM_STATEMENTS, PgSqlStatementBind());
- PGresult * r = PQexec(status, "SET AUTOCOMMIT TO OFF");
- PQclear(r);
-
for(int i = 0; tagged_statements[i].text != NULL; ++ i) {
+ /// @todo why do we bother with select here? If they are already
+ /// defined we should let the error occur because we only do this
+ /// once per open anyway.
string checkstatement = "SELECT * FROM pg_prepared_statements "
"WHERE name = '";
checkstatement += tagged_statements[i].name;
checkstatement += "'";
- r = PQexec(status, checkstatement.c_str());
+ PGresult* r = PQexec(status, checkstatement.c_str());
if(!PQntuples(r)) {
PQclear(r);
@@ -674,9 +643,6 @@ void PgSqlLeaseMgr::prepareStatements() {
PQclear(r);
}
}
-
- r = PQexec(status, "SET AUTOCOMMIT TO ON");
- PQclear(r);
}
void
@@ -717,8 +683,16 @@ PgSqlLeaseMgr::openDatabase() {
}
status = PQconnectdb(dbconnparameters.c_str());
- if(status == NULL || PQstatus(status) != CONNECTION_OK) {
- isc_throw(DbOpenError, PQerrorMessage(status));
+ if (status == NULL) {
+ isc_throw(DbOpenError, "could not allocate connection object");
+ }
+
+ if (PQstatus(status) != CONNECTION_OK) {
+ // If we have a connection object, we have to call finish
+ // to release it, but grab the error message first.
+ std::string error_message = PQerrorMessage(status);
+ PQfinish(status);
+ isc_throw(DbOpenError, error_message);
}
}
@@ -789,11 +763,7 @@ void PgSqlLeaseMgr::getLeaseCollection(StatementIndex stindex,
vector<int> out_lengths;
vector<int> out_formats;
convertToQuery(params, out_values, out_lengths, out_formats);
-
- PGresult * r = PQexec(status, "SET AUTOCOMMIT TO OFF");
- PQclear(r);
-
- r = PQexec(status, "BEGIN");
+ PGresult* r = PQexec(status, "BEGIN");
PQclear(r);
r = PQexecPrepared(status, statements_[stindex].stmt_name,
@@ -833,9 +803,6 @@ void PgSqlLeaseMgr::getLeaseCollection(StatementIndex stindex,
r = PQexec(status, "END");
PQclear(r);
-
- r = PQexec(status, "SET AUTOCOMMIT TO ON");
- PQclear(r);
}
void
@@ -884,12 +851,9 @@ PgSqlLeaseMgr::getLease4(const isc::asiolink::IOAddress& addr) const {
// Set up the WHERE clause value
bindparams inparams;
ostringstream tmp;
- string baddr;
tmp << static_cast<uint32_t>(addr);
- PgSqlParam addr4 = { .value = tmp.str() };
-
- inparams.push_back(addr4);
+ inparams.push_back(PgSqlParam(tmp.str()));
// Get the data
Lease4Ptr result;
@@ -907,11 +871,7 @@ PgSqlLeaseMgr::getLease4(const HWAddr& hwaddr) const {
bindparams inparams;
if (!hwaddr.hwaddr_.empty()) {
- uint8_t* data = const_cast<uint8_t *>(&hwaddr.hwaddr_[0]);
- PgSqlParam pdest = { .value = reinterpret_cast<char *>(data),
- .isbinary = 1,
- .binarylen = static_cast<int>(hwaddr.hwaddr_.size()) };
- inparams.push_back(pdest);
+ inparams.push_back(PgSqlParam(hwaddr.hwaddr_));
} else {
inparams.push_back(PgSqlParam());
}
@@ -933,15 +893,14 @@ PgSqlLeaseMgr::getLease4(const HWAddr& hwaddr, SubnetID subnet_id) const {
bindparams inparams;
ostringstream tmp;
- PgSqlParam pdest = { .value = string(hwaddr.hwaddr_.begin(), hwaddr.hwaddr_.end()),
- .isbinary = 1,
- .binarylen = static_cast<int>(hwaddr.hwaddr_.size()) };
- inparams.push_back(pdest);
+ if (!hwaddr.hwaddr_.empty()) {
+ inparams.push_back(PgSqlParam(hwaddr.hwaddr_));
+ } else {
+ inparams.push_back(PgSqlParam());
+ }
tmp << static_cast<unsigned long>(subnet_id);
- PgSqlParam psubnet_id = { .value = tmp.str() };
-
- inparams.push_back(psubnet_id);
+ inparams.push_back(PgSqlParam(tmp.str()));
// Get the data
Lease4Ptr result;
@@ -958,11 +917,8 @@ PgSqlLeaseMgr::getLease4(const ClientId& clientid) const {
// Set up the WHERE clause value
bindparams inparams;
- vector<uint8_t> client_data = clientid.getClientId();
- PgSqlParam pdest = { .value = reinterpret_cast<char *>(&client_data[0]),
- .isbinary = 1,
- .binarylen = static_cast<int>(clientid.getClientId().size()) };
- inparams.push_back(pdest);
+ // CLIENT_ID
+ inparams.push_back(PgSqlParam(clientid.getClientId()));
// Get the data
Lease4Collection result;
@@ -981,15 +937,11 @@ PgSqlLeaseMgr::getLease4(const ClientId& clientid, SubnetID subnet_id) const {
bindparams inparams;
ostringstream tmp;
- vector<uint8_t> client_data = clientid.getClientId();
- PgSqlParam pdest = { .value = reinterpret_cast<char *>(&client_data[0]),
- .isbinary = 1,
- .binarylen = static_cast<int>(clientid.getClientId().size()) };
- inparams.push_back(pdest);
+ // CLIENT_ID
+ inparams.push_back(PgSqlParam(clientid.getClientId()));
tmp << static_cast<unsigned long>(subnet_id);
- PgSqlParam psubnet_id = { .value = tmp.str() };
- inparams.push_back(psubnet_id);
+ inparams.push_back(PgSqlParam(tmp.str()));
// Get the data
Lease4Ptr result;
@@ -1015,15 +967,16 @@ PgSqlLeaseMgr::getLease6(Lease::Type lease_type,
// Set up the WHERE clause value
bindparams inparams;
+ ostringstream tmp;
- PgSqlParam addr6 = { .value = addr.toText() };
- inparams.push_back(addr6);
+ // ADDRESS
+ inparams.push_back(PgSqlParam(addr.toText()));
- ostringstream tmp;
+ // LEASE_TYPE
tmp << static_cast<uint16_t>(lease_type);
- PgSqlParam qtype = { .value = tmp.str() };
- inparams.push_back(qtype);
+ inparams.push_back(PgSqlParam(tmp.str()));
+ // ... and get the data
Lease6Ptr result;
getLease(GET_LEASE6_ADDR, inparams, result);
@@ -1038,24 +991,19 @@ PgSqlLeaseMgr::getLeases6(Lease::Type type, const DUID& duid, uint32_t iaid) con
// Set up the WHERE clause value
bindparams inparams;
ostringstream tmp;
- vector<uint8_t> duid_data = duid.getDuid();
- PgSqlParam pdest = { .value = reinterpret_cast<char *>(&duid_data[0]),
- .isbinary = 1,
- .binarylen = static_cast<int>(duid.getDuid().size()) };
- inparams.push_back(pdest);
- /// @todo: use type
+ // DUID
+ inparams.push_back(PgSqlParam(duid.getDuid()));
// IAID
tmp << static_cast<unsigned long>(iaid);
- PgSqlParam piaid = { .value = tmp.str() };
- inparams.push_back(piaid);
+ inparams.push_back(PgSqlParam(tmp.str()));
tmp.str("");
tmp.clear();
+ // LEASE_TYPE
tmp << static_cast<uint16_t>(type);
- PgSqlParam param_lease_type = { .value = tmp.str() };
- inparams.push_back(param_lease_type);
+ inparams.push_back(PgSqlParam(tmp.str()));
// ... and get the data
Lease6Collection result;
@@ -1074,35 +1022,28 @@ PgSqlLeaseMgr::getLeases6(Lease::Type lease_type, const DUID& duid, uint32_t iai
bindparams inparams;
ostringstream tmp;
- // Lease type
+ // LEASE_TYPE
tmp << static_cast<uint16_t>(lease_type);
- PgSqlParam qtype = { .value = tmp.str() };
+ inparams.push_back(PgSqlParam(tmp.str()));
tmp.str("");
tmp.clear();
- inparams.push_back(qtype);
- // See the earlier description of the use of "const_cast" when accessing
- // the DUID for an explanation of the reason.
- vector<uint8_t> duid_data = duid.getDuid();
- PgSqlParam pdest = { .value = string(duid_data.begin(), duid_data.end()),
- .isbinary = 1,
- .binarylen = static_cast<int>(duid.getDuid().size()) };
- inparams.push_back(pdest);
+ // DUID
+ inparams.push_back(PgSqlParam(duid.getDuid()));
// IAID
tmp << static_cast<unsigned long>(iaid);
- PgSqlParam piaid = { .value = tmp.str() };
- inparams.push_back(piaid);
+ inparams.push_back(PgSqlParam(tmp.str()));
tmp.str("");
tmp.clear();
// Subnet ID
tmp << static_cast<unsigned long>(subnet_id);
- PgSqlParam psubnet_id = { .value = tmp.str() };
- inparams.push_back(psubnet_id);
+ inparams.push_back(PgSqlParam(tmp.str()));
tmp.str("");
tmp.clear();
+ // ... and get the data
Lease6Collection result;
getLeaseCollection(GET_LEASE6_DUID_IAID_SUBID, inparams, result);
@@ -1153,8 +1094,7 @@ PgSqlLeaseMgr::updateLease4(const Lease4Ptr& lease) {
// Set up the WHERE clause and append it to the MYSQL_BIND array
tmp << static_cast<uint32_t>(lease->addr_);
- PgSqlParam addr4 = { .value = tmp.str() };
- params.push_back(addr4);
+ params.push_back(PgSqlParam(tmp.str()));
// Drop to common update code
updateLeaseCommon(stindex, params, lease);
@@ -1171,8 +1111,7 @@ PgSqlLeaseMgr::updateLease6(const Lease6Ptr& lease) {
bindparams params = exchange6_->createBindForSend(lease);
// Set up the WHERE clause and append it to the MYSQL_BIND array
- PgSqlParam addr6 = { .value = lease->addr_.toText() };
- params.push_back(addr6);
+ params.push_back(PgSqlParam(lease->addr_.toText()));
// Drop to common update code
updateLeaseCommon(stindex, params, lease);
@@ -1206,19 +1145,12 @@ PgSqlLeaseMgr::deleteLease(const isc::asiolink::IOAddress& addr) {
if (addr.isV4()) {
ostringstream tmp;
tmp << static_cast<uint32_t>(addr);
- PgSqlParam addr4 = { .value = tmp.str() };
-
- inparams.push_back(addr4);
-
+ inparams.push_back(PgSqlParam(tmp.str()));
return (deleteLeaseCommon(DELETE_LEASE4, inparams));
-
- } else {
- PgSqlParam addr6 = { .value = addr.toText() };
-
- inparams.push_back(addr6);
-
- return (deleteLeaseCommon(DELETE_LEASE6, inparams));
}
+
+ inparams.push_back(PgSqlParam(addr.toText()));
+ return (deleteLeaseCommon(DELETE_LEASE6, inparams));
}
string
@@ -1272,10 +1204,7 @@ PgSqlLeaseMgr::getVersion() const {
LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
DHCPSRV_PGSQL_GET_VERSION);
- PGresult* r = PQexec(status, "SET AUTOCOMMIT TO OFF");
- PQclear(r);
-
- r = PQexec(status, "BEGIN");
+ PGresult* r = PQexec(status, "BEGIN");
PQclear(r);
r = PQexecPrepared(status, "get_version", 0, NULL, NULL, NULL, 0);
@@ -1306,9 +1235,6 @@ PgSqlLeaseMgr::getVersion() const {
r = PQexec(status, "END");
PQclear(r);
- r = PQexec(status, "SET AUTOCOMMIT TO ON");
- PQclear(r);
-
return make_pair<uint32_t, uint32_t>(version, minor);
}
diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.h b/src/lib/dhcpsrv/pgsql_lease_mgr.h
index 4043d52..9480248 100644
--- a/src/lib/dhcpsrv/pgsql_lease_mgr.h
+++ b/src/lib/dhcpsrv/pgsql_lease_mgr.h
@@ -22,6 +22,8 @@
#include <boost/utility.hpp>
#include <libpq-fe.h>
+#include <vector>
+
namespace isc {
namespace dhcp {
@@ -31,8 +33,28 @@ namespace dhcp {
/// or UPDATE clauses).
struct PgSqlParam {
std::string value; ///< The actual value represented as text
- bool isbinary; ///< Boolean flag that controls whether the data is binary
+ bool isbinary; ///< Boolean flag that indicates if data is binary
int binarylen; ///< Specified binary length
+
+ /// @brief Constructor for text parameters
+ ///
+ /// Constructs a text (i.e. non-binary) instance given a string value.
+ /// @param val string containing the text value of the parameter. The
+ /// default is an empty string which serves as the default or empty
+ /// parameter constructor.
+ PgSqlParam (const std::string& val = "")
+ : value(val), isbinary(false), binarylen(0) {
+ }
+
+ /// @brief Constructor for binary data parameters
+ ///
+ /// Constructs a binary data instance given a vector of binary data.
+ /// @param data vector of binary data from which to set the parameter's
+ /// value.
+ PgSqlParam (const std::vector<uint8_t>& data)
+ : value(data.begin(), data.end()), isbinary(true),
+ binarylen(data.size()) {
+ }
};
/// @brief Defines all parameters for binding a compiled statement
diff --git a/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc
index a7376df..653b843 100644
--- a/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc
+++ b/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc
@@ -142,6 +142,8 @@ void createSchema() {
r = PQexec(conn, create_statement[i]);
PQclear(r);
}
+
+ PQfinish(conn);
}
/// @brief Test fixture class for testing PostgreSQL Lease Manager
@@ -221,7 +223,6 @@ TEST(PgSqlOpenTest, OpenDatabase) {
<< "*** The test environment is broken and must be fixed\n"
<< "*** before the PostgreSQL tests will run correctly.\n";
}
-
// Check that attempting to get an instance of the lease manager when
// none is set throws an exception.
EXPECT_THROW(LeaseMgrFactory::instance(), NoLeaseManager);
@@ -232,6 +233,7 @@ TEST(PgSqlOpenTest, OpenDatabase) {
EXPECT_THROW(LeaseMgrFactory::create(connectionString(
NULL, VALID_NAME, VALID_HOST, INVALID_USER, VALID_PASSWORD)),
InvalidParameter);
+
EXPECT_THROW(LeaseMgrFactory::create(connectionString(
INVALID_TYPE, VALID_NAME, VALID_HOST, VALID_USER, VALID_PASSWORD)),
InvalidType);
@@ -240,12 +242,15 @@ TEST(PgSqlOpenTest, OpenDatabase) {
EXPECT_THROW(LeaseMgrFactory::create(connectionString(
VALID_TYPE, INVALID_NAME, VALID_HOST, VALID_USER, VALID_PASSWORD)),
DbOpenError);
+
EXPECT_THROW(LeaseMgrFactory::create(connectionString(
VALID_TYPE, VALID_NAME, INVALID_HOST, VALID_USER, VALID_PASSWORD)),
DbOpenError);
+
EXPECT_THROW(LeaseMgrFactory::create(connectionString(
VALID_TYPE, VALID_NAME, VALID_HOST, INVALID_USER, VALID_PASSWORD)),
DbOpenError);
+
EXPECT_THROW(LeaseMgrFactory::create(connectionString(
VALID_TYPE, VALID_NAME, VALID_HOST, VALID_USER, INVALID_PASSWORD)),
DbOpenError);
diff --git a/src/lib/dhcpsrv/tests/schema_pgsql_copy.h b/src/lib/dhcpsrv/tests/schema_pgsql_copy.h
index 9fa0cee..22cda40 100644
--- a/src/lib/dhcpsrv/tests/schema_pgsql_copy.h
+++ b/src/lib/dhcpsrv/tests/schema_pgsql_copy.h
@@ -48,7 +48,7 @@ const char* create_statement[] = {
"hwaddr BYTEA,"
"client_id BYTEA,"
"valid_lifetime BIGINT,"
- "expire TIMESTAMP,"
+ "expire TIMESTAMP WITH TIME ZONE,"
"subnet_id BIGINT,"
"fqdn_fwd BOOLEAN,"
"fqdn_rev BOOLEAN,"
@@ -59,7 +59,7 @@ const char* create_statement[] = {
"address VARCHAR(39) PRIMARY KEY NOT NULL,"
"duid BYTEA,"
"valid_lifetime BIGINT,"
- "expire TIMESTAMP,"
+ "expire TIMESTAMP WITH TIME ZONE,"
"subnet_id BIGINT,"
"pref_lifetime BIGINT,"
"lease_type SMALLINT,"
More information about the bind10-changes
mailing list