Fixed incorrect string comparison on some platforms (fixes #1198)

This commit is contained in:
Benoit Blanchon
2020-02-27 11:44:09 +01:00
parent 2996503b27
commit 2641697e0b
10 changed files with 110 additions and 57 deletions

View File

@ -10,6 +10,7 @@ HEAD
* Fixed "deprecated-copy" warning on GCC 9 (fixes #1184)
* Fixed `MemberProxy::set(char[])` not duplicating the string (issue #1191)
* Fixed enums serialized as booleans (issue #1197)
* Fixed incorrect string comparison on some platforms (issue #1198)
v6.14.1 (2020-01-27)
-------

View File

@ -4,9 +4,11 @@
#include "custom_string.hpp"
#include "progmem_emulation.hpp"
#include "weird_strcmp.hpp"
#include <ArduinoJson/Strings/ConstRamStringAdapter.hpp>
#include <ArduinoJson/Strings/FlashStringAdapter.hpp>
#include <ArduinoJson/Strings/SizedRamStringAdapter.hpp>
#include <ArduinoJson/Strings/StlStringAdapter.hpp>
#include <catch.hpp>
@ -17,27 +19,55 @@ TEST_CASE("ConstRamStringAdapter") {
SECTION("null") {
ConstRamStringAdapter adapter(NULL);
REQUIRE(adapter.compare("bravo") < 0);
REQUIRE(adapter.compare(NULL) == 0);
CHECK(adapter.compare("bravo") < 0);
CHECK(adapter.compare(NULL) == 0);
REQUIRE(adapter.equals(NULL));
REQUIRE_FALSE(adapter.equals("charlie"));
CHECK(adapter.equals(NULL));
CHECK_FALSE(adapter.equals("charlie"));
REQUIRE(adapter.size() == 0);
CHECK(adapter.size() == 0);
}
SECTION("non-null") {
ConstRamStringAdapter adapter("bravo");
REQUIRE(adapter.compare(NULL) > 0);
REQUIRE(adapter.compare("alpha") > 0);
REQUIRE(adapter.compare("bravo") == 0);
REQUIRE(adapter.compare("charlie") < 0);
CHECK(adapter.compare(NULL) > 0);
CHECK(adapter.compare("alpha") > 0);
CHECK(adapter.compare("bravo") == 0);
CHECK(adapter.compare("charlie") < 0);
REQUIRE(adapter.equals("bravo"));
REQUIRE_FALSE(adapter.equals("charlie"));
CHECK(adapter.equals("bravo"));
CHECK_FALSE(adapter.equals("charlie"));
REQUIRE(adapter.size() == 5);
CHECK(adapter.size() == 5);
}
}
TEST_CASE("SizedRamStringAdapter") {
SECTION("null") {
SizedRamStringAdapter adapter(NULL, 10);
CHECK(adapter.compare("bravo") < 0);
CHECK(adapter.compare(NULL) == 0);
CHECK(adapter.equals(NULL));
CHECK_FALSE(adapter.equals("charlie"));
CHECK(adapter.size() == 10);
}
SECTION("non-null") {
SizedRamStringAdapter adapter("bravo", 5);
CHECK(adapter.compare(NULL) > 0);
CHECK(adapter.compare("alpha") > 0);
CHECK(adapter.compare("bravo") == 0);
CHECK(adapter.compare("charlie") < 0);
CHECK(adapter.equals("bravo"));
CHECK_FALSE(adapter.equals("charlie"));
CHECK(adapter.size() == 5);
}
}
@ -45,27 +75,27 @@ TEST_CASE("FlashStringAdapter") {
SECTION("null") {
FlashStringAdapter adapter(NULL);
REQUIRE(adapter.compare("bravo") < 0);
REQUIRE(adapter.compare(NULL) == 0);
CHECK(adapter.compare("bravo") < 0);
CHECK(adapter.compare(NULL) == 0);
REQUIRE(adapter.equals(NULL));
REQUIRE_FALSE(adapter.equals("charlie"));
CHECK(adapter.equals(NULL));
CHECK_FALSE(adapter.equals("charlie"));
REQUIRE(adapter.size() == 0);
CHECK(adapter.size() == 0);
}
SECTION("non-null") {
FlashStringAdapter adapter = adaptString(F("bravo"));
REQUIRE(adapter.compare(NULL) > 0);
REQUIRE(adapter.compare("alpha") > 0);
REQUIRE(adapter.compare("bravo") == 0);
REQUIRE(adapter.compare("charlie") < 0);
CHECK(adapter.compare(NULL) > 0);
CHECK(adapter.compare("alpha") > 0);
CHECK(adapter.compare("bravo") == 0);
CHECK(adapter.compare("charlie") < 0);
REQUIRE(adapter.equals("bravo"));
REQUIRE_FALSE(adapter.equals("charlie"));
CHECK(adapter.equals("bravo"));
CHECK_FALSE(adapter.equals("charlie"));
REQUIRE(adapter.size() == 5);
CHECK(adapter.size() == 5);
}
}
@ -73,46 +103,46 @@ TEST_CASE("std::string") {
std::string str("bravo");
StlStringAdapter<std::string> adapter = adaptString(str);
REQUIRE(adapter.compare(NULL) > 0);
REQUIRE(adapter.compare("alpha") > 0);
REQUIRE(adapter.compare("bravo") == 0);
REQUIRE(adapter.compare("charlie") < 0);
CHECK(adapter.compare(NULL) > 0);
CHECK(adapter.compare("alpha") > 0);
CHECK(adapter.compare("bravo") == 0);
CHECK(adapter.compare("charlie") < 0);
REQUIRE(adapter.equals("bravo"));
REQUIRE_FALSE(adapter.equals("charlie"));
CHECK(adapter.equals("bravo"));
CHECK_FALSE(adapter.equals("charlie"));
REQUIRE(adapter.size() == 5);
CHECK(adapter.size() == 5);
}
TEST_CASE("custom_string") {
custom_string str("bravo");
StlStringAdapter<custom_string> adapter = adaptString(str);
REQUIRE(adapter.compare(NULL) > 0);
REQUIRE(adapter.compare("alpha") > 0);
REQUIRE(adapter.compare("bravo") == 0);
REQUIRE(adapter.compare("charlie") < 0);
CHECK(adapter.compare(NULL) > 0);
CHECK(adapter.compare("alpha") > 0);
CHECK(adapter.compare("bravo") == 0);
CHECK(adapter.compare("charlie") < 0);
REQUIRE(adapter.equals("bravo"));
REQUIRE_FALSE(adapter.equals("charlie"));
CHECK(adapter.equals("bravo"));
CHECK_FALSE(adapter.equals("charlie"));
REQUIRE(adapter.size() == 5);
CHECK(adapter.size() == 5);
}
TEST_CASE("IsString<T>") {
SECTION("std::string") {
REQUIRE(IsString<std::string>::value == true);
CHECK(IsString<std::string>::value == true);
}
SECTION("basic_string<wchar_t>") {
REQUIRE(IsString<std::basic_string<wchar_t> >::value == false);
CHECK(IsString<std::basic_string<wchar_t> >::value == false);
}
SECTION("custom_string") {
REQUIRE(IsString<custom_string>::value == true);
CHECK(IsString<custom_string>::value == true);
}
SECTION("const __FlashStringHelper*") {
REQUIRE(IsString<const __FlashStringHelper*>::value == true);
CHECK(IsString<const __FlashStringHelper*>::value == true);
}
}

View File

@ -0,0 +1,23 @@
#include <ArduinoJson/Namespace.hpp>
// Issue #1198: strcmp() implementation that returns a value larger than 8-bit
namespace ARDUINOJSON_NAMESPACE {
int strcmp(const char* a, const char* b) {
int result = ::strcmp(a, b);
if (result > 0)
return 2147483647;
if (result < 0)
return -214748364;
return 0;
}
int strncmp(const char* a, const char* b, size_t n) {
int result = ::strncmp(a, b, n);
if (result > 0)
return 2147483647;
if (result < 0)
return -214748364;
return 0;
}
} // namespace ARDUINOJSON_NAMESPACE

View File

@ -10,23 +10,23 @@
namespace ARDUINOJSON_NAMESPACE {
inline int8_t safe_strcmp(const char* a, const char* b) {
inline int safe_strcmp(const char* a, const char* b) {
if (a == b)
return 0;
if (!a)
return -1;
if (!b)
return 1;
return static_cast<int8_t>(strcmp(a, b));
return strcmp(a, b);
}
inline int8_t safe_strncmp(const char* a, const char* b, size_t n) {
inline int safe_strncmp(const char* a, const char* b, size_t n) {
if (a == b)
return 0;
if (!a)
return -1;
if (!b)
return 1;
return static_cast<int8_t>(strncmp(a, b, n));
return strncmp(a, b, n);
}
} // namespace ARDUINOJSON_NAMESPACE

View File

@ -31,7 +31,7 @@ class ArduinoStringAdapter {
return !_str->c_str();
}
int8_t compare(const char* other) const {
int compare(const char* other) const {
// Arduino's String::c_str() can return NULL
const char* me = _str->c_str();
return safe_strcmp(me, other);

View File

@ -17,7 +17,7 @@ class ConstRamStringAdapter {
public:
ConstRamStringAdapter(const char* str = 0) : _str(str) {}
int8_t compare(const char* other) const {
int compare(const char* other) const {
return safe_strcmp(_str, other);
}

View File

@ -15,14 +15,14 @@ class FlashStringAdapter {
public:
FlashStringAdapter(const __FlashStringHelper* str) : _str(str) {}
int8_t compare(const char* other) const {
int compare(const char* other) const {
if (!other && !_str)
return 0;
if (!_str)
return -1;
if (!other)
return 1;
return int8_t(-strcmp_P(other, reinterpret_cast<const char*>(_str)));
return -strcmp_P(other, reinterpret_cast<const char*>(_str));
}
bool equals(const char* expected) const {

View File

@ -15,15 +15,14 @@ class SizedFlashStringAdapter {
SizedFlashStringAdapter(const __FlashStringHelper* str, size_t sz)
: _str(str), _size(sz) {}
int8_t compare(const char* other) const {
int compare(const char* other) const {
if (!other && !_str)
return 0;
if (!_str)
return -1;
if (!other)
return 1;
return int8_t(
-strncmp_P(other, reinterpret_cast<const char*>(_str), _size));
return -strncmp_P(other, reinterpret_cast<const char*>(_str), _size);
}
bool equals(const char* expected) const {

View File

@ -16,8 +16,8 @@ class SizedRamStringAdapter {
public:
SizedRamStringAdapter(const char* str, size_t n) : _str(str), _size(n) {}
int8_t compare(const char* other) const {
return safe_strncmp(_str, other, _size) == 0;
int compare(const char* other) const {
return safe_strncmp(_str, other, _size);
}
bool equals(const char* expected) const {

View File

@ -30,10 +30,10 @@ class StlStringAdapter {
return false;
}
int8_t compare(const char* other) const {
int compare(const char* other) const {
if (!other)
return 1;
return static_cast<int8_t>(_str->compare(other));
return _str->compare(other);
}
bool equals(const char* expected) const {