Hi Lennart, Except this '/2017-02-23 13:44:28,233 INFO - *** [src/log/logd/lgs_filehdl.cc:577]: (style) Variable 'len' is assigned a value that is never used./' all other are not related/involved with this patch, they are all existing , I try to fix them, but they involved some function changes so I prefer to be done by the developer who introduce them , so I fixed only my part of above and pushed this patch.
-AVM On 2/23/2017 7:35 PM, Lennart Lund wrote: > Hi Mahesh > > Ack with comments > > I have not found any big issues but cppcheck reports some problems. I have > tested with parent changeset 8611:15ea36eab483 > '2017-02-23 13:44:28,233 INFO - *** List of cppcheck errors added by patch > ***' > '2017-02-23 13:44:28,233 INFO - *** [src/log/logd/lgs_filehdl.cc:577]: > (style) Variable 'len' is assigned a value that is never used.' > '2017-02-23 13:44:28,233 INFO - *** [src/log/logd/lgs_stream.cc:1337]: > (style) Unsigned variable 'id' can't be negative so it is unnecessary to test > it.' > '2017-02-23 13:44:28,234 INFO - *** [src/log/logd/lgs_stream.cc:1350]: > (style) The scope of the variable 'tmp' can be reduced.' > '2017-02-23 13:44:28,234 INFO - *** [src/log/logd/lgs_stream.cc:1447]: > (style) The function 'log_stream_id_print' is never used.' > '2017-02-23 13:44:28,234 INFO - *** [src/log/logd/lgs_stream.cc:408]: > (style) The function 'log_free_stream_resources' is never used.' > '2017-02-23 13:44:28,234 INFO - *** [src/log/logd/lgs_stream.cc:416]: > (warning) Assignment of function parameter has no effect outside the > function. Did you forget dereferencing it?' > '2017-02-23 13:44:28,235 INFO - *** [src/log/logd/lgs_stream.cc:533]: > (portability) Non reentrant function 'strtok' called. For threadsafe > applications it is recommended to use the reentrant replacement function > 'strtok_r'.' > '2017-02-23 13:44:28,235 INFO - *** [src/log/logd/lgs_stream.cc:852]: > (style) Obsolescent function 'usleep' called. It is recommended to use > 'nanosleep' or 'setitimer' instead.' > '2017-02-23 13:44:28,235 INFO - *** [src/log/logd/lgs_stream.cc:869]: > (style) Obsolescent function 'usleep' called. It is recommended to use > 'nanosleep' or 'setitimer' instead.' > > Also, would it be possible to add new test case for this to logtest? > > Thanks > Lennart > >> -----Original Message----- >> From: [email protected] [mailto:[email protected]] >> Sent: den 6 februari 2017 07:42 >> To: Canh Van Truong <[email protected]>; Lennart Lund >> <[email protected]>; Vu Minh Nguyen >> <[email protected]> >> Cc: [email protected] >> Subject: [PATCH 1 of 1] logd: implement cfg files rotation [#2239] >> >> src/log/logd/lgs_file.cc | 4 + >> src/log/logd/lgs_file.h | 1 + >> src/log/logd/lgs_filehdl.cc | 182 >> ++++++++++++++++++++++++++++++++++++++++++- >> src/log/logd/lgs_filehdl.h | 2 + >> src/log/logd/lgs_stream.cc | 88 +++++++++++++++++++- >> 5 files changed, 262 insertions(+), 15 deletions(-) >> >> >> Requirement : >> >> Current implementation of LOGsv do rotation of log files as defined, but not >> for cfg files. >> So, if we play with log streams application in a long time a large numbers >> of >> cfg files are generated , >> which is causing system space issue. >> >> Implementation : >> >> SAI-AIS-LOG-A.02.01 specification not defied the approach on .cfg ROTATE >> policy, >> so it is implementation specific , this enhancement taken approach as CFG >> file can also be ROTATED concurrently >> whenever the log files with the same <closetime> suffix is being ROTATED. >> >> This enhancement ensures Until the old log files is not deleted and the old >> .cfg file is still available since it contains information about how >> to read the old log files, in other words when the last old log file has been >> rotated out also the old .cfg >> file will be removed (but not before that). >> >> Notes : README will be update if LOG developers thinks it is required >> >> diff --git a/src/log/logd/lgs_file.cc b/src/log/logd/lgs_file.cc >> --- a/src/log/logd/lgs_file.cc >> +++ b/src/log/logd/lgs_file.cc >> @@ -152,6 +152,10 @@ static void *file_hndl_thread(void *nopa >> hndl_rc = get_number_of_log_files_hdl(lgs_com_data.indata_ptr, >> lgs_com_data.outdata_ptr, >> lgs_com_data.outdata_size); >> break; >> + case LGSF_GET_NUM_CFGFILES: >> + hndl_rc = get_number_of_cfg_files_hdl(lgs_com_data.indata_ptr, >> + lgs_com_data.outdata_ptr, >> lgs_com_data.outdata_size); >> + break; >> case LGSF_MAKELOGDIR: >> hndl_rc = make_log_dir_hdl(lgs_com_data.indata_ptr, >> lgs_com_data.outdata_ptr, >> lgs_com_data.outdata_size); >> diff --git a/src/log/logd/lgs_file.h b/src/log/logd/lgs_file.h >> --- a/src/log/logd/lgs_file.h >> +++ b/src/log/logd/lgs_file.h >> @@ -44,6 +44,7 @@ typedef enum { >> LGSF_CHECKDIR, >> LGSF_OWN_LOGFILES, >> LGSF_GET_FILE_PAR, >> + LGSF_GET_NUM_CFGFILES, >> LGSF_NOREQ >> }lgsf_treq_t; >> >> diff --git a/src/log/logd/lgs_filehdl.cc b/src/log/logd/lgs_filehdl.cc >> --- a/src/log/logd/lgs_filehdl.cc >> +++ b/src/log/logd/lgs_filehdl.cc >> @@ -531,7 +531,7 @@ int delete_file_hdl(void *indata, void * >> >> /********************************************************** >> ******************** >> * Utility functions for get_number_of_log_files_hdl >> */ >> -static int check_oldest(char *line, char *fname_prefix, int >> fname_prefix_size, int *old_date, int *old_time) { >> +static int check_log_oldest(char *line, char *fname_prefix, int >> fname_prefix_size, int *old_date, int *old_time) { >> int date, time, c, d; >> date = time = c = d = 0; >> int len = 0; >> @@ -558,15 +558,50 @@ static int check_oldest(char *line, char >> return 1; >> } >> } else { >> - TRACE_3("no match"); >> + TRACE_3("no log_oldest match"); >> } >> return 0; >> } >> >> -/* Filter function used by scandir. */ >> +/********************************************************* >> ********************* >> + * Utility functions for get_number_of_cfg_files_hdl >> + */ >> +static int check_cfg_oldest(char *line, char *fname_prefix, int >> fname_prefix_size, int *old_date, int *old_time) { >> + int date, time; >> + date = time = 0; >> + int len = 0; >> + std::string name_format; >> + char time_stamps[] = "_%d_%d.cfg"; >> + >> + len = strlen(time_stamps); >> + len += fname_prefix_size; >> + >> + name_format = std::string(fname_prefix); >> + TRACE_3("fname: %s", name_format.c_str()); >> + >> + name_format = name_format + time_stamps; >> + if (sscanf(line, name_format.c_str(), &date, &time) >= 1) { >> + TRACE_3("%s: line: arg 1: %d 2: %d ok", __FUNCTION__, >> + date, time); >> + if (date < *old_date || *old_date == -1) { >> + *old_date = date; >> + *old_time = time; >> + return 1; >> + } else if ((date == *old_date) && (time < *old_time)) { >> + *old_date = date; >> + *old_time = time; >> + return 1; >> + } >> + } else { >> + TRACE_3("no cfg_oldest match"); >> + } >> + return 0; >> +} >> + >> +/* Log Filter function used by scandir. */ >> static std::string file_prefix; >> >> -static int filter_func(const struct dirent *finfo) { >> +static int log_filter_func(const struct dirent *finfo) { >> int ret; >> int filenameLen = strlen(finfo->d_name) - strlen(".log"); >> >> @@ -576,6 +611,17 @@ static int filter_func(const struct dire >> return !ret; >> } >> >> +/* Cfg Filter function used by scandir. */ >> +static int cfg_filter_func(const struct dirent *finfo) { >> + int ret; >> + int filenameLen = strlen(finfo->d_name) - strlen(".cfg"); >> + >> + ret = strncmp(file_prefix.c_str(), finfo->d_name, file_prefix.size()) >> + || strcmp(finfo->d_name + filenameLen, ".cfg"); >> + >> + return !ret; >> +} >> + >> /** >> * Return number of log files in a dir and the name of the oldest file. >> * @param indata, see gnolfh_in_t >> @@ -602,7 +648,7 @@ int get_number_of_log_files_hdl(void *in >> path = std::string(params_in->logsv_root_dir) + "/" + params_in- >>> pathName; >> osaf_mutex_unlock_ordie(&lgs_ftcom_mutex); /* UNLOCK critical section >> */ >> - files = n = scandir(path.c_str(), &namelist, filter_func, alphasort); >> + files = n = scandir(path.c_str(), &namelist, log_filter_func, alphasort); >> osaf_mutex_lock_ordie(&lgs_ftcom_mutex); /* LOCK after critical section >> */ >> >> if (n == -1 && errno == ENOENT) { >> @@ -623,7 +669,7 @@ int get_number_of_log_files_hdl(void *in >> >> while (n--) { >> TRACE_3("%s", namelist[n]->d_name); >> - if (check_oldest(namelist[n]->d_name, params_in->file_name, >> + if (check_log_oldest(namelist[n]->d_name, params_in->file_name, >> strlen(params_in->file_name), &old_date, &old_time)) { >> old_ind = n; >> } else { >> @@ -657,6 +703,128 @@ done_exit: >> } >> >> /** >> + * Return number of cfg files in a dir and the name of the oldest file. >> + * @param indata, see gnolfh_in_t >> + * @param outdata, char *oldest_file >> + * @param max_outsize, Max size for oldest_file string >> + * >> + * @return int, number of cfgfiles or -1 if error >> + */ >> +int get_number_of_cfg_files_hdl(void *indata, void *outdata, size_t >> max_outsize) { >> + struct dirent **cfg_namelist; >> + struct dirent **log_namelist; >> + int n, cfg_old_date = -1, cfg_old_time = -1, old_ind = -1, cfg_files = 0, >> i, >> failed = 0; >> + int log_old_date = -1, log_old_time = -1 , log_files = 0; >> + std::string path; >> + gnolfh_in_t *params_in; >> + char *oldest_file; >> + int rc = 0; >> + >> + TRACE_ENTER(); >> + >> + params_in = static_cast<gnolfh_in_t *>(indata); >> + oldest_file = static_cast<char *>(outdata); >> + /* Initialize the filter */ >> + file_prefix = params_in->file_name; >> + path = std::string(params_in->logsv_root_dir) + "/" + params_in- >>> pathName; >> + >> + osaf_mutex_unlock_ordie(&lgs_ftcom_mutex); /* UNLOCK critical section >> */ >> + log_files = n = scandir(path.c_str(), &log_namelist, log_filter_func, >> alphasort); >> + osaf_mutex_lock_ordie(&lgs_ftcom_mutex); /* LOCK after critical section >> */ >> + >> + if (n == -1 && errno == ENOENT) { >> + rc = 0; >> + goto done_exit; >> + } >> + >> + if (n < 0) { >> + LOG_WA("scandir:%s - %s", strerror(errno), path.c_str()); >> + rc = -1; >> + goto done_exit; >> + } >> + >> + if (n == 0) { >> + rc = log_files; >> + goto done_exit; >> + } >> + >> + while (n--) { >> + TRACE_3("%s", log_namelist[n]->d_name); >> + if (check_log_oldest(log_namelist[n]->d_name, params_in->file_name, >> + strlen(params_in->file_name), &log_old_date, >> &log_old_time)) { >> + old_ind = n; >> + } else { >> + failed++; /* wrong format */ >> + } >> + } >> + >> + if (old_ind != -1) { >> + n = 0; >> + old_ind = -1; >> + failed = 0; >> + osaf_mutex_unlock_ordie(&lgs_ftcom_mutex); /* UNLOCK critical >> section */ >> + cfg_files = n = scandir(path.c_str(), &cfg_namelist, cfg_filter_func, >> alphasort); >> + osaf_mutex_lock_ordie(&lgs_ftcom_mutex); /* LOCK after critical >> section */ >> + >> + if (n == -1 && errno == ENOENT) { >> + rc = 0; >> + goto done_log_free; >> + } >> + >> + if (n < 0) { >> + LOG_WA("scandir:%s - %s", strerror(errno), path.c_str()); >> + rc = -1; >> + goto done_log_free; >> + } >> + >> + if (n == 0) { >> + rc = cfg_files; >> + goto done_log_free; >> + } >> + >> + while (n--) { >> + if (check_cfg_oldest(cfg_namelist[n]->d_name, params_in->file_name, >> + strlen(params_in->file_name), &cfg_old_date, >> &cfg_old_time)) { >> + old_ind = n; >> + } else { >> + failed++; /* wrong format */ >> + } >> + } >> + >> + if ((old_ind != -1) && (cfg_old_date == log_old_date) && (cfg_old_time >> <= log_old_time)) { >> + TRACE_1(" (cfg_old_date:%d == log_old_date:%d) && (cfg_old_time:%d >> <= log_old_time:%d )", cfg_old_date, log_old_date, cfg_old_time, >> log_old_time); >> + TRACE_1("oldest: %s", cfg_namelist[old_ind]->d_name); >> + n = snprintf(oldest_file, max_outsize, "%s/%s", >> + path.c_str(), cfg_namelist[old_ind]->d_name); >> + >> + if (n < 0 || static_cast<uint32_t>(n) >= max_outsize) { >> + LOG_WA("oldest_file > max_outsize"); >> + rc = -1; >> + goto done_cfg_free; >> + } else { >> + rc = (cfg_files - failed); >> + } >> + } >> + } >> + >> +done_cfg_free: >> + /* Free scandir allocated memory */ >> + for (i = 0; i < cfg_files; i++) >> + free(cfg_namelist[i]); >> + free(cfg_namelist); >> + >> +done_log_free: >> + /* Free scandir allocated memory */ >> + for (i = 0; i < log_files; i++) >> + free(log_namelist[i]); >> + free(log_namelist); >> + >> +done_exit: >> + TRACE_LEAVE(); >> + return rc; >> +} >> + >> +/** >> * Change the ownership of all log files to a new group. >> * The input directory will be scanned and any file suffixed by ".log" >> * and prefixed by input file_name will be marked as log files. >> @@ -681,7 +849,7 @@ int own_log_files_by_group_hdl(void *ind >> file_prefix = params_in->file_name; >> >> osaf_mutex_unlock_ordie(&lgs_ftcom_mutex); /* UNLOCK critical section >> */ >> - files = n = scandir(params_in->dir_path, &namelist, filter_func, >> alphasort); >> + files = n = scandir(params_in->dir_path, &namelist, log_filter_func, >> alphasort); >> >> if (n == -1 && errno == ENOENT) { >> rc = 0; >> diff --git a/src/log/logd/lgs_filehdl.h b/src/log/logd/lgs_filehdl.h >> --- a/src/log/logd/lgs_filehdl.h >> +++ b/src/log/logd/lgs_filehdl.h >> @@ -73,6 +73,7 @@ typedef struct { >> >> /* >> * get_number_of_log_files_hdl(..) >> + * get_number_of_cfg_files_hdl(..) >> */ >> typedef struct { >> /* File name prefix (name part before time stamps) */ >> @@ -185,6 +186,7 @@ int fileopen_hdl(void *indata, void *out >> int fileclose_hdl(void *indata, void *outdata, size_t max_outsize); >> int delete_file_hdl(void *indata, void *outdata, size_t max_outsize); >> int get_number_of_log_files_hdl(void *indata, void *outdata, size_t >> max_outsize); >> +int get_number_of_cfg_files_hdl(void *indata, void *outdata, size_t >> max_outsize); >> int own_log_files_by_group_hdl(void *indata, void *outdata, size_t >> max_outsize); >> int lgs_get_file_params_hdl(void *indata, void *outdata, size_t >> max_outsize); >> >> diff --git a/src/log/logd/lgs_stream.cc b/src/log/logd/lgs_stream.cc >> --- a/src/log/logd/lgs_stream.cc >> +++ b/src/log/logd/lgs_stream.cc >> @@ -47,6 +47,7 @@ static int lgs_stream_array_insert(log_s >> static int lgs_stream_array_insert_new(log_stream_t *stream, uint32_t >> *id); >> static int lgs_stream_array_remove(int id); >> static int get_number_of_log_files_h(log_stream_t *logStream, char >> *oldest_file); >> +static int get_number_of_cfg_files_h(log_stream_t *logStream, char >> *oldest_file); >> >> /** >> * Open/create a file for append in non blocking mode. >> @@ -264,32 +265,53 @@ static int delete_config_file(log_stream >> * @return -1 on error >> */ >> static int rotate_if_needed(log_stream_t *stream) { >> - char oldest_file[PATH_MAX]; >> + char oldest_log_file[PATH_MAX]; >> + char oldest_cfg_file[PATH_MAX]; >> int rc = 0; >> - int file_cnt; >> + int log_file_cnt, cfg_file_cnt; >> + bool oldest_cfg = false; >> >> TRACE_ENTER(); >> >> - /* Rotate out files from previous lifes */ >> - if ((file_cnt = get_number_of_log_files_h(stream, oldest_file)) == -1) { >> + /* Rotate out log files from previous lifes */ >> + if ((log_file_cnt = get_number_of_log_files_h(stream, oldest_log_file)) == >> -1) { >> rc = -1; >> goto done; >> } >> >> + /* Rotate out cfg files from previous lifes */ >> + if (!((cfg_file_cnt = get_number_of_cfg_files_h(stream, oldest_cfg_file)) >> == -1)) { >> + oldest_cfg = true; >> + } >> + >> + TRACE("delete oldest_cfg_file: %s oldest_log_file %s", oldest_cfg_file, >> oldest_log_file); >> + >> /* >> ** Remove until we have one less than allowed, we are just about to >> ** create a new one again. >> */ >> - while (file_cnt >= static_cast<int>(stream->maxFilesRotated)) { >> - if ((rc = file_unlink_h(oldest_file)) == -1) { >> - LOG_NO("Could not delete: %s - %s", oldest_file, strerror(errno)); >> + while (log_file_cnt >= static_cast<int>(stream->maxFilesRotated)) { >> + if ((rc = file_unlink_h(oldest_log_file)) == -1) { >> + LOG_NO("Could not log delete: %s - %s", oldest_log_file, >> strerror(errno)); >> goto done; >> } >> >> - if ((file_cnt = get_number_of_log_files_h(stream, oldest_file)) == -1) { >> + if (oldest_cfg == true) { >> + oldest_cfg = false; >> + if ((rc = file_unlink_h(oldest_cfg_file)) == -1) { >> + LOG_NO("Could not cfg delete: %s - %s", oldest_cfg_file, >> strerror(errno)); >> + goto done; >> + } >> + } >> + >> + if ((log_file_cnt = get_number_of_log_files_h(stream, oldest_log_file)) >> == -1) { >> rc = -1; >> goto done; >> } >> + >> + if (!((cfg_file_cnt = get_number_of_cfg_files_h(stream, >> oldest_cfg_file)) >> == -1)) { >> + oldest_cfg = true; >> + } >> } >> >> done: >> @@ -938,6 +960,56 @@ done: >> TRACE_LEAVE(); >> return rc; >> } >> +/** >> + * Return number of cfg files in a dir and the name of the oldest file. >> + * @param logStream[in] >> + * @param oldest_file[out] >> + * >> + * @return -1 if error else number of log files >> + */ >> +static int get_number_of_cfg_files_h(log_stream_t *logStream, char >> *oldest_file) { >> + lgsf_apipar_t apipar; >> + lgsf_retcode_t api_rc; >> + gnolfh_in_t parameters_in; >> + size_t path_len; >> + int rc = 0; >> + >> + TRACE_ENTER(); >> + >> + std::string logsv_root_dir = static_cast<const char *>( >> + lgs_cfg_get(LGS_IMM_LOG_ROOT_DIRECTORY)); >> + >> + parameters_in.file_name = const_cast<char *>(logStream- >>> fileName.c_str()); >> + parameters_in.logsv_root_dir = const_cast<char >> *>(logsv_root_dir.c_str()); >> + parameters_in.pathName = const_cast<char *>(logStream- >>> pathName.c_str()); >> + >> + path_len = strlen(parameters_in.file_name) + >> + strlen(parameters_in.logsv_root_dir) + >> + strlen(parameters_in.pathName); >> + if (path_len > PATH_MAX) { >> + LOG_WA("Path to cfg files > PATH_MAX"); >> + rc = -1; >> + goto done; >> + } >> + >> + /* Fill in API structure */ >> + apipar.req_code_in = LGSF_GET_NUM_CFGFILES; >> + apipar.data_in_size = sizeof(gnolfh_in_t); >> + apipar.data_in = ¶meters_in; >> + apipar.data_out_size = PATH_MAX; >> + apipar.data_out = oldest_file; >> + api_rc = log_file_api(&apipar); >> + if (api_rc != LGSF_SUCESS) { >> + TRACE("%s - API error %s",__FUNCTION__,lgsf_retcode_str(api_rc)); >> + rc = -1; >> + } else { >> + rc = apipar.hdl_ret_code_out; >> + } >> + >> +done: >> + TRACE_LEAVE(); >> + return rc; >> +} >> >> /** >> * Handle log file rotation on standby node. ------------------------------------------------------------------------------ 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
