Refs #19378: Apply some other suggestions

Signed-off-by: EduPonz <eduardoponz@eprosima.com>
eProsima · Oct 24, 2023 · 10a45b0 · 10a45b0
1 parent 59e3e62
commit 10a45b0
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 10 deletions.
diff --git a/src/cpp/rtps/xmlparser/XMLElementParser.cpp b/src/cpp/rtps/xmlparser/XMLElementParser.cpp
@@ -3758,15 +3758,30 @@ XMLP_ret XMLParser::getXMLUint(
 
     auto to_uint64 = [](const char* str, unsigned long int* value) -> bool
             {
+                // Look for a '-' sign
+                bool ret = false;
+                const char minus = '-';
+                const char* minus_result = str;
+                if (nullptr == std::strchr(minus_result, minus))
+                {
+                    // Minus not found
+                    ret = true;
+                }
+
+                if (ret)
+                {
+                    ret = false;
 #ifdef _WIN32
-                if (sscanf_s(str, "%lu", value) == 1)
+                    if (sscanf_s(str, "%lu", value) == 1)
 #else
-                if (sscanf(str, "%lu", value) == 1)
+                    if (sscanf(str, "%lu", value) == 1)
 #endif // ifdef _WIN32
-                {
-                    return true;
+                    {
+                        // Number found
+                        ret = true;
+                    }
                 }
-                return false;
+                return ret;
             };
 
     std::string text = get_element_text(elem);

diff --git a/src/cpp/rtps/xmlparser/XMLParser.cpp b/src/cpp/rtps/xmlparser/XMLParser.cpp
@@ -2213,16 +2213,18 @@ XMLP_ret XMLParser::fillDataNode(
                 return XMLP_ret::XML_ERROR;
             }
         }
-#if HAVE_SECURITY
         else if (strcmp(name, SECURITY_LOG_THREAD) == 0)
         {
+#if HAVE_SECURITY
             if (XMLP_ret::XML_OK != getXMLThreadSettings(*p_aux0, participant_node.get()->rtps.security_log_thread))
             {
                 return XMLP_ret::XML_ERROR;
             }
-        }
+#else
+            EPROSIMA_LOG_WARNING(XMLPARSER, "Ignoring '" << SECURITY_LOG_THREAD << "' since security is disabled");
 #endif // if HAVE_SECURITY
-        else if (strcmp(name, SECURITY_LOG_THREAD) != 0)
+        }
+        else
         {
             EPROSIMA_LOG_ERROR(XMLPARSER, "Invalid element found into 'rtpsParticipantAttributesType'. Name: " << name);
             return XMLP_ret::XML_ERROR;

diff --git a/test/unittest/xmlparser/XMLElementParserTests.cpp b/test/unittest/xmlparser/XMLElementParserTests.cpp
@@ -4138,6 +4138,7 @@ TEST_F(XMLParserTests, getXMLThreadSettings)
         {{"12", "-1", "12", "12", ""}, XMLP_ret::XML_OK},
         {{"12", "12", "12", "-1", ""}, XMLP_ret::XML_OK},
         {{"-2", "12", "12", "12", ""}, XMLP_ret::XML_ERROR},
+        {{"12", "12", "-2", "12", ""}, XMLP_ret::XML_ERROR},
         {{"12", "12", "12", "-2", ""}, XMLP_ret::XML_ERROR},
         {{"a", "12", "12", "12", ""}, XMLP_ret::XML_ERROR},
         {{"12", "a", "12", "12", ""}, XMLP_ret::XML_ERROR},
@@ -4172,7 +4173,7 @@ TEST_F(XMLParserTests, getXMLThreadSettings)
                 "</thread_settings>";
 
         // Parse the XML snippet
-        ASSERT_EQ(tinyxml2::XMLError::XML_SUCCESS, xml_doc.Parse(xml.c_str()));
+        ASSERT_EQ(tinyxml2::XMLError::XML_SUCCESS, xml_doc.Parse(xml.c_str())) << xml;
 
         // Extract ThreadSetting
         titleElement = xml_doc.RootElement();
@@ -4236,7 +4237,7 @@ TEST_F(XMLParserTests, getXMLThreadSettingsWithPort)
                 "</thread_settings>";
 
         // Parse the XML snippet
-        ASSERT_EQ(tinyxml2::XMLError::XML_SUCCESS, xml_doc.Parse(xml.c_str()));
+        ASSERT_EQ(tinyxml2::XMLError::XML_SUCCESS, xml_doc.Parse(xml.c_str())) << xml;
 
         // Extract ThreadSetting
         titleElement = xml_doc.RootElement();