danmar · firewave · Oct 20, 2023
diff --git a/lib/platform.cpp b/lib/platform.cpp
@@ -50,11 +50,7 @@ bool Platform::set(Type t)
         sizeof_wchar_t = sizeof(wchar_t);
         sizeof_size_t = sizeof(std::size_t);
         sizeof_pointer = sizeof(void *);
-        if (type == Type::Unspecified) {
-            defaultSign = '\0';
-        } else {
-            defaultSign = std::numeric_limits<char>::is_signed ? 's' : 'u';
-        }
+        defaultSign = std::numeric_limits<char>::is_signed ? 's' : 'u';
         char_bit = 8;
         short_bit = char_bit * sizeof_short;
         int_bit = char_bit * sizeof_int;
@@ -75,7 +71,7 @@ bool Platform::set(Type t)
         sizeof_wchar_t = 2;
         sizeof_size_t = 4;
         sizeof_pointer = 4;
-        defaultSign = '\0';
+        defaultSign = 's';
         char_bit = 8;
         short_bit = char_bit * sizeof_short;
         int_bit = char_bit * sizeof_int;
@@ -95,7 +91,7 @@ bool Platform::set(Type t)
         sizeof_wchar_t = 2;
         sizeof_size_t = 8;
         sizeof_pointer = 8;
-        defaultSign = '\0';
+        defaultSign = 's';
         char_bit = 8;
         short_bit = char_bit * sizeof_short;
         int_bit = char_bit * sizeof_int;
@@ -115,7 +111,7 @@ bool Platform::set(Type t)
         sizeof_wchar_t = 4;
         sizeof_size_t = 4;
         sizeof_pointer = 4;
-        defaultSign = '\0';
+        defaultSign = 's';
         char_bit = 8;
         short_bit = char_bit * sizeof_short;
         int_bit = char_bit * sizeof_int;
@@ -135,7 +131,7 @@ bool Platform::set(Type t)
         sizeof_wchar_t = 4;
         sizeof_size_t = 8;
         sizeof_pointer = 8;
-        defaultSign = '\0';
+        defaultSign = 's';
         char_bit = 8;
         short_bit = char_bit * sizeof_short;
         int_bit = char_bit * sizeof_int;
@@ -165,7 +161,10 @@ bool Platform::set(const std::string& platformstr, std::string& errstr, const st
     else if (platformstr == "native")
         set(Type::Native);
     else if (platformstr == "unspecified")
+    {
+        std::cout << "Platform 'unspecified' is deprecated and will be removed in a future version. It is also now identical to 'native' (i.e. char type signedness based on compiler instead of unknown)." << std::endl;
         set(Type::Unspecified);
+    }
     else if (paths.empty()) {
         errstr = "unrecognized platform: '" + platformstr + "' (no lookup).";
         return false;
@@ -175,7 +174,9 @@ bool Platform::set(const std::string& platformstr, std::string& errstr, const st
         for (const std::string& path : paths) {
             if (debug)
                 std::cout << "looking for platform '" + platformstr + "' in '" + path + "'" << std::endl;
-            if (loadFromFile(path.c_str(), platformstr, debug)) {
+            // TODO: differentiate between missing and failure
+            std::string errmsg;
+            if (loadFromFile(path.c_str(), platformstr, errmsg, debug)) {
                 found = true;
                 break;
             }
@@ -189,7 +190,7 @@ bool Platform::set(const std::string& platformstr, std::string& errstr, const st
     return true;
 }
 
-bool Platform::loadFromFile(const char exename[], const std::string &filename, bool debug)
+bool Platform::loadFromFile(const char exename[], const std::string &filename, std::string& errmsg, bool debug)
 {
     const bool is_abs_path = Path::isAbsolute(filename);
 
@@ -237,7 +238,7 @@ bool Platform::loadFromFile(const char exename[], const std::string &filename, b
     if (err != tinyxml2::XML_SUCCESS)
         return false;
 
-    return loadFromXmlDocument(&doc);
+    return loadFromXmlDocument(&doc, errmsg);
 }
 
 static unsigned int xmlTextAsUInt(const tinyxml2::XMLElement* node, bool& error)
@@ -248,19 +249,30 @@ static unsigned int xmlTextAsUInt(const tinyxml2::XMLElement* node, bool& error)
     return retval;
 }
 
-bool Platform::loadFromXmlDocument(const tinyxml2::XMLDocument *doc)
+bool Platform::loadFromXmlDocument(const tinyxml2::XMLDocument *doc, std::string& errmsg)
 {
     const tinyxml2::XMLElement * const rootnode = doc->FirstChildElement();
 
-    if (!rootnode || std::strcmp(rootnode->Name(), "platform") != 0)
+    if (!rootnode)
+    {
+        errmsg = "no root node found";
         return false;
+    }
 
-    bool error = false;
+    if (std::strcmp(rootnode->Name(), "platform") != 0)
+    {
+        errmsg = "invalid root node";
+        return false;
+    }
+
+    // TODO: improve error reporting
+    bool res = true;
     for (const tinyxml2::XMLElement *node = rootnode->FirstChildElement(); node; node = node->NextSiblingElement()) {
+        bool error = false;
         const char* name = node->Name();
         if (std::strcmp(name, "default-sign") == 0) {
             const char* str = node->GetText();
-            if (str)
+            if (str && (*str == 'u' || *str == 's'))
                 defaultSign = *str;
             else
                 error = true;
@@ -293,6 +305,11 @@ bool Platform::loadFromXmlDocument(const tinyxml2::XMLDocument *doc)
                     sizeof_wchar_t = xmlTextAsUInt(sz, error);
             }
         }
+        if (error)
+        {
+            res = false;
+            errmsg = std::string("'") + name + "' failed";
+        }
     }
 
     short_bit = char_bit * sizeof_short;
@@ -301,7 +318,7 @@ bool Platform::loadFromXmlDocument(const tinyxml2::XMLDocument *doc)
     long_long_bit = char_bit * sizeof_long_long;
 
     type = Type::File;
-    return !error;
+    return res;
 }
 
 std::string Platform::getLimitsDefines(bool c99) const

diff --git a/lib/platform.h b/lib/platform.h
@@ -142,10 +142,10 @@ class CPPCHECKLIB Platform {
      * @param debug log verbose information about the lookup
      * @return returns true if file was loaded successfully
      */
-    bool loadFromFile(const char exename[], const std::string &filename, bool debug = false);
+    bool loadFromFile(const char exename[], const std::string &filename, std::string& errmsg, bool debug = false);
 
     /** load platform from xml document, primarily for testing */
-    bool loadFromXmlDocument(const tinyxml2::XMLDocument *doc);
+    bool loadFromXmlDocument(const tinyxml2::XMLDocument *doc, std::string& errmsg);
 
     /**
      * @brief Returns true if platform type is Windows

diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp
@@ -61,12 +61,10 @@ SymbolDatabase::SymbolDatabase(Tokenizer& tokenizer)
     if (!mTokenizer.tokens())
         return;
 
-    if (mSettings.platform.defaultSign == 's' || mSettings.platform.defaultSign == 'S')
-        mDefaultSignedness = ValueType::SIGNED;
-    else if (mSettings.platform.defaultSign == 'u' || mSettings.platform.defaultSign == 'U')
+    if (mSettings.platform.defaultSign == 'u')
         mDefaultSignedness = ValueType::UNSIGNED;
     else
-        mDefaultSignedness = ValueType::UNKNOWN_SIGN;
+        mDefaultSignedness = ValueType::SIGNED;
 
     createSymbolDatabaseFindAllScopes();
     createSymbolDatabaseClassInfo();

diff --git a/releasenotes.txt b/releasenotes.txt
@@ -16,6 +16,7 @@ Deprecations:
 - Support for building with Qt 5 will be removed in Cppcheck 2.19.
 - The platform 'unix32-unsigned' has been deprecated and will be removed in Cppcheck 2.19. Please use '--platform=unix32 --funsigned-char' instead.
 - The platform 'unix64-unsigned' has been deprecated and will be removed in Cppcheck 2.19. Please use '--platform=unix64 --funsigned-char' instead.
+- The "unspecified" platform has been deprecated and will be removed in a future version. It is also now identical to 'native' (i.e. char type signedness based on compiler instead of unknown).
 -
 
 Other:
@@ -28,4 +29,6 @@ Other:
 - Fix checking a project that contains several project file entries for the same file.
 - Fixed --file-filter matching of looked up files in provided paths.
 - Split up cstyleCast checker; dangerous casts produce portability/warning reports, safe casts produce style reports.
+- The possibility of an unknown signedness of the char type in a platform definition has been removed.
+- The built-in "win*" and "unix*" platforms will now default to signed char type instead of unknown signedness. If you require unsigned chars please specify "--funsigned-char"
 -
diff --git a/test/cli/other_test.py b/test/cli/other_test.py
@@ -3545,3 +3545,88 @@ def test_debug_syntaxerror_c(tmp_path):
     assert stderr.splitlines() == [
         "{}:2:1: error: Code 'template<...' is invalid C code. [syntaxError]".format(test_file)
     ]
+
+
+def test_platform_custom(tmp_path):
+    test_cfg = tmp_path / 'test.cfg'
+    with open(test_cfg, 'wt') as f:
+        f.write("""
+<?xml version="1.0"?>
+<platform>
+    <char_bit>8</char_bit>
+    <default-sign>unsigned</default-sign>
+    <sizeof>
+        <bool>1</bool>
+        <short>2</short>
+        <int>2</int>
+        <long>4</long>
+        <long-long>8</long-long>
+        <float>4</float>
+        <double>4</double>
+        <long-double>4</long-double>
+        <pointer>2</pointer>
+        <size_t>2</size_t>
+        <wchar_t>2</wchar_t>
+    </sizeof>
+</platform>
+        """)
+
+    # TODO: use a sample to make sure the file is actually used
+    test_file = tmp_path / 'test.c'
+    with open(test_file, 'wt') as f:
+        f.write("""""")
+
+    args = [
+        '--platform={}'.format(test_cfg),
+        str(test_file)
+    ]
+
+    exitcode, stdout, stderr = cppcheck(args)
+    assert exitcode == 0, stdout
+    lines = stdout.splitlines()
+    assert lines == [
+        'Checking {} ...'.format(test_file)
+    ]
+    assert stderr == ''
+
+
+def test_platform_invalid_defaultsign(tmp_path):
+    test_cfg = tmp_path / 'test.cfg'
+    with open(test_cfg, 'wt') as f:
+        f.write("""
+<?xml version="1.0"?>
+<platform>
+    <char_bit>8</char_bit>
+    <default-sign>notsigned</default-sign>
+    <sizeof>
+        <bool>1</bool>
+        <short>2</short>
+        <int>2</int>
+        <long>4</long>
+        <long-long>8</long-long>
+        <float>4</float>
+        <double>4</double>
+        <long-double>4</long-double>
+        <pointer>2</pointer>
+        <size_t>2</size_t>
+        <wchar_t>2</wchar_t>
+    </sizeof>
+</platform>
+        """)
+
+    test_file = tmp_path / 'test.c'
+    with open(test_file, 'wt') as f:
+        f.write("""""")
+
+    args = [
+        '--platform={}'.format(test_cfg),
+        str(test_file)
+    ]
+
+    exitcode, stdout, stderr = cppcheck(args)
+    assert exitcode == 0, stdout
+    lines = stdout.splitlines()
+    assert lines == [
+        'Checking {} ...'.format(test_file)
+    ]
+    assert stderr == ''
diff --git a/test/testcmdlineparser.cpp b/test/testcmdlineparser.cpp
@@ -1641,39 +1641,34 @@ class TestCmdlineParser : public TestFixture {
     void platformWin64() {
         REDIRECT;
         const char * const argv[] = {"cppcheck", "--platform=win64", "file.cpp"};
-        ASSERT(settings->platform.set(Platform::Type::Unspecified));
         ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parseFromArgs(argv));
         ASSERT_EQUALS(Platform::Type::Win64, settings->platform.type);
     }
 
     void platformWin32A() {
         REDIRECT;
         const char * const argv[] = {"cppcheck", "--platform=win32A", "file.cpp"};
-        ASSERT(settings->platform.set(Platform::Type::Unspecified));
         ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parseFromArgs(argv));
         ASSERT_EQUALS(Platform::Type::Win32A, settings->platform.type);
     }
 
     void platformWin32W() {
         REDIRECT;
         const char * const argv[] = {"cppcheck", "--platform=win32W", "file.cpp"};
-        ASSERT(settings->platform.set(Platform::Type::Unspecified));
         ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parseFromArgs(argv));
         ASSERT_EQUALS(Platform::Type::Win32W, settings->platform.type);
     }
 
     void platformUnix32() {
         REDIRECT;
         const char * const argv[] = {"cppcheck", "--platform=unix32", "file.cpp"};
-        ASSERT(settings->platform.set(Platform::Type::Unspecified));
         ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parseFromArgs(argv));
         ASSERT_EQUALS(Platform::Type::Unix32, settings->platform.type);
     }
 
     void platformUnix32Unsigned() {
         REDIRECT;
         const char * const argv[] = {"cppcheck", "--platform=unix32-unsigned", "file.cpp"};
-        ASSERT(settings->platform.set(Platform::Type::Unspecified));
         ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parseFromArgs(argv));
         ASSERT_EQUALS(Platform::Type::Unix32, settings->platform.type);
         ASSERT_EQUALS("cppcheck: The platform 'unix32-unsigned' has been deprecated and will be removed in Cppcheck 2.19. Please use '--platform=unix32 --funsigned-char' instead\n", logger->str());
@@ -1682,15 +1677,13 @@ class TestCmdlineParser : public TestFixture {
     void platformUnix64() {
         REDIRECT;
         const char * const argv[] = {"cppcheck", "--platform=unix64", "file.cpp"};
-        ASSERT(settings->platform.set(Platform::Type::Unspecified));
         ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parseFromArgs(argv));
         ASSERT_EQUALS(Platform::Type::Unix64, settings->platform.type);
     }
 
     void platformUnix64Unsigned() {
         REDIRECT;
         const char * const argv[] = {"cppcheck", "--platform=unix64-unsigned", "file.cpp"};
-        ASSERT(settings->platform.set(Platform::Type::Unspecified));
         ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parseFromArgs(argv));
         ASSERT_EQUALS(Platform::Type::Unix64, settings->platform.type);
         ASSERT_EQUALS("cppcheck: The platform 'unix64-unsigned' has been deprecated and will be removed in Cppcheck 2.19. Please use '--platform=unix64 --funsigned-char' instead\n", logger->str());
@@ -1699,23 +1692,20 @@ class TestCmdlineParser : public TestFixture {
     void platformNative() {
         REDIRECT;
         const char * const argv[] = {"cppcheck", "--platform=native", "file.cpp"};
-        ASSERT(settings->platform.set(Platform::Type::Unspecified));
         ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parseFromArgs(argv));
         ASSERT_EQUALS(Platform::Type::Native, settings->platform.type);
     }
 
     void platformUnspecified() {
         REDIRECT;
         const char * const argv[] = {"cppcheck", "--platform=unspecified", "file.cpp"};
-        ASSERT(settings->platform.set(Platform::Type::Native));
         ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parseFromArgs(argv));
         ASSERT_EQUALS(Platform::Type::Unspecified, settings->platform.type);
     }
 
     void platformPlatformFile() {
         REDIRECT;
         const char * const argv[] = {"cppcheck", "--platform=avr8", "file.cpp"};
-        ASSERT(settings->platform.set(Platform::Type::Unspecified));
         ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parseFromArgs(argv));
         ASSERT_EQUALS(Platform::Type::File, settings->platform.type);
     }

diff --git a/test/testplatform.cpp b/test/testplatform.cpp
@@ -52,7 +52,8 @@ class TestPlatform : public TestFixture {
 
     static bool readPlatform(Platform& platform, const char* xmldata) {
         tinyxml2::XMLDocument doc;
-        return (doc.Parse(xmldata) == tinyxml2::XML_SUCCESS) && platform.loadFromXmlDocument(&doc);
+        std::string errmsg; // TODO: use this
+        return (doc.Parse(xmldata) == tinyxml2::XML_SUCCESS) && platform.loadFromXmlDocument(&doc, errmsg);
     }
 
     void empty() const {