On Wed, 11 Feb 2026 at 11:45, Dave Airlie <[email protected]> wrote:
>
> Hi all,
> This is a public inbox, you can also git clone
>
> https://gitlab.freedesktop.org/drm/ai-reviews-public-inbox
What an odd format that is, and I don't have anything that reads it
natively, so I just did a one-liner script for it:
git show $(git rev-list HEAD | sed 's/$/:m/')
and having done that I think the review output format is not exactly
lovely, but whatever.
But the details in reviews look mostly pretty good to me. I don't know
the code in question well enough to say whether they are useful to
*you*, but it certainly doesn't look bad to me.
The one review I reacted to was because I *do* know the code enough.
So when Claude reacted to this nonsense patch with that whole "Fix
passing zero to ERR_PTR" thing:
- return ERR_PTR(err);
+ return err ? ERR_PTR(err) : NULL;
*muy* reaction is that it's the opposite of a "fix" and we should get
rid of that whole
err ? ERR_PTR(err) : NULL
pattern, and just admit that ERR_PTR(err) works fine for 0.
I don't know who started doing that "turn 0 into NULL explicitly"
originally or why, but we certainly shouldn't add to them.
Yes, Claude seems to think that ERR_PTR(0) is "erroneous" and somehow
different from NULL, and maybe that's where some of the existing users
came from? People who already used claude and got the wrong impression
from that?
Claude is correct that casting a non-constant zero value to a pointer
*technically* isn't NULL according to the C standards. But we don't do
standard C, and the fact is, we depend on NULL being not just
"constant 0" at compile time, but also actually have the (dynamic)
value of zero.
However, Claude's other reaction to it is actually much more
interesting than my initial "that's just a nonsense patch". So while
I disagree with the claude on a detail, I don't think it was overall
bad.
Linus
