From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/colorop: make lut(1/3)d_interpolation mutable
Date: Mon, 25 May 2026 22:24:20 +1000
Message-ID: <review-patch4-20260519211111.228303-5-mwen@igalia.com>
In-Reply-To: <20260519211111.228303-5-mwen@igalia.com>
References: <20260519211111.228303-1-mwen@igalia.com> <20260519211111.228303-5-mwen@igalia.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

Patch Review

**Status: Has critical bugs =E2=80=94 breaks build**

The design is correct: move `lut1d_interpolation` and `lut3d_interpolation`=
 from `struct drm_colorop` to `struct drm_colorop_state`, update getters/se=
tters/print to use `state->` instead of `colorop->`, and initialize default=
s in `__drm_colorop_state_reset`. The header and `drm_atomic.c`/`drm_atomic=
_uapi.c` changes are all correct.

**Bug 1 =E2=80=94 spurious semicolon (lut1d):**
```c
if(!drm_object_property_get_default_value(&colorop->base,
                      colorop->lut1d_interpolation_property,
                      &val));
    colorop_state->lut1d_interpolation =3D val;
```
The trailing `;` after the `)` makes the `if` body empty. The assignment ex=
ecutes unconditionally, using whatever `val` contained. When `drm_object_pr=
operty_get_default_value` fails, `val` is uninitialized garbage. In practic=
e, this function probably always succeeds here (property was just attached)=
, so it's accidentally correct =E2=80=94 but it's still wrong code and will=
 trigger compiler warnings.

**Bug 2 =E2=80=94 syntax error (lut3d):**
```c
if(!drm_object_property_get_default_value(&colorop->base,
                      colorop->lut3d_interpolation_property,
                      &val);
    colorop_state->lut3d_interpolation =3D val;
```
This has `&val);` =E2=80=94 the `)` closes the function call, `;` appears i=
nside the `if(...)` condition before the closing `)` of the `if`. **This is=
 a compile error** =E2=80=94 the `if` parenthesis is never properly closed.

Both are fixed by Patch 5, but Patch 4 alone doesn't compile, **breaking gi=
t bisect**. The fix from Patch 5 should be folded into this patch.

Also, minor style: `if(!` should have a space: `if (!` per kernel coding st=
yle.

---

---
Generated by Claude Code Patch Reviewer