Skip to content

Commit

Permalink
Fix String::compareTo(const char*) for invalid strings
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
matthijskooijman committed Jan 25, 2021
1 parent f7311d9 commit a675d5a
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 5 deletions.
2 changes: 1 addition & 1 deletion api/String.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions test/src/String/test_String.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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]")
Expand Down Expand Up @@ -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);
}
12 changes: 10 additions & 2 deletions test/src/String/test_operators.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,20 @@ TEST_CASE ("Testing & String::operator = (const String &) with invalid buffer of
REQUIRE(str1.compareTo(str2) == 0);
}

TEST_CASE ("Testing & String::operator = (const char *)", "[String-operator+-14]")
TEST_CASE ("Testing & String::operator = (const char *) with NULL does not leave string unchanged", "[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 char *) with NULL produces invalid string", "[String-operator+-14]")
{
char *buffer = NULL;
arduino::String str("Hello");
str = buffer;
REQUIRE_FALSE(str);
}

TEST_CASE ("Testing & String::operator = (const String &) with invalid buffer of first string", "[String-operator+-15]")
Expand Down

0 comments on commit a675d5a

Please sign in to comment.