Re: [PATCH 0/5] Revert cleanups for IS_ERR_OR_NULL() usage
From: Timur Tabi
Date: Thu May 28 2026 - 16:31:51 EST
On Thu, 2026-05-28 at 21:44 +0200, Danilo Krummrich wrote:
>
> @Timur: I do think cleaning this up is the right call in general though, and I
> also don't think that the whole driver necessarily needs to be consistent on
> whether IS_ERR_OR_NULL() or IS_ERR() is used -- it depends on the context
> (although I usually prefer not to mix up NULL and ERR semantics in the first
> place).
>
> It should however be consistent in terms of what functions can actually return.
>
> ret = foo();
> if (IS_ERR_OR_NULL(ret))
> return ret;
>
> If foo() can never return NULL, the above is misleading, as it puts an
> obligation on the caller to somehow handle the NULL case and come up with an
> actual error code for it.
Sure, I get that. My point is that it's often not clear whether foo() actually can never return
NULL.
It's been a while since I've dug through the RPC call chains in Nouveau, so my memory is a little
hazy here. I do remember noticing that Nouveau frequently has situations where foo() call bar1()
and bar2(), where bar1() can return NULL but bar2() can't. So the question is not whether foo() can
return NULL, it's whether bar1() should not return NULL, or whether bar2() should.
> So, I think it is the right call to align that to what functions can actually
> return, but while doing this, the contract should be properly documented, such
> that subsequent changes can be properly validated.
"Properly documented" and "Nouveau" are not two things that go together.