osaf/services/saf/logsv/lgs/lgs_amf.cc    |  34 ++++++++++-----
 osaf/services/saf/logsv/lgs/lgs_config.cc |  12 +++--
 osaf/services/saf/logsv/lgs/lgs_evt.cc    |  31 +++++++++-----
 osaf/services/saf/logsv/lgs/lgs_imm.cc    |  63 +++++++++++++++++++-----------
 osaf/services/saf/logsv/lgs/lgs_mbcsv.cc  |  29 +++++++++----
 osaf/services/saf/logsv/lgs/lgs_stream.cc |  16 ++++++-
 osaf/services/saf/logsv/lgs/lgs_stream.h  |   3 +-
 7 files changed, 123 insertions(+), 65 deletions(-)


The `number of streams` refers to total existing log streams in cluster.
And `stream_array` is the database holding all existing log streams.
When interating all log streams, logsv first started at the index `number of 
streams`,
if getting NULL, logsv considered that case as `no stream`. This is absolutely 
wrong.

This patch provides other way to iterate all log streams.

diff --git a/osaf/services/saf/logsv/lgs/lgs_amf.cc 
b/osaf/services/saf/logsv/lgs/lgs_amf.cc
--- a/osaf/services/saf/logsv/lgs/lgs_amf.cc
+++ b/osaf/services/saf/logsv/lgs/lgs_amf.cc
@@ -26,13 +26,19 @@
 
 static void close_all_files() {
   log_stream_t *stream;
-  int num = get_number_of_streams();
-  stream = log_stream_get_by_id(--num);
-  while (stream != NULL) {
+  uint32_t count = 0, stream_id = 0, max = 0, num = 0;
+
+  num = get_number_of_streams();
+  max = get_max_number_of_streams();
+  // Iterate all existing log streams in cluster
+  // the condition `stream_id < max` to avoid blocking
+  while (count < num && stream_id < max) {
+    stream = log_stream_get_by_id(stream_id++);
+    if (stream == nullptr) continue;
+
+    count++;
     if (log_stream_file_close(stream) != 0)
       LOG_WA("Could not close file for stream %s", stream->name.c_str());
-
-    stream = log_stream_get_by_id(--num);
   }
 }
 
@@ -52,7 +58,7 @@ static void close_all_files() {
 static SaAisErrorT amf_active_state_handler(lgs_cb_t *cb, SaInvocationT 
invocation) {
   log_stream_t *stream;
   SaAisErrorT error = SA_AIS_OK;
-  int num;
+  uint32_t count = 0, stream_id = 0, max = 0, num = 0;
 
   TRACE_ENTER2("HA ACTIVE request");
 
@@ -67,13 +73,17 @@ static SaAisErrorT amf_active_state_hand
 
   /* check existing streams */
   num = get_number_of_streams();
-  stream = log_stream_get_by_id(--num);
-  if (!stream)
+  max = get_max_number_of_streams();
+  while (count < num && stream_id < max) {
+    stream = log_stream_get_by_id(stream_id++);
+    if (stream == nullptr) continue;
+
+    count++;
+    *stream->p_fd = -1; /* First Initialize fd */
+  }
+
+  if (count == 0)
     LOG_ER("No streams exist!");
-  while (stream != NULL) {
-    *stream->p_fd = -1; /* First Initialize fd */
-    stream = log_stream_get_by_id(--num);
-  }
 
 done:
   /* Update role independent of stream processing */
diff --git a/osaf/services/saf/logsv/lgs/lgs_config.cc 
b/osaf/services/saf/logsv/lgs/lgs_config.cc
--- a/osaf/services/saf/logsv/lgs/lgs_config.cc
+++ b/osaf/services/saf/logsv/lgs/lgs_config.cc
@@ -458,7 +458,7 @@ int lgs_cfg_verify_root_dir(const std::s
   int rc = 0;
   log_stream_t *stream = NULL;
   size_t n = root_str_in.size();
-  int num;
+  uint32_t count = 0, stream_id = 0, max = 0, num = 0;
 
   if (n > PATH_MAX) {
     LOG_NO("verify_root_dir Fail. Path > PATH_MAX");
@@ -471,16 +471,18 @@ int lgs_cfg_verify_root_dir(const std::s
    * must not be larger than PATH_MAX.
    */
   num = get_number_of_streams();
-  stream = log_stream_get_by_id(--num);
-  while (stream != NULL) {
+  max = get_max_number_of_streams();
+  while (count < num && stream_id < max) {
+    stream = log_stream_get_by_id(stream_id++);
+    if (stream == nullptr) continue;
+
+    count++;
     if (lgs_is_valid_pathlength(stream->pathName, stream->fileName,
                                 root_str_in) == false) {
       TRACE("The rootPath is invalid (%s)", root_str_in.c_str());
       rc = -1;
       goto done;
     }
-
-    stream = log_stream_get_by_id(--num);
   }
 
   if (lgs_path_is_writeable_dir_h(root_str_in) == false) {
diff --git a/osaf/services/saf/logsv/lgs/lgs_evt.cc 
b/osaf/services/saf/logsv/lgs/lgs_evt.cc
--- a/osaf/services/saf/logsv/lgs/lgs_evt.cc
+++ b/osaf/services/saf/logsv/lgs/lgs_evt.cc
@@ -532,14 +532,19 @@ static uint32_t proc_rda_cb_msg(lgsv_lgs
     lgs_process_lga_down_list();
 
     /* Check existing streams */
-    int num = get_number_of_streams();
-    stream = log_stream_get_by_id(--num);
-    if (!stream)
+    uint32_t count = 0, stream_id = 0, max = 0;
+    uint32_t num = get_number_of_streams();
+    max = get_max_number_of_streams();
+    while (count < num && stream_id < max) {
+      stream = log_stream_get_by_id(stream_id++);
+      if (stream == nullptr) continue;
+
+      count++;
+      *stream->p_fd = -1; /* Initialize fd */
+    }
+
+    if (count == 0)
       LOG_ER("No streams exist!");
-    while (stream != NULL) {
-      *stream->p_fd = -1; /* Initialize fd */
-      stream = log_stream_get_by_id(--num);
-    }
   }
 
   TRACE_LEAVE();
@@ -800,7 +805,8 @@ SaAisErrorT create_new_app_stream(lgsv_s
   SaBoolT twelveHourModeFlag;
   SaUint32T logMaxLogrecsize_conf = 0;
   SaConstStringT str_name;
-  int num, err = 0;
+  int err = 0;
+  uint32_t count = 0, stream_id = 0, max = 0, num = 0;
   const char *dnPrefix = "safLgStr=";
 
   TRACE_ENTER();
@@ -863,15 +869,18 @@ SaAisErrorT create_new_app_stream(lgsv_s
 
   /* Verify that path and file are unique */
   num = get_number_of_streams();
-  stream = log_stream_get_by_id(--num);
-  while (stream != NULL) {
+  max = get_max_number_of_streams();
+  while (count < num && stream_id < max) {
+    stream = log_stream_get_by_id(stream_id++);
+    if (stream == nullptr) continue;
+
+    count++;
     if ((stream->fileName == open_sync_param->logFileName) &&
         (stream->pathName == open_sync_param->logFilePathName)) {
       TRACE("pathname already exist");
       rc = SA_AIS_ERR_INVALID_PARAM;
       goto done;
     }
-    stream = log_stream_get_by_id(--num);
   }
 
   /* Verify that the name seems to be a DN */
diff --git a/osaf/services/saf/logsv/lgs/lgs_imm.cc 
b/osaf/services/saf/logsv/lgs/lgs_imm.cc
--- a/osaf/services/saf/logsv/lgs/lgs_imm.cc
+++ b/osaf/services/saf/logsv/lgs/lgs_imm.cc
@@ -1022,7 +1022,7 @@ bool chk_filepath_stream_exist(
   log_stream_t *i_stream = NULL;
   std::string i_fileName;
   std::string i_pathName;
-  int num;
+  uint32_t count = 0, stream_id = 0, max = 0, num = 0;
   bool rc = false;
 
   TRACE_ENTER();
@@ -1073,16 +1073,18 @@ bool chk_filepath_stream_exist(
   /* Check if any stream has given filename and path */
   TRACE("Check if any stream has given filename and path");
   num = get_number_of_streams();
-  i_stream = log_stream_get_by_id(--num);
-  while (i_stream != NULL) {
+  max = get_max_number_of_streams();
+  while (count < num && stream_id < max) {
+    i_stream = log_stream_get_by_id(stream_id++);
+    if (i_stream == nullptr) continue;
+
+    count++;
     TRACE("Check stream \"%s\"", i_stream->name.c_str());
     if ((i_stream->fileName == i_fileName) &&
         (i_stream->pathName == i_pathName)) {
       rc = true;
       break;
     }
-
-    i_stream = log_stream_get_by_id(--num);
   }
 
   TRACE_LEAVE2("rc = %d", rc);
@@ -1832,15 +1834,19 @@ void logRootDirectory_filemove(
   TRACE_ENTER();
   log_stream_t *stream;
   std::string current_logfile;
-  int num;
+  uint32_t count = 0, stream_id = 0, max = 0, num = 0;
 
   /* Close and rename files at current path
    */
   num = get_number_of_streams();
-  stream = log_stream_get_by_id(--num);
-  while (stream != NULL) {
+  max = get_max_number_of_streams();
+  while (count < num && stream_id < max) {
+    stream = log_stream_get_by_id(stream_id++);
+    if (stream == nullptr) continue;
+
     TRACE("Handling file %s", stream->logFileCurrent.c_str());
 
+    count++;
     if (lgs_cb->ha_state == SA_AMF_HA_ACTIVE) {
       current_logfile = stream->logFileCurrent;
     } else {
@@ -1855,21 +1861,25 @@ void logRootDirectory_filemove(
       LOG_ER("Old log files could not be renamed and closed for stream: %s",
              stream->name.c_str());
     }
-    stream = log_stream_get_by_id(--num);
   }
 
   /* Create new files at new path
    */
-  num = get_number_of_streams();
-  stream = log_stream_get_by_id(--num);
-  while (stream != NULL) {
+  char *current_time;
+  count = 0;
+  stream_id = 0;
+  while (count < num && stream_id < max) {
+    stream = log_stream_get_by_id(stream_id++);
+    if (stream == nullptr) continue;
+
+    count++;
     if (lgs_create_config_file_h(new_logRootDirectory, stream) != 0) {
       LOG_ER("New config file could not be created for stream: %s",
              stream->name.c_str());
     }
 
     /* Create the new log file based on updated configuration */
-    char *current_time = lgs_get_time(cur_time_in);
+    current_time = lgs_get_time(cur_time_in);
     stream->logFileCurrent = stream->fileName + "_" + current_time;
 
     if ((*stream->p_fd = log_file_open(new_logRootDirectory,
@@ -1882,8 +1892,8 @@ void logRootDirectory_filemove(
      * Used if standby and configured for split file system
      */
     stream->stb_logFileCurrent = stream->logFileCurrent;
-    stream = log_stream_get_by_id(--num);
   }
+
   TRACE_LEAVE();
 }
 
@@ -1898,7 +1908,7 @@ void logRootDirectory_filemove(
 void logDataGroupname_fileown(const char *new_logDataGroupname) {
   TRACE_ENTER();
   log_stream_t *stream;
-  int num;
+  uint32_t count = 0, stream_id = 0, max = 0, num = 0;
 
   if (new_logDataGroupname == NULL) {
     LOG_ER("Data group is NULL");
@@ -1911,12 +1921,16 @@ void logDataGroupname_fileown(const char
      * Change ownership of log files to this new group
      */
     num = get_number_of_streams();
-    stream = log_stream_get_by_id(--num);
-    while (stream != NULL) {
+    max = get_max_number_of_streams();
+    while (count < num && stream_id < max) {
+      stream = log_stream_get_by_id(stream_id++);
+      if (stream == nullptr) continue;
+
+      count++;
       lgs_own_log_files_h(stream, new_logDataGroupname);
-      stream = log_stream_get_by_id(--num);
     }
   }
+
   TRACE_LEAVE();
 }
 
@@ -2708,7 +2722,8 @@ SaAisErrorT lgs_imm_init_configStreams(l
   SaImmAttrValuesT_2 **attributes;
   int wellknownStreamId = 0;
   int appStreamId = 3;
-  int streamId = 0, num;
+  uint32_t streamId = 0;
+  uint32_t count = 0, stream_id = 0, max = 0, num = 0;
   SaNameT objectName;
   const char *className = "SaLogStreamConfig";
 
@@ -2775,8 +2790,12 @@ SaAisErrorT lgs_imm_init_configStreams(l
   }
 
   num = get_number_of_streams();
-  stream = log_stream_get_by_id(--num);
-  while (stream != NULL) {
+  max = get_max_number_of_streams();
+  while (count < num && stream_id < max) {
+    stream = log_stream_get_by_id(stream_id++);
+    if (stream == nullptr) continue;
+
+    count++;
     if (cb->scAbsenceAllowed != 0) {
       int_rc = log_stream_open_file_restore(stream);
       if (int_rc == -1) {
@@ -2802,8 +2821,6 @@ SaAisErrorT lgs_imm_init_configStreams(l
       LOG_ER("immutil_update_one_rattr failed %s", saf_error(ais_rc));
       osaf_abort(0);
     }
-
-    stream = log_stream_get_by_id(--num);
   }
 
 done:
diff --git a/osaf/services/saf/logsv/lgs/lgs_mbcsv.cc 
b/osaf/services/saf/logsv/lgs/lgs_mbcsv.cc
--- a/osaf/services/saf/logsv/lgs/lgs_mbcsv.cc
+++ b/osaf/services/saf/logsv/lgs/lgs_mbcsv.cc
@@ -264,7 +264,7 @@ done:
 uint32_t lgs_mbcsv_change_HA_state(lgs_cb_t *cb, SaAmfHAStateT ha_state) {
   TRACE_ENTER();
   NCS_MBCSV_ARG mbcsv_arg;
-  int num;
+  uint32_t count = 0, stream_id = 0, max = 0, num = 0;
 
   memset(&mbcsv_arg, '\0', sizeof(NCS_MBCSV_ARG));
 
@@ -288,8 +288,14 @@ uint32_t lgs_mbcsv_change_HA_state(lgs_c
   log_stream_t *stream;
   if (lgs_is_split_file_system()) {
     num = get_number_of_streams();
-    stream = log_stream_get_by_id(--num);
-    while (stream != NULL) { /* Iterate over all streams */
+    max = get_max_number_of_streams();
+    // Iterate all existing log streams in cluster
+    // the condition `stream_id < max` is extra protection to avoid blocking
+    while (count < num && stream_id < max) {
+      stream = log_stream_get_by_id(stream_id++);
+      if (stream == nullptr) continue;
+
+      count++;
       if (ha_state == SA_AMF_HA_ACTIVE) {
         stream->logFileCurrent = stream->stb_logFileCurrent;
         stream->curFileSize = stream->stb_curFileSize;
@@ -300,8 +306,6 @@ uint32_t lgs_mbcsv_change_HA_state(lgs_c
         stream->stb_curFileSize = stream->curFileSize;
         *stream->p_fd = -1; /* Reopen files */
       }
-
-      stream = log_stream_get_by_id(--num);
     }
   }
 
@@ -612,7 +616,7 @@ static uint32_t edu_enc_streams(lgs_cb_t
   uint32_t rc = NCSCC_RC_SUCCESS, num_rec = 0;
   uint8_t *pheader = NULL;
   lgsv_ckpt_header_t ckpt_hdr;
-  int num;
+  uint32_t count = 0, stream_id = 0, max = 0, num = 0;
 
   /* Prepare reg. structure to encode */
   ckpt_stream_rec = static_cast<lgs_ckpt_stream_open_t 
*>(malloc(sizeof(lgs_ckpt_stream_open_t)));
@@ -630,10 +634,16 @@ static uint32_t edu_enc_streams(lgs_cb_t
   }
   ncs_enc_claim_space(uba, sizeof(lgsv_ckpt_header_t));
 
+  /* Walk through the reg list and encode record by record */
   num = get_number_of_streams();
-  log_stream_rec = log_stream_get_by_id(--num);
-  /* Walk through the reg list and encode record by record */
-  while (log_stream_rec != NULL) {
+  max = get_max_number_of_streams();
+  // Iterate all existing log streams in cluster
+  // the condition `stream_id < max` is extra protection to avoid blocking
+  while (count < num && stream_id < max) {
+    log_stream_rec = log_stream_get_by_id(stream_id++);
+    if (log_stream_rec == nullptr) continue;
+
+    count++;
     lgs_ckpt_stream_open_set(log_stream_rec, ckpt_stream_rec);
     rc = m_NCS_EDU_EXEC(&cb->edu_hdl,
                         edp_ed_open_stream_rec, uba, EDP_OP_TYPE_ENC, 
ckpt_stream_rec, &ederror);
@@ -645,7 +655,6 @@ static uint32_t edu_enc_streams(lgs_cb_t
       return rc;
     }
     ++num_rec;
-    log_stream_rec = log_stream_get_by_id(--num);
   }                       /* End while RegRec */
 
   /* Encode RegHeader */
diff --git a/osaf/services/saf/logsv/lgs/lgs_stream.cc 
b/osaf/services/saf/logsv/lgs/lgs_stream.cc
--- a/osaf/services/saf/logsv/lgs/lgs_stream.cc
+++ b/osaf/services/saf/logsv/lgs/lgs_stream.cc
@@ -37,9 +37,9 @@
 
 static log_stream_t **stream_array;
 /* We have at least the 3 well known streams. */
-static unsigned int stream_array_size = 3;
+static uint32_t stream_array_size = 3;
 /* Current number of streams */
-static unsigned int numb_of_streams;
+static uint32_t numb_of_streams;
 
 static const uint32_t kInvalidId = static_cast<uint32_t> (-1);
 
@@ -107,11 +107,21 @@ done:
  * @param: none
  * @return current number of opening streams
  */
-unsigned int get_number_of_streams() {
+uint32_t get_number_of_streams() {
   return numb_of_streams;
 }
 
 /**
+ * Get max number of allowed log streams in cluster
+ *
+ * @param: none
+ * @return the max number of log streams
+ */
+uint32_t get_max_number_of_streams() {
+  return stream_array_size;
+}
+
+/**
  * Close with retry at EINTR
  *
  * @param fd [in]
diff --git a/osaf/services/saf/logsv/lgs/lgs_stream.h 
b/osaf/services/saf/logsv/lgs/lgs_stream.h
--- a/osaf/services/saf/logsv/lgs/lgs_stream.h
+++ b/osaf/services/saf/logsv/lgs/lgs_stream.h
@@ -130,7 +130,8 @@ extern void log_stream_print(log_stream_
 extern log_stream_t *log_stream_get_by_id(uint32_t id);
 extern bool check_max_stream();
 void log_free_stream_resources(log_stream_t *stream);
-unsigned int get_number_of_streams();
+uint32_t get_number_of_streams();
+uint32_t get_max_number_of_streams();
 extern log_stream_t *log_stream_get_by_name(const std::string &name);
 
 #endif

------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to