On Thu May 28, 2026 at 10:23 PM CEST, Timur Tabi wrote: > 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.
If there are multiple, it has to be the superset of course. >> 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. Unfortunately -- but the changes submitted by Hongling can add the documentation for the places that are touched. @Hongling, can you consider this in a v2 please? Thanks, Danilo
