Bug#35990372 Wrong index used in ConfigValues::openSection()

Context:
Some NDBAPI tests uses ConfigValues::openSection()
function in order to get/set config parameters.
openSection has as argument the section 'type'
(SYSTEM, NODE, CONNECTION ...) and the index to the
section we are interested in.
Section indexes are sequential, starting from zero
and are ordered by nodeId BUT nodeId and index can
be different.

Problem:
In some testes openSection is used with wrong
arguments (nodeId is used instead index). Depending
on the configuration this can lead openSection to
open wrong/invalid section.

Solution:
Tests changed to always use index instead nodeId
to openSection.
In particular, when iterating over all sections
looking to a parameter, iteration now starts from
index 0, preventing any section from being ignored.

Change-Id: I10468675b1ef07cf01a89961f5d5a84a498b4568

Author: vinc13e

storage/ndb/test/include/NdbMgmd.hpp

@@ -482,24 +482,19 @@ class NdbMgmd {
 
     Uint64 default_value = 0;
     ConfigValues::Iterator iter(conf.m_configValues->m_config);
-    for (int nodeid = 1; nodeid < MAX_NODES; nodeid++)
+    for(int idx=0; iter.openSection(type_of_section, idx); idx++)
     {
-      if (!iter.openSection(type_of_section, nodeid))
-        continue;
       Uint64 old_value = 0;
       if (iter.get(config_variable, &old_value))
       {
         if (default_value == 0)
         {
           default_value = old_value;
-        }
-        else if (old_value != default_value)
-        {
-          g_err << "StartDiskPageBufferMemory is not consistent across nodes"
-                << ". Node id " << nodeid
-                << ": value " << old_value
-                << ". Overwriting it with the given value " << new_value
+        } else if (old_value != default_value) {
+          g_err << "Config value is not consistent across sections."
                 << endl;
+          iter.closeSection();
+          return false;
         }
       }
       iter.set(config_variable, new_value);
@@ -544,24 +539,19 @@ class NdbMgmd {
 
     Uint32 default_value = 0;
     ConfigValues::Iterator iter(conf.m_configValues->m_config);
-    for (int nodeid = 1; nodeid < MAX_NODES; nodeid++)
+    for(int idx=0; iter.openSection(type_of_section, idx); idx++)
     {
-      if (!iter.openSection(type_of_section, nodeid))
-        continue;
       Uint32 old_value = 0;
       if (iter.get(config_variable, &old_value))
       {
         if (default_value == 0)
         {
           default_value = old_value;
-        }
-        else if (old_value != default_value)
-        {
-          g_err << "Config value is not consistent across nodes"
-                << ". Node id " << nodeid
-                << ": value " << old_value
-                << ". Overwriting it with the given value " << new_value
+        } else if (old_value != default_value) {
+          g_err << "Config value is not consistent across sections."
                 << endl;
+          iter.closeSection();
+          return false;
         }
       }
       iter.set(config_variable, new_value);
@@ -605,15 +595,12 @@ class NdbMgmd {
     }
 
     ConfigValues::Iterator iter(conf.m_configValues->m_config);
-    for (int nodeid = 1; nodeid < MAX_NODES; nodeid++)
+    for(int idx=0; iter.openSection(type_of_section, idx); idx++)
     {
-      if (!iter.openSection(type_of_section, nodeid))
-        continue;
       Uint32 current_value = 0;
-      if (iter.get(config_variable, &current_value))
-      {
-        if (current_value > 0)
-        {
+      if (iter.get(config_variable, &current_value)) {
+        if (current_value > 0) {
+          iter.closeSection();
           return current_value;
         }
       }

storage/ndb/test/ndbapi/testMgm.cpp

@@ -1,5 +1,5 @@
 /*
-   Copyright (c) 2003, 2021, Oracle and/or its affiliates.
+   Copyright (c) 2003, 2023, Oracle and/or its affiliates.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License, version 2.0,
@@ -3664,12 +3664,9 @@ int runTestNdbApiConfig(NDBT_Context* ctx, NDBT_Step* step)
       return NDBT_FAILED;
 
     ConfigValues::Iterator iter(conf.m_configValues->m_config);
-    for (Uint32 nodeid = 1; nodeid < MAX_NODES; nodeid ++)
+    for(int idx=0; iter.openSection(CFG_SECTION_NODE, idx); idx++)
     {
       Uint32 type;
-      if (!iter.openSection(CFG_SECTION_NODE, nodeid))
-        continue;
-
       if (iter.get(CFG_TYPE_OF_SECTION, &type) &&
           type == NDB_MGM_NODE_TYPE_API)
       {
@@ -3678,7 +3675,6 @@ int runTestNdbApiConfig(NDBT_Context* ctx, NDBT_Step* step)
           iter.set(parameters[param].key, parameters[param].values[i]);
         }
       }
-
       iter.closeSection();
     }
 

storage/ndb/test/ndbapi/testNodeRestart.cpp

@@ -65,10 +65,8 @@ changeStartPartitionedTimeout(NDBT_Context *ctx, NDBT_Step *step)
     g_err << "Setting StartPartitionedTimeout to " << startPartitionedTimeout
           << endl;
     ConfigValues::Iterator iter(conf.m_configValues->m_config);
-    for (int nodeid = 1; nodeid < MAX_NODES; nodeid++)
+    for(int idx=0; iter.openSection(CFG_SECTION_NODE, idx); idx++)
     {
-      if (!iter.openSection(CFG_SECTION_NODE, nodeid))
-        continue;
       Uint32 oldValue = 0;
       if (iter.get(CFG_DB_START_PARTITION_TIMEOUT, oldValue))
       {
@@ -78,7 +76,8 @@ changeStartPartitionedTimeout(NDBT_Context *ctx, NDBT_Step *step)
         }
         else if (oldValue != defaultValue)
         {
-          g_err << "StartPartitionedTimeout is not consistent across nodes" << endl;
+          g_err << "StartPartitionedTimeout is not consistent across data node"
+                   "sections" << endl;
           break;
         }
       }
@@ -8911,12 +8910,8 @@ int run_PLCP_many_parts(NDBT_Context *ctx, NDBT_Step *step)
   }
   ConfigValues::Iterator iter(conf.m_configValues->m_config);
   Uint32 enabledPartialLCP = 1;
-  for (int idx = 0; idx < MAX_NODES; idx ++)
+  for(int idx=0; iter.openSection(CFG_SECTION_NODE, idx); idx++)
   {
-    if (!iter.openSection(CFG_SECTION_NODE, idx))
-    {
-      break;
-    }
     Uint32 nodeId=0;
     if(iter.get(CFG_NODE_ID, &nodeId))
     {

storage/ndb/test/ndbapi/testRedo.cpp

@@ -1,5 +1,5 @@
 /*
-   Copyright (c) 2012, 2021, Oracle and/or its affiliates.
+   Copyright (c) 2012, 2023, Oracle and/or its affiliates.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License, version 2.0,
@@ -1317,15 +1317,13 @@ resizeRedoLog(NDBT_Context* ctx, NDBT_Step* step)
     }
 
     g_err << "Setting NoOfFragmentLogFiles = " << noOfLogFiles
-          << " FragmentLogFileSize = " << logFileSize
-          << " TimeBetweenLCP " << LCPinterval << endl;
+          << " FragmentLogFileSize = " << logFileSize << " TimeBetweenLCP "
+          << LCPinterval << endl;
     ConfigValues::Iterator iter(conf.m_configValues->m_config);
-    for (int nodeid = 1; nodeid < MAX_NODES; nodeid ++)
+    for(int idx=0; iter.openSection(CFG_SECTION_NODE, idx); idx++)
     {
       Uint32 oldValue;
-      if (!iter.openSection(CFG_SECTION_NODE, nodeid))
-        continue;
-      if (iter.get(CFG_DB_NO_REDOLOG_FILES, &oldValue)) 
+      if (iter.get(CFG_DB_NO_REDOLOG_FILES, &oldValue))
       {
         iter.set(CFG_DB_NO_REDOLOG_FILES, noOfLogFiles);
         if(defaultNoOfLogFiles == 0)
@@ -1334,7 +1332,8 @@ resizeRedoLog(NDBT_Context* ctx, NDBT_Step* step)
         }
         else if(oldValue != defaultNoOfLogFiles) 
         {
-          g_err << "NoOfFragmentLogFiles is not consistent across nodes" << endl;
+          g_err << "NoOfFragmentLogFiles is not consistent across nodes"
+                << endl;
           break; 
         }
       }