Findings from review

exasol · Oct 28, 2024 · 87a667f · 87a667f
1 parent 506db77
commit 87a667f
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 7 deletions.
diff --git a/exaudfclient/base/javacontainer/script_options/converter.cc b/exaudfclient/base/javacontainer/script_options/converter.cc
@@ -11,9 +11,11 @@ Converter::Converter()
 , m_whitespace(" \t\f\v") {}
 
 void Converter::convertScriptClassName(const std::string & value) {
-    if (value != "") {
-        m_jvmOptions.push_back("-Dexasol.scriptclass=" + value);
+
+    if (value.empty()) {
+        return;
     }
+    m_jvmOptions.push_back("-Dexasol.scriptclass=" + value);
 }
 
 void Converter::convertJvmOption(const std::string & value) {

diff --git a/exaudfclient/base/javacontainer/script_options/string_ops.cc b/exaudfclient/base/javacontainer/script_options/string_ops.cc
@@ -1,5 +1,6 @@
 #include "base/javacontainer/script_options/string_ops.h"
 #include <regex>
+#include <iostream>
 
 namespace SWIGVMContainers {
 
@@ -9,7 +10,7 @@ namespace StringOps {
 
 inline uint32_t countBackslashesBackwards(const std::string & s, size_t pos) {
     uint32_t retVal(0);
-    while (pos >= 0 && s[pos--] == '\\') retVal++;
+    while (pos >= 0 && s.at(pos--) == '\\') retVal++;
     return retVal;
 }
 
@@ -32,7 +33,7 @@ inline size_t replaceBackslashSequencesAndWhitespaceSequence(std::string & s, si
 }
 
 inline size_t replaceCharAtPositionAndBackslashes(std::string & s, size_t pos, const char* replacement) {
-    const uint32_t nBackslashes = countBackslashesBackwards(s, pos-1);
+    const size_t nBackslashes =  (pos > 0) ? countBackslashesBackwards(s, pos-1) : 0;
 
     const size_t backslashStartIdx = pos-nBackslashes;
     if(nBackslashes % 2 == 0) {
@@ -51,7 +52,7 @@ void replaceTrailingEscapeWhitespaces(std::string & s) {
             if (s.size() > 1) {
                 if(s[lastNoneWhitespaceIdx] == 't') {
                     firstWhitespaceAfterNoneWhitespaceIdx = replaceCharAtPositionAndBackslashes(s, lastNoneWhitespaceIdx, "\t");
-                } else if (s[lastNoneWhitespaceIdx] == '\\' && s[lastNoneWhitespaceIdx+1] == ' ') {
+                } else if (s[lastNoneWhitespaceIdx] == '\\' && lastNoneWhitespaceIdx < (s.size() - 1) && s.at(lastNoneWhitespaceIdx+1) == ' ') {
                     firstWhitespaceAfterNoneWhitespaceIdx = replaceCharAtPositionAndBackslashes(s, lastNoneWhitespaceIdx+1, " ");
                 } else if (s[lastNoneWhitespaceIdx] == 'f') {
                     firstWhitespaceAfterNoneWhitespaceIdx = replaceCharAtPositionAndBackslashes(s, lastNoneWhitespaceIdx, "\f");

diff --git a/exaudfclient/base/javacontainer/script_options/test/converter_test.cc b/exaudfclient/base/javacontainer/script_options/test/converter_test.cc
@@ -40,7 +40,6 @@ class ConverterV2JarTest : public ::testing::TestWithParam<std::pair<std::string
 TEST_P(ConverterV2JarTest, jar) {
     const std::pair<std::string, std::vector<std::string>> option_value = GetParam();
     const std::string jar_option_value = option_value.first;
-    std::cerr << "DEBUG: " << jar_option_value << std::endl;
 
     ConverterV2 converter;
     converter.convertExternalJar(option_value.first);

diff --git a/exaudfclient/base/javacontainer/script_options/test/string_ops_tests.cc b/exaudfclient/base/javacontainer/script_options/test/string_ops_tests.cc
@@ -32,7 +32,6 @@ TEST_P(ReplaceTrailingEscapeWhitespacesTest, s) {
 
     std::string str = underTest.first;
     StringOps::replaceTrailingEscapeWhitespaces(str);
-    std::cerr << "str='" << str << "' underTest.second='" << underTest.second << "'" << std::endl;
     ASSERT_EQ(str, underTest.second);
 }
 
@@ -48,8 +47,11 @@ const std::vector<std::pair<std::string, std::string>> replace_trailing_escape_w
             std::make_pair("hello world\\\\\\t\t", std::string("hello world\\\t")),
             std::make_pair("hello world\\\\\\\\t\t", std::string("hello world\\\\t")),
             std::make_pair("hello worl\td\\\\\\\\t\t", std::string("hello worl\td\\\\t")),
+            std::make_pair("t\t ", std::string("t")),
+            std::make_pair("hello worl\td\\\\\\\\", std::string("hello worl\td\\\\\\\\")), //If no whitespace escape sequence, backslashes are not interpreted as backslash escape sequence
         };
 
+
 INSTANTIATE_TEST_SUITE_P(
     StringOpsTest,
     ReplaceTrailingEscapeWhitespacesTest,