Hi Srinivas, The vector just has elements when lgs check the sequence of changed request contained in CCB. And then remove all elements after the loop. So when exiting the function ccbCompletedCallback, the vector always is empty.
In case switchover as you said, lgs will always completed this job before polling to get any events(requests). Thanks Canh -----Original Message----- From: Srinivas Mangipudy [mailto:[email protected]] Sent: Wednesday, January 17, 2018 7:44 PM To: Canh Van Truong <[email protected]>; [email protected]; [email protected]; [email protected] Cc: [email protected] Subject: RE: [PATCH 1/1] log: fix to reject creating log streams with same file name in same CCB [#2752] Hi Canh, Ack from my end, but I suspect a potential memory leak. Say we are creating many log streams in a single CCB operation and there is a switch over, then we will not delete the elements in the vector, since we will call "goto done" and the memory is cleared before the "done" label. if (lgs_cb->ha_state != SA_AMF_HA_ACTIVE) { TRACE("State Not Active. Nothing to do, we are an applier"); goto done; } Thank you Srinivas -----Original Message----- From: Canh Van Truong [mailto:[email protected]] Sent: Thursday, January 11, 2018 3:04 PM To: [email protected]; [email protected]; [email protected] Cc: [email protected]; Canh Van Truong <[email protected]> Subject: [PATCH 1/1] log: fix to reject creating log streams with same file name in same CCB [#2752] LOG server should not allow creating streams which have the same target local log file --- src/log/logd/lgs_imm.cc | 49 +++++++++++++++++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/src/log/logd/lgs_imm.cc b/src/log/logd/lgs_imm.cc index 2c918555b..84411fc0e 100644 --- a/src/log/logd/lgs_imm.cc +++ b/src/log/logd/lgs_imm.cc @@ -82,6 +82,11 @@ static const SaImmClassNameT logConfig_str = static const SaImmClassNameT streamConfig_str = const_cast<SaImmClassNameT>("SaLogStreamConfig"); +// The list contains path-file names when validating its name in CCB +completed // callback. This help for log service prevents that creating +any streams // in same CCB with duplicated log path-file name +std::vector<std::string *> file_path_list; + /* FUNCTIONS * --------- */ @@ -1034,8 +1039,8 @@ static lgs_stream_defval_t *get_SaLogStreamConfig_default() { } /** - * Check if a stream with the same file name and relative path already - * exist + * Check if file name and relative path of new stream are duplicated + with + * a stream that already exists or in same CCB * * @param immOiHandle * @param fileName @@ -1044,9 +1049,10 @@ static lgs_stream_defval_t *get_SaLogStreamConfig_default() { * @param operationType * @return true if exists */ -bool chk_filepath_stream_exist(std::string &fileName, std::string &pathName, - log_stream_t *stream, - enum CcbUtilOperationType operationType) { +static bool check_duplicated_file_path( + std::string &fileName, std::string &pathName, + log_stream_t *stream, + enum CcbUtilOperationType operationType) { log_stream_t *i_stream = NULL; std::string i_fileName; std::string i_pathName; @@ -1097,9 +1103,18 @@ bool chk_filepath_stream_exist(std::string &fileName, std::string &pathName, osafassert(0); } - /* Check if any stream has given filename and path */ - TRACE("Check if any stream has given filename and path"); - // Iterate all existing log streams in cluster. + TRACE("Check if filename and pathname are duplicated"); + + // Check if any streams in the same CCB has given filename and + pathname for (auto file_path : file_path_list) { + if (*file_path == (i_pathName + i_fileName)) { + return true; + } + } + std::string *file_path_name = new std::string(i_pathName + + i_fileName); file_path_list.push_back(file_path_name); + + // Check if any current streams has given filename and pathname SaBoolT endloop = SA_FALSE, jstart = SA_TRUE; while ((i_stream = iterate_all_streams(endloop, jstart)) && !endloop) { jstart = SA_FALSE; @@ -1562,13 +1577,13 @@ static SaAisErrorT check_attr_validity( goto done; } - if (chk_filepath_stream_exist(i_fileName, i_pathName, stream, - opdata->operationType)) { + if (check_duplicated_file_path(i_fileName, i_pathName, stream, + opdata->operationType)) { report_oi_error(immOiHandle, opdata->ccbId, - "Path/file %s/%s already exist", i_pathName.c_str(), + "Path/file %s/%s is duplicated", + i_pathName.c_str(), i_fileName.c_str()); rc = SA_AIS_ERR_BAD_OPERATION; - TRACE("Path/file %s/%s already exist", i_pathName.c_str(), + TRACE("Path/file %s/%s is duplicated", i_pathName.c_str(), i_fileName.c_str()); goto done; } @@ -1864,6 +1879,15 @@ static SaAisErrorT ccbCompletedCallback(SaImmOiHandleT immOiHandle, assert(0); break; } + + if (rc != SA_AIS_OK) break; + } + + // Deleting all elements in "file_path_list" list for (auto it = + file_path_list.begin(); it != file_path_list.end();) { + std::string *name = *it; + it = file_path_list.erase(it); + delete name; } done: @@ -1875,6 +1899,7 @@ done: TRACE_LEAVE2("rc = %u", cb_rc); return cb_rc; } + /** * Set logRootDirectory to new value * - Close all open logfiles -- 2.15.1 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
