Skip to content

iccApplyToLink's transform-type range check is unreachable: nType is always 0-9, so icXformLutSpectral..icXformLutNamedDevice (0xA-0xD) cannot be selected from the CLI (iccApplyToLink.cpp:1057 vs :1082) #2270

Description

@colourbill-ctrl

Describe the Bug

iccApplyToLink's transform-type range check is unreachable, and as a consequence
icXformLutSpectral (0xA) through icXformLutNamedDevice (0xD) cannot be selected from
that command line at all.

Tools/CmdLine/IccApplyToLink/iccApplyToLink.cpp:1057 computes the transform type as

nIntent = nIntent % 100;
nType = abs(nIntent) / 10;

so nType is abs(0..99) / 10, i.e. always 0..9. The check at :1082

if (nType < (int)icXformLutMinimum || nType > (int)icXformLutMaximum) {
  printf("Invalid rendering intent '%s': decoded transform type is out of range\n", ...);

therefore tests two bounds that no input can cross: icXformLutMinimum is 0 and abs()
already guarantees non-negative, while icXformLutMaximum is 0xD (13) and the tens
column tops out at 9. The message is dead code.

The check's existence implies the author expected 0..13 to be reachable.

Correction (2026-09-01). This paragraph originally said the four types in
IccCmm.h:141-145 above 9 — icXformLutSpectral, icXformLutNamedColorimetric,
icXformLutNamedSpectral, icXformLutNamedDevice — "have no CLI spelling."
That is wrong. They have no spelling on this tool, but they are reachable
from iccApplyNamedCmm, iccApplyProfiles and iccApplySearch, because
CIccCfgProfileSequence::fromArgs strips % 1000 rather than % 100, so its
hundreds column feeds nType. Measured on master 12038392 with
iccApplyNamedCmm -exportcfg:

code "transform"
100 spectral
110 namedColorimetric
120 namedSpectral
130 namedDevice
140 rejected — by the very bound that is dead in iccApplyToLink

This strengthens option (a) rather than weakening it: the hundreds column
already means two different things across the tool set (100 is a spectral
transform to one family and a luminance request to the other), so widening the
tens column here would silently redefine every code from 100 up. It also means
the identical check in IccCmmConfig.cpp is not dead — only its lower half
is, which abs() hides from CodeQL.

This predates and is untouched by #2269. CodeQL does not flag it; cpp/constant-comparison
did fire on the structurally identical dead bounds in IccCmmConfig.cpp
(#2357/#2358/#2359), so the abs() is likely what hides it from the query.

Build Instructions

cmake -S Build/Cmake -B build -DCMAKE_BUILD_TYPE=Release
cmake --build build -j4 --target iccApplyToLink

Reproduce

Sweep every intent code 0..999 and tally which rejection fires. Measured on master
e23426a0, clang-18 Release:

for c in $(seq 0 999); do
  build/Tools/IccApplyToLink/iccApplyToLink /tmp/sw.icc 0 2 1 t 0 1 0 0 \
    Testing/sRGB_v4_ICC_preference.icc $c 2>&1 |
  grep -oE "decoded (intent|transform type) is out of range"
done | sort | uniq -c
    600 decoded intent is out of range
    400 (accepted)

decoded transform type is out of range appears zero times in 1000 codes. The 600/400
split is exactly the units digit 4..9 vs 0..3, confirming the tens and hundreds columns
never cause a rejection.

Corollary — the tens column saturates, so the higher columns cannot reach it. All of these
produce a byte-identical device link (compared after zeroing dateTime 24..35 and
profileID 84..99):

code normalized sha256[:12]
40 05eda91123ec
140 05eda91123ec
240 05eda91123ec
940 05eda91123ec
1040 05eda91123ec
0 (control) e6d6ed1485c3

The control differs, so the comparison is not blind.

Expected Behavior

This needs a ruling before a patch, because two readings are defensible and they lead to
opposite changes:

(a) The types above 9 were never meant to be CLI-selectable. Then the check at :1082
is simply dead and should be removed, as #2267 removed the dead intent bound. Small, and
the tool's surface is unchanged.

(b) 0..13 was the intent, and the decode is too narrow. Then the tens column needs to
widen — which changes what existing codes mean and is not backward compatible, so it would
want a deliberate spelling rather than an in-place widening.

I have the analysis for either and can open the PR once you rule. I'd lean (a): the shared
CIccCfgProfile::fromArgs decode in IccCmmConfig.cpp has the same single-digit type
column, so widening only this tool would create exactly the cross-tool divergence #2268
and #2190 were about.

Environment

  • OS: Ubuntu 24.04 (WSL2), clang-18 Release, master e23426a0

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

BuildBuild system, CMake, compiler, or packagingJSONICC JSON parser, config, or round-trip behaviorQAMaintainer indicates topic of Quality AssuranceTestingCTest, regression, or test coveragebugConfirmed or suspected defectciContinuous integration workflow changesfeatureFeature request or enhancementneeds-triageMaintainer triage required

Type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions