From 9c28427af0330d969eae0a86caea2519b7977ebb Mon Sep 17 00:00:00 2001 From: Yohann Ferreira Date: Sun, 14 Nov 2010 17:47:12 +0100 Subject: Simplified the use of binding when using MySQL. This permits to avoid a memleak with the former vector form and to use the 'place' variable when binding. The badly prepared statements are also a bit better handled now. With this patch, IMHO, the MySQL support is in shape. Reviewed-by: Jaxad0127. --- src/dal/mysqldataprovider.cpp | 109 +++++++++++++++++++++++++----------------- src/dal/mysqldataprovider.h | 20 ++++++-- 2 files changed, 82 insertions(+), 47 deletions(-) diff --git a/src/dal/mysqldataprovider.cpp b/src/dal/mysqldataprovider.cpp index d0c488a8..d297c95b 100644 --- a/src/dal/mysqldataprovider.cpp +++ b/src/dal/mysqldataprovider.cpp @@ -42,8 +42,10 @@ const std::string MySqlDataProvider::CFGPARAM_MYSQL_PWD_DEF = "mana"; */ MySqlDataProvider::MySqlDataProvider() throw() - : mDb(0) - , mInTransaction(false) + : mDb(0), + mStmt(0), + mBind(0), + mInTransaction(false) { } @@ -127,6 +129,7 @@ void MySqlDataProvider::connect() // Save the Db Name. mDbName = dbName; + // Initialize statement structure mStmt = mysql_stmt_init(mDb); mIsConnected = true; @@ -207,6 +210,7 @@ void MySqlDataProvider::disconnect() // handle allocated by mysql_init(). mysql_close(mDb); + // Clean possible statement. mysql_stmt_close(mStmt); // deinitialize the MySQL client library. @@ -222,20 +226,21 @@ void MySqlDataProvider::beginTransaction() if (!mIsConnected) { const std::string error = "Trying to begin a transaction while not " - "connected to the database!"; + "connected to the database!"; LOG_ERROR(error); throw std::runtime_error(error); } if (inTransaction()) { - const std::string error = "Trying to begin a transaction while anoter " - "one is still open!"; + const std::string error = "Trying to begin a transaction while another " + "one is still open!"; LOG_ERROR(error); throw std::runtime_error(error); } - if (mysql_autocommit(mDb, AUTOCOMMIT_OFF)) { + if (mysql_autocommit(mDb, AUTOCOMMIT_OFF)) + { const std::string error = "Error while trying to disable autocommit"; LOG_ERROR(error); throw std::runtime_error(error); @@ -252,7 +257,7 @@ void MySqlDataProvider::commitTransaction() if (!mIsConnected) { const std::string error = "Trying to commit a transaction while not " - "connected to the database!"; + "connected to the database!"; LOG_ERROR(error); throw std::runtime_error(error); } @@ -260,7 +265,7 @@ void MySqlDataProvider::commitTransaction() if (!inTransaction()) { const std::string error = "Trying to commit a transaction while no " - "one is open!"; + "one is open!"; LOG_ERROR(error); throw std::runtime_error(error); } @@ -271,7 +276,8 @@ void MySqlDataProvider::commitTransaction() throw DbSqlQueryExecFailure(mysql_error(mDb)); } - if (mysql_autocommit(mDb, AUTOCOMMIT_ON)) { + if (mysql_autocommit(mDb, AUTOCOMMIT_ON)) + { const std::string error = "Error while trying to enable autocommit"; LOG_ERROR(error); throw std::runtime_error(error); @@ -287,7 +293,7 @@ void MySqlDataProvider::rollbackTransaction() if (!mIsConnected) { const std::string error = "Trying to rollback a transaction while not " - "connected to the database!"; + "connected to the database!"; LOG_ERROR(error); throw std::runtime_error(error); } @@ -295,16 +301,18 @@ void MySqlDataProvider::rollbackTransaction() if (!inTransaction()) { const std::string error = "Trying to rollback a transaction while no " - "one is open!"; + "one is open!"; LOG_ERROR(error); throw std::runtime_error(error); } if (mysql_rollback(mDb) != 0) { - LOG_ERROR("MySqlDataProvider::rollbackTransaction: " << mysql_error(mDb)); + LOG_ERROR("MySqlDataProvider::rollbackTransaction: " + << mysql_error(mDb)); throw DbSqlQueryExecFailure(mysql_error(mDb)); } + mysql_autocommit(mDb, AUTOCOMMIT_ON); mInTransaction = false; LOG_DEBUG("SQL: transaction rolled back"); @@ -320,17 +328,16 @@ unsigned MySqlDataProvider::getModifiedRows() const if (!mIsConnected) { const std::string error = "Trying to getModifiedRows while not " - "connected to the database!"; + "connected to the database!"; LOG_ERROR(error); throw std::runtime_error(error); } - // FIXME: not sure if this is correct to bring 64bit int into int? const my_ulonglong affected = mysql_affected_rows(mDb); if (affected > INT_MAX) throw std::runtime_error( - "MySqlDataProvider::getLastId exceeded INT_MAX"); + "MySqlDataProvider::getLastId exceeded INT_MAX"); if (affected == (my_ulonglong)-1) { @@ -350,10 +357,10 @@ unsigned MySqlDataProvider::getLastId() const throw std::runtime_error(error); } - // FIXME: not sure if this is correct to bring 64bit int into int? const my_ulonglong lastId = mysql_insert_id(mDb); if (lastId > UINT_MAX) - throw std::runtime_error("MySqlDataProvider::getLastId exceeded INT_MAX"); + throw std::runtime_error( + "MySqlDataProvider::getLastId exceeded UINT_MAX"); return (unsigned) lastId; } @@ -363,45 +370,48 @@ bool MySqlDataProvider::prepareSql(const std::string &sql) if (!mIsConnected) return false; - LOG_DEBUG("MySqlDataProvider::prepareSql Preparing SQL statement: "<buffer_type; - paramsBind[i].buffer = mBind[i]->buffer; - paramsBind[i].buffer_length = mBind[i]->buffer_length; - paramsBind[i].is_null = 0; - paramsBind[i].length = mBind[i]->length; + // Since we'll have to return something in all cases, + // we clear the result member first. + mRecordSet.clear(); + + if (!mBind) + { + LOG_ERROR("MySqlDataProvider::processSql: " + "No bind done before processing."); + return mRecordSet; } - if (mysql_stmt_bind_param(mStmt, paramsBind)) + if (mysql_stmt_bind_param(mStmt, mBind)) { LOG_ERROR("MySqlDataProvider::processSql Bind params failed: " << mysql_stmt_error(mStmt)); + return mRecordSet; } if (mysql_stmt_field_count(mStmt) > 0) { - mRecordSet.clear(); MYSQL_BIND* resultBind; MYSQL_RES* res; @@ -420,6 +430,7 @@ const RecordSet &MySqlDataProvider::processSql() resultBind = new MYSQL_BIND[mysql_num_fields(res)]; + unsigned int i = 0; for (i = 0; i < mysql_num_fields(res); ++i) { resultBind[i].buffer_type = MYSQL_TYPE_STRING; @@ -450,7 +461,7 @@ const RecordSet &MySqlDataProvider::processSql() { Row r; - for (unsigned int i = 0; i < nFields; ++i) + for (i = 0; i < nFields; ++i) r.push_back(static_cast(resultBind[i].buffer)); mRecordSet.add(r); @@ -462,12 +473,12 @@ const RecordSet &MySqlDataProvider::processSql() { if (mysql_stmt_execute(mStmt)) { - LOG_ERROR("MySqlDataProvider::processSql Execute failed: " << mysql_stmt_error(mStmt)); + LOG_ERROR("MySqlDataProvider::processSql Execute failed: " + << mysql_stmt_error(mStmt)); } } - // free memory - delete[] paramsBind; + // Free memory mysql_stmt_free_result(mStmt); return mRecordSet; @@ -482,9 +493,9 @@ void MySqlDataProvider::bindValue(int place, const std::string &value) bind->buffer= (void*) value.c_str(); bind->buffer_length= value.size(); bind->length = size; + bind->is_null = 0; - //FIXME : Isn't taking care of the place param - mBind.push_back(bind); + bindValue(place, bind); } void MySqlDataProvider::bindValue(int place, int value) @@ -492,9 +503,21 @@ void MySqlDataProvider::bindValue(int place, int value) MYSQL_BIND* bind = new MYSQL_BIND; bind->buffer_type= MYSQL_TYPE_LONG; bind->buffer= &value; + bind->is_null = 0; + + bindValue(place, bind); +} - //FIXME : Isn't taking care of the place param - mBind.push_back(bind); +void MySqlDataProvider::bindValue(int place, MYSQL_BIND* bind) +{ + if (!mBind) + LOG_ERROR("MySqlDataProvider::bindValue: " + "Attempted to use an unprepared bind!"); + else if (place > 0 && place <= (int)mysql_stmt_param_count(mStmt)) + mBind[place - 1] = *bind; + else + LOG_ERROR("MySqlDataProvider::bindValue: " + "Attempted bind index out of range"); } } // namespace dal diff --git a/src/dal/mysqldataprovider.h b/src/dal/mysqldataprovider.h index 02fc8ea8..4850a146 100644 --- a/src/dal/mysqldataprovider.h +++ b/src/dal/mysqldataprovider.h @@ -47,7 +47,8 @@ class MySqlDataProvider: public DataProvider * Replacement for mysql my_bool datatype used in mysql_autocommit() * function. */ - enum { + enum + { AUTOCOMMIT_OFF = 0, AUTOCOMMIT_ON = 1 }; @@ -173,6 +174,13 @@ class MySqlDataProvider: public DataProvider void bindValue(int place, int value); private: + /** + * Bind Value (MYSQL_BIND*) + * @param place - which parameter to bind to + * @param value - the MySQL bind structure itself + */ + void bindValue(int place, MYSQL_BIND* bind); + /** defines the name of the hostname config parameter */ static const std::string CFGPARAM_MYSQL_HOST; /** defines the name of the server port config parameter */ @@ -196,9 +204,13 @@ class MySqlDataProvider: public DataProvider static const std::string CFGPARAM_MYSQL_PWD_DEF; - MYSQL *mDb; /**< the handle to the database connection */ - MYSQL_STMT *mStmt; /**< the prepared statement to process */ - std::vector mBind; + /** The handle to the database connection */ + MYSQL *mDb; + /** The prepared statement to process */ + MYSQL_STMT *mStmt; + /** The Bind structure used in prepared statement */ + MYSQL_BIND* mBind; + /** Tells whether we're in the middle of a transaction */ bool mInTransaction; }; -- cgit v1.2.3-60-g2f50