Fix String::compareTo(const char*) for invalid strings

matthijskooijman · matthijskooijman · commit e4c65c1c6beb · 2021-01-25T09:51:51.000+01:00
When comparing an invalid String object to a non-empty char*, this would
erronously return 0 (equal) because of a typo.

This bug also masked three incorrect checks in related testcases. In two
cases, a String was made invalid and then checked to still contain a
value (these were changed to check that the string is invalid) and in
one case the wrong string was checked.
diff --git a/api/String.cpp b/api/String.cpp
@@ -462,7 +462,7 @@ int String::compareTo(const String &s) const
 int String::compareTo(const char *cstr) const
 {
 	if (!buffer || !cstr) {
-		if (cstr && !*cstr) return 0 - *(unsigned char *)cstr;
+		if (cstr && *cstr) return 0 - *(unsigned char *)cstr;
 		if (buffer && len > 0) return *(unsigned char *)buffer;
 		return 0;
 	}
diff --git a/test/src/String/test_String.cpp b/test/src/String/test_String.cpp
@@ -127,7 +127,7 @@ TEST_CASE ("Testing String(const __FlashStringHelper) constructor() with invalid
   char *buffer = NULL;
 
   arduino::String str1(F(buffer));
-  REQUIRE(str1.compareTo("Hello") == 0);
+  REQUIRE_FALSE(str1);
 }
 
 TEST_CASE ("Testing String(StringSumHelper &&) constructor()", "[String-Ctor-13]")
@@ -158,5 +158,5 @@ TEST_CASE ("Testing String(String &&) with move(String &rhs) from larger to smal
   arduino::String str("Hello");
   arduino::String str1("Arduino");
   str = static_cast<arduino::String&&>(str1);
-  REQUIRE(str1.compareTo("Arduino") == 0);
+  REQUIRE(str.compareTo("Arduino") == 0);
 }
diff --git a/test/src/String/test_operators.cpp b/test/src/String/test_operators.cpp
@@ -126,7 +126,7 @@ TEST_CASE ("Testing & String::operator = (const char *)", "[String-operator+-14]
   char *buffer = NULL;
   arduino::String str("Hello");
   str = buffer;
-  REQUIRE(str.compareTo("Hello") == 0);
+  REQUIRE(str.compareTo("Hello") != 0);
 }
 
 TEST_CASE ("Testing & String::operator = (const String &) with invalid buffer of first string", "[String-operator+-15]")

Original file line number	Diff line number	Diff line change
`@@ -462,7 +462,7 @@ int String::compareTo(const String &s) const`
`462`	`462`	`int String::compareTo(const char *cstr) const`
`463`	`463`	`{`
`464`	`464`	`if (!buffer \|\| !cstr) {`
`465`		`- if (cstr && !cstr) return 0 - (unsigned char *)cstr;`
	`465`	`+ if (cstr && cstr) return 0 - (unsigned char *)cstr;`
`466`	`466`	`if (buffer && len > 0) return (unsigned char )buffer;`
`467`	`467`	`return 0;`
`468`	`468`	`}`
Original file line number	Diff line number	Diff line change
`@@ -127,7 +127,7 @@ TEST_CASE ("Testing String(const __FlashStringHelper) constructor() with invalid`
`127`	`127`	`char *buffer = NULL;`
`128`	`128`
`129`	`129`	`arduino::String str1(F(buffer));`
`130`		`- REQUIRE(str1.compareTo("Hello") == 0);`
	`130`	`+ REQUIRE_FALSE(str1);`
`131`	`131`	`}`
`132`	`132`
`133`	`133`	`TEST_CASE ("Testing String(StringSumHelper &&) constructor()", "[String-Ctor-13]")`
`@@ -158,5 +158,5 @@ TEST_CASE ("Testing String(String &&) with move(String &rhs) from larger to smal`
`158`	`158`	`arduino::String str("Hello");`
`159`	`159`	`arduino::String str1("Arduino");`
`160`	`160`	`str = static_cast<arduino::String&&>(str1);`
`161`		`- REQUIRE(str1.compareTo("Arduino") == 0);`
	`161`	`+ REQUIRE(str.compareTo("Arduino") == 0);`
`162`	`162`	`}`
Original file line number	Diff line number	Diff line change
`@@ -126,7 +126,7 @@ TEST_CASE ("Testing & String::operator = (const char *)", "[String-operator+-14]`
`126`	`126`	`char *buffer = NULL;`
`127`	`127`	`arduino::String str("Hello");`
`128`	`128`	`str = buffer;`
`129`		`- REQUIRE(str.compareTo("Hello") == 0);`
	`129`	`+ REQUIRE(str.compareTo("Hello") != 0);`
`130`	`130`	`}`
`131`	`131`
`132`	`132`	`TEST_CASE ("Testing & String::operator = (const String &) with invalid buffer of first string", "[String-operator+-15]")`