Merged
Conversation
Closed
Member
Author
Status2026-04-23 12:12:47 UTC Adding a regression check to warn IF a Readme.md is modified |
ChrisCoxArt
approved these changes
Apr 23, 2026
xsscx
added
Pending Merge
Mirror the IccProfileJson type-check pattern (commit 7c720cc) at four analogous unguarded .get<std::string>() sites in IccTagJson.cpp: - struct member sameAs (was the direct analog of the IccProfileJson fix) - struct member PrivateType typeSig - array element PrivateType sig - dict locale language/country/text fields Each call site now checks .is_string() before .get<std::string>() and emits a parseStr error on type mismatch, preventing nlohmann::json:: type_error (302) -> SIGABRT on profiles whose JSON encoding supplies non-string values for these fields. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Three remaining sites in IccTagXml.cpp use file->GetLength() (size_t) to size buffers and feed downstream icUInt32Number consumers without the icXmlValidateFileCount() guard introduced in commit 90bfcd2. - icXmlParseTextString TextData File= sidecar (line ~240) - CIccTagXmlTextDescription TextDescription File= sidecar (line ~441) - CIccTagXmlFloatNum text-format sidecar (line ~1536) Each site now validates the file length against UINT32_MAX via the shared helper and aborts with a parseStr error on overflow, matching the established pattern in the patched curve segment and binary CLUT loaders. Also corrected two latent defects in the same hunks: - missing NUL terminator after malloc(n+1) + ReadLine(buf, n) in two text-sidecar paths (icUtf8ToAnsi() / CIccUTF16String() require a NUL-terminated C string but only the +1 slot was reserved, never written, leaving an OOB read on the trailing byte). a NULL deref on OOM; reordered after the check. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CIccTagXmlTextDescription::ParseXml had two bugs in the same statement:
1. memcpy(m_szScriptText, buf, 67) is hardcoded to 67 bytes, but buf
is malloc(fileLength + 1). When the referenced file is < 66 bytes,
ASAN reports a heap-buffer-overflow READ of size 67 reading past
the end of buf.
2. m_nScriptSize = (icUInt8Number) fileLength + 1 truncates fileLength
through an 8-bit cast: a 300-byte sidecar yields m_nScriptSize = 45,
silently corrupting the resulting profile.
Fix: clamp the source read to min(fileLength, 67) (the destination capacity
per ICC v2 textDescriptionType ScriptCode field), zero the destination
first, and clamp m_nScriptSize before the icUInt8Number narrowing.
Discovered while validating the malloc(fileLength+1) fix in 170379b
(ReadLine OOB write in the same function).
PoC: minimal v2 mntr profile with <textDescriptionType><TextData
File="/tmp/poc.txt"/> where /tmp/poc.txt is 5 bytes "hello".
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Updated README to improve formatting and clarity of API documentation and IccJSON section.
Adds virtual ~Class() {} to seven interface/base classes so that polymorphic
deletion through the base pointer is well-defined and -Wnon-virtual-dtor stops
firing across the tree. With -Wall -Wextra -Wnon-virtual-dtor this eliminates
all 262 instances reported on a clean Release build:
IIccExtensionTag (IccProfLib/IccTagBasic.h)
IIccExtensionMpe (IccProfLib/IccTagMPE.h)
CIccEvalCompare (IccProfLib/IccEval.h)
IIccMatrixSolver (IccProfLib/IccSolve.h)
IIccMatrixInverter (IccProfLib/IccSolve.h)
IIccOpenFileIO (IccXML/IccLibXML/IccIoXml.h)
IIccJsonOpenFileIO (IccJSON/IccLibJSON/IccIoJson.h)
Derived classes (CIccStandardFileIO, CIccJsonStandardFileIO,
CIccSimpleMatrixSolver, CIccSimpleMatrixInverter, CIccMinMaxEval in
iccRoundTrip.cpp and wxProfileDump.cpp) inherit the now-virtual destructor
implicitly and require no further changes.
This is a prerequisite for adding -Werror to the default Linux/macOS warning
set in Build/Cmake/CMakeLists.txt.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- IccMpeXml.cpp: rename inner pNode→pCurveNode/pTfNode in ToneMap loops (also fixes pre-existing latent bug where pNode->name in inner block referenced loop iterator only because of the shadow) - IccProfileXml.cpp: remove redundant inner char str[100] declaration (outer str[strSize] already in scope, same size) - IccTagXml.cpp: drop redundant inner xmlAttr *attr; use outer in 3 sites - iccApplyToLink.cpp: rename inner 'auto i' → 'auto si' (2 loops) - iccApplyNamedCmm.cpp: rename inner 'icUInt32Number i' → 'si' - IccCmmConfig.cpp: rename inner pEntry → pCurEntry (and references) Eliminates 10 -Wshadow warnings under -Wall -Wextra -Wnon-virtual-dtor -Wuninitialized -Werror=uninitialized -Wshadow -Wnull-dereference -Wundef -Wpointer-arith Build clean (0 warnings) on clang-18. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ten) Adds the following to platform compile_options: -Wnon-virtual-dtor -Wuninitialized -Werror=uninitialized -Wshadow -Wnull-dereference -Wundef -Wpointer-arith GCC-only (gated by CMAKE_CXX_COMPILER_ID): -Wlogical-op Adds two opt-in cmake options for future cleanup work: -DENABLE_STRICT_EFFC=ON -> -Weffc++ (~3000 sites) -DENABLE_DOUBLE_PROMOTION=ON -> -Wdouble-promotion (~2000 sites) Verified: 0 warnings, 0 errors on a clean Release build with clang-18 on Linux against the safe default set. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
xsscx
force-pushed
the
Bisect-1f0a9dd-size_t
branch
from
eb7f791 to
3c1c8fb
Compare
xsscx
added
Pending Merge
…r=maybe-uninitialized ParseXml path declared icResponse16Number response; then conditionally wrote .reserved only when the XML Reserved attribute was present. GCC 15's tighter -Wmaybe-uninitialized analysis flagged the subsequent std::vector::push_back as reading an uninitialized field, breaking the ci-docker (ubuntu 26.04 / GCC 15) build now that -Werror=uninitialized is enabled by the strict-warnings change. Aggregate-zero-init the POD. Matches ICC spec which requires the reserved field to be zero unless explicitly set. clang-18 / GCC 14 already produce identical machine code; only the GCC 15 diagnostic changes. Reproduced locally via docker build -f Dockerfile (ubuntu:26.04). Both Dockerfile and Dockerfile.nixos now build clean (0 errors, 0 warnings). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CIccSearchVec(unsigned int n) : n(n) shadowed the member 'n' of the same class, triggering -Wshadow under the strict-warnings baseline. Rename the constructor parameter to nElems; semantics unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CMakeLists.txt: split warning options into a base hardening set (works on any C++17 compiler) and a strict tier (-Werror=uninitialized, -Wshadow, -Wnull-dereference, -Wundef, -Wpointer-arith, GCC -Wlogical-op) that only activates on GCC >=15 or Clang >=14. CMake prints the detected tier per platform (macOS, Linux, Emscripten). Older toolchains still build cleanly with reduced diagnostics; CI (ubuntu:26.04 GCC 15, clang-18 Tool Tests) exercises the strict tier so contributors who match either compiler see the same diagnostics CI sees. Verified locally: - Clang 18.1.3 -> 'strict warnings ENABLED', clean build - GCC 14.2.0 -> 'strict warnings DISABLED -- below floor', clean build Documentation: add Compiler Requirements tables to docs/build.md, CONTRIBUTING.md, AGENTS.md, examples/hello-iccdev/README.md, build-system.instructions.md, docs/bisect.md, copilot-instructions.md. State the recommended floor (GCC 15 / Clang 14 / MSVC 19.40) and the absolute minimum (GCC 11 / Clang 10 / MSVC 19.30). Document opt-in flags ENABLE_STRICT_EFFC and ENABLE_DOUBLE_PROMOTION. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
xsscx
removed
Pending Merge
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Checklist
#842 #843