From 9597199bd697e22caa578e49137d66a903382992 Mon Sep 17 00:00:00 2001 From: waelma Date: Thu, 13 Aug 2026 20:03:56 +0100 Subject: [PATCH] fix: avoid cctype UB, dedupe msvc-runtime errors, use std::any_of --- src/cmake_generator.cpp | 25 +++++++++++-------------- src/project_parser.cpp | 40 +++++++++++++++++++++++----------------- 2 files changed, 34 insertions(+), 31 deletions(-) diff --git a/src/cmake_generator.cpp b/src/cmake_generator.cpp index a66710d..314d728 100644 --- a/src/cmake_generator.cpp +++ b/src/cmake_generator.cpp @@ -5,6 +5,7 @@ #include "fs.hpp" #include "project_parser.hpp" #include +#include // Wael-MA: we call isspace below, better to include it explicitly #include #include #include @@ -765,13 +766,10 @@ void generate_cmake(const char *path, const parser::Project *parent_project) { } auto contains_language_source = [&project_extensions](const std::vector &sources) { - for (const auto &source : sources) { - auto extension = fs::path(source).extension().string(); - if (project_extensions.count(extension) > 0) { - return true; - } - } - return false; + // Wael-MA: swapped the manual scan for std::any_of, behavior is the same + return std::any_of(sources.begin(), sources.end(), [&project_extensions](const std::string &source) { + return project_extensions.count(fs::path(source).extension().string()) > 0; + }); }; Generator gen(project, path); @@ -1244,12 +1242,10 @@ void generate_cmake(const char *path, const parser::Project *parent_project) { auto has_include_after = false; { auto has_include = [](const parser::ConditionVector &includes) { - for (const auto &itr : includes) { - for (const auto &jtr : itr.second) { - return true; - } - } - return false; + // Wael-MA: std::any_of does the same scan, just a lot easier to read + using value_type = std::pair>; + return std::any_of(includes.begin(), includes.end(), + [](const value_type &itr) { return !itr.second.empty(); }); }; auto has_include_helper = [&](const parser::Target &target) { if (!target.cmake_before.empty() || has_include(target.include_before)) { @@ -1935,7 +1931,8 @@ void generate_cmake(const char *path, const parser::Project *parent_project) { auto generated_cmake = ss.str(); // Make sure the file ends in a single newline - while (!generated_cmake.empty() && std::isspace(generated_cmake.back())) { + // Wael-MA: isspace must receive an unsigned char, plain chars can be negative + while (!generated_cmake.empty() && std::isspace(static_cast(generated_cmake.back()))) { generated_cmake.pop_back(); } generated_cmake += '\n'; diff --git a/src/project_parser.cpp b/src/project_parser.cpp index 95bb6ea..6d335fe 100644 --- a/src/project_parser.cpp +++ b/src/project_parser.cpp @@ -31,6 +31,19 @@ static MsvcRuntimeType parse_msvcRuntimeType(const std::string &name) { return msvc_last; } +static std::string msvc_runtime_error(const std::string &runtime) { + // Wael-MA: one shared spot for this error text, it was copy-pasted in two places + std::string error = "Unknown runtime '" + runtime + "'\n"; + error += "Available types:\n"; + for (const char *type_name : msvcRuntimeTypeNames) { + error += " - "; + error += type_name; + error += '\n'; + } + error.pop_back(); // Remove last newline + return error; +} + using TomlBasicValue = toml::basic_value; static std::string format_key_message(const std::string &message, const toml::key &ky, const TomlBasicValue &value) { @@ -419,13 +432,7 @@ Project::Project(const Project *parent, const std::string &path, bool build) : p if (!msvc_runtime.empty()) { project_msvc_runtime = parse_msvcRuntimeType(msvc_runtime); if (project_msvc_runtime == msvc_last) { - std::string error = "Unknown runtime '" + msvc_runtime + "'\n"; - error += "Available types:\n"; - for (std::string type_name : msvcRuntimeTypeNames) { - error += " - " + type_name + "\n"; - } - error.pop_back(); // Remove last newline - throw_key_error(error, msvc_runtime, project.find("msvc-runtime")); + throw_key_error(msvc_runtime_error(msvc_runtime), msvc_runtime, project.find("msvc-runtime")); } } } @@ -657,7 +664,9 @@ Project::Project(const Project *parent, const std::string &path, bool build) : p auto is_cmake_arg = [](const std::string &s) { for (auto c : s) { - if (!(std::isdigit(c) || std::isupper(c) || c == '_')) { + // Wael-MA: cast to unsigned char first, functions are UB on negative bytes + auto uc = static_cast(c); + if (!(std::isdigit(uc) || std::isupper(uc) || c == '_')) { return false; } } @@ -695,7 +704,8 @@ Project::Project(const Project *parent, const std::string &path, bool build) : p throw_key_error("Empty hash value", argItr.first, argItr.second); } for (char c : value) { - if (!std::isxdigit(c)) { + // Wael-MA: isxdigit also wants an unsigned char, otherwise it's the same UB + if (!std::isxdigit(static_cast(c))) { throw_key_error("Hash value must be a hex string", argItr.first, argItr.second); } } @@ -1098,19 +1108,13 @@ Project::Project(const Project *parent, const std::string &path, bool build) : p target.properties[cond_itr.first]["MSVC_RUNTIME_LIBRARY"] = "MultiThreaded$<$:Debug>"; break; default: { - std::string error = "Unknown runtime '" + cond_itr.second + "'\n"; - error += "Available types:\n"; - for (std::string type_name : msvcRuntimeTypeNames) { - error += " - " + type_name + "\n"; - } - error.pop_back(); // Remove last newline const TomlBasicValue *report; if (cond_itr.first.empty()) { report = &t.find("msvc-runtime"); } else { report = &t.find(cond_itr.first).as_table().find("msvc-runtime").value(); } - throw_key_error(error, cond_itr.second, *report); + throw_key_error(msvc_runtime_error(cond_itr.second), cond_itr.second, *report); } } } @@ -1308,7 +1312,9 @@ bool Project::cmake_minimum_version(int major, int minor) const { bool Project::is_condition_name(const std::string &name) { for (auto ch : name) { - if (!std::isalnum(ch) && ch != '-' && ch != '_') { + // Wael-MA: casting to unsigned char keeps isalnum well-defined for any input byte + auto uc = static_cast(ch); + if (!std::isalnum(uc) && ch != '-' && ch != '_') { return false; } }