From 7ebcff8fdbd9f046b92efa1535452cbc6ef4f641 Mon Sep 17 00:00:00 2001 From: Razvan Becheriu Date: Sun, 7 Apr 2019 12:48:44 +0300 Subject: [PATCH] fixed memory leak and log error messages --- src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc | 9 --------- src/lib/dhcpsrv/mysql_host_data_source.cc | 21 ++++----------------- src/lib/dhcpsrv/mysql_lease_mgr.cc | 10 ++++++---- src/lib/dhcpsrv/pgsql_host_data_source.cc | 10 ++++++---- src/lib/dhcpsrv/pgsql_lease_mgr.cc | 2 +- 5 files changed, 17 insertions(+), 35 deletions(-) diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc index c7c81c4007..1f1b6e4b37 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc @@ -60,15 +60,6 @@ MySqlConfigBackendImpl(const DatabaseConnection::ParameterMap& parameters) } MySqlConfigBackendImpl::~MySqlConfigBackendImpl() { - // Free up the prepared statements, ignoring errors. (What would we do - // about them? We're destroying this object and are not really concerned - // with errors on a database connection that is about to go away.) - for (int i = 0; i < conn_.handle().statements_.size(); ++i) { - if (conn_.handle().statements_[i] != NULL) { - (void) mysql_stmt_close(conn_.handle().statements_[i]); - conn_.handle().statements_[i] = NULL; - } - } } MySqlBindingPtr diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc index dc987749d2..0c263ec54b 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.cc +++ b/src/lib/dhcpsrv/mysql_host_data_source.cc @@ -2455,18 +2455,6 @@ MySqlHostDataSourceImpl(const MySqlConnection::ParameterMap& parameters) } MySqlHostDataSourceImpl::~MySqlHostDataSourceImpl() { - // Free up the prepared statements, ignoring errors. (What would we do - // about them? We're destroying this object and are not really concerned - // with errors on a database connection that is about to go away.) - for (int i = 0; i < conn_.handle().statements_.size(); ++i) { - if (conn_.handle().statements_[i] != NULL) { - (void) mysql_stmt_close(conn_.handle().statements_[i]); - conn_.handle().statements_[i] = NULL; - } - } - - // There is no need to close the database in this destructor: it is - // closed in the destructor of the mysql_ member variable. } std::pair @@ -2487,14 +2475,14 @@ MySqlHostDataSourceImpl::getVersion() const { if (status != 0) { mysql_stmt_close(stmt); isc_throw(DbOperationError, "unable to prepare MySQL statement <" - << version_sql << ">, reason: " << mysql_errno(conn_.handle())); + << version_sql << ">, reason: " << mysql_error(conn_.handle())); } // Execute the prepared statement. if (mysql_stmt_execute(stmt) != 0) { mysql_stmt_close(stmt); isc_throw(DbOperationError, "cannot execute schema version query <" - << version_sql << ">, reason: " << mysql_errno(conn_.handle())); + << version_sql << ">, reason: " << mysql_error(conn_.handle())); } // Bind the output of the statement to the appropriate variables. @@ -2515,14 +2503,14 @@ MySqlHostDataSourceImpl::getVersion() const { if (mysql_stmt_bind_result(stmt, bind)) { isc_throw(DbOperationError, "unable to bind result set for <" - << version_sql << ">, reason: " << mysql_errno(conn_.handle())); + << version_sql << ">, reason: " << mysql_error(conn_.handle())); } // Fetch the data. if (mysql_stmt_fetch(stmt)) { mysql_stmt_close(stmt); isc_throw(DbOperationError, "unable to bind result set for <" - << version_sql << ">, reason: " << mysql_errno(conn_.handle())); + << version_sql << ">, reason: " << mysql_error(conn_.handle())); } // Discard the statement and its resources @@ -2531,7 +2519,6 @@ MySqlHostDataSourceImpl::getVersion() const { return (std::make_pair(major, minor)); } - void MySqlHostDataSourceImpl::addStatement(StatementIndex stindex, std::vector& bind) { diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc index a11ebf62c6..bf79c7476b 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.cc +++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc @@ -2719,21 +2719,23 @@ MySqlLeaseMgr::getVersion() const { MYSQL_STMT *stmt = mysql_stmt_init(conn_.handle()); if (stmt == NULL) { isc_throw(DbOperationError, "unable to allocate MySQL prepared " - "statement structure, reason: " << mysql_error(conn_.handle())); + "statement structure, reason: " << mysql_error(conn_.handle())); } // Prepare the statement from SQL text. const char* version_sql = "SELECT version, minor FROM schema_version"; int status = mysql_stmt_prepare(stmt, version_sql, strlen(version_sql)); if (status != 0) { + mysql_stmt_close(stmt); isc_throw(DbOperationError, "unable to prepare MySQL statement <" << version_sql << ">, reason: " << mysql_error(conn_.handle())); } // Execute the prepared statement. if (mysql_stmt_execute(stmt) != 0) { + mysql_stmt_close(stmt); isc_throw(DbOperationError, "cannot execute schema version query <" - << version_sql << ">, reason: " << mysql_errno(conn_.handle())); + << version_sql << ">, reason: " << mysql_error(conn_.handle())); } // Bind the output of the statement to the appropriate variables. @@ -2754,14 +2756,14 @@ MySqlLeaseMgr::getVersion() const { if (mysql_stmt_bind_result(stmt, bind)) { isc_throw(DbOperationError, "unable to bind result set for <" - << version_sql << ">, reason: " << mysql_errno(conn_.handle())); + << version_sql << ">, reason: " << mysql_error(conn_.handle())); } // Fetch the data. if (mysql_stmt_fetch(stmt)) { mysql_stmt_close(stmt); isc_throw(DbOperationError, "unable to bind result set for <" - << version_sql << ">, reason: " << mysql_errno(conn_.handle())); + << version_sql << ">, reason: " << mysql_error(conn_.handle())); } // Discard the statement and its resources diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.cc b/src/lib/dhcpsrv/pgsql_host_data_source.cc index 363a009f1f..9cf7f7756b 100644 --- a/src/lib/dhcpsrv/pgsql_host_data_source.cc +++ b/src/lib/dhcpsrv/pgsql_host_data_source.cc @@ -2013,9 +2013,11 @@ getHost(const SubnetID& subnet_id, return (result); } -std::pair PgSqlHostDataSourceImpl::getVersion() const { +pair +PgSqlHostDataSourceImpl::getVersion() const { LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_PGSQL_HOST_DB_GET_VERSION); + const char* version_sql = "SELECT version, minor FROM schema_version;"; PgSqlResult r(PQexec(conn_.handle(), version_sql)); if(PQresultStatus(r) != PGRES_TUPLES_OK) { @@ -2023,13 +2025,13 @@ std::pair PgSqlHostDataSourceImpl::getVersion() const { << version_sql << ">, reason: " << PQerrorMessage(conn_.handle())); } - uint32_t version; - PgSqlExchange::getColumnValue(r, 0, 0, version); + uint32_t major; + PgSqlExchange::getColumnValue(r, 0, 0, major); uint32_t minor; PgSqlExchange::getColumnValue(r, 0, 1, minor); - return (std::make_pair(version, minor)); + return (make_pair(major, minor)); } void diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.cc b/src/lib/dhcpsrv/pgsql_lease_mgr.cc index 45c59627ff..9fe3f19eef 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.cc +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.cc @@ -1888,7 +1888,7 @@ PgSqlLeaseMgr::getVersion() const { PgSqlResult r(PQexec(conn_.handle(), version_sql)); if(PQresultStatus(r) != PGRES_TUPLES_OK) { isc_throw(DbOperationError, "unable to execute PostgreSQL statement <" - << version_sql << ", reason: " << PQerrorMessage(conn_.handle())); + << version_sql << ">, reason: " << PQerrorMessage(conn_.handle())); } uint32_t major; -- 2.47.2