Hi Danilo,
I apologize for the confusion with my previous patch submission.
Please disregard the patches I submitted earlier.
I have now regenerated a v2 patch series following your requirements
to properly document return value contracts and clean up IS_ERR_OR_NULL
usage.
Return value analysis:
- r535_gsp_msgq_peek(): Never returns NULL
- r5sp_msgq_recv_one_elem(): Never returns NULL
- r535_gsp_msgq_recv(): CAN return NULL (when RPC length invalid)
- r535_gsp_msg_recv(): CAN return NULL (queue drained/no matching
message)
- r535_gsp_rpc_get(): Never returns NULL
- r535_gsp_rpc_handle_reply(): CAN return NULL (NOWAIT/NOSEQ policies)
- r535_gsp_rpc_send(): CAN return NULL (via handle_reply)
- r535_gsp_rpc_push(): CAN return NULL (via handle_reply)
I've been careful to only clean up IS_ERR_OR_NULL() for functions
that actually never return NULL, while preserving the checks for functions
that can return NULL (like r5_gsp_msg_recv() and r535_gsp_msg_recv()).
Could you please review if this approach is correct?
Thanks,
Hongling
在 2026年05月29日 17:40, 曾红玲 写道:
Hi Danilo Krummrich:
like this?
1. r535_gsp_msgq_peek(): ERR_PTR on error, valid pointer on success,
never NULL
2. r535_gsp_msgq_recv_one_elem(): ERR_PTR on error, valid pointer on
success, never NULL
3. r535_gsp_msgq_recv(): ERR_PTR on error, valid pointer on success,
never NULL
4. r535_gsp_msg_recv(): ERR_PTR on error, valid pointer on success,
NULL for queue drained or no payload
5. r535_gsp_rpc_get(): ERR_PTR on error, valid pointer on success,
never NULL
6. r535_gsp_rpc_handle_reply(): Depends on policy, can return NULL
TKS!
*主 题：*Re: [PATCH 0/5] Revert cleanups for IS_ERR_OR_NULL() usage
*日 期：*2026年05月29日04:48
*发件人：*Danilo Krummrich
*收件人：*Timur Tabi,Danilo Krummrich
*抄送
人：*[email protected],[email protected],[email protected],[email protected],[email protected],dri-
[email protected],Zhi
Wang,[email protected],[email protected],[email protected],[email protected],[email protected],[email protected]
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
---
