Re: 回复: Re: [PATCH 0/5] Revert cleanups for IS_ERR_OR_NULL() usage

Re: 回复: Re: [PATCH 0/5] Revert cleanups for IS_ERR_OR_NULL() usage

[Hongling Zeng]

   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
> *抄送 
> 人：*lyude@redhat.com,rongqianfeng@vivo.com,airlied@gmail.com,kees@kernel.org,simona@ffwll.ch,dri- 
> devel@lists.freedesktop.org,Zhi 
> Wang,nouveau@lists.freedesktop.org,airlied@redhat.com,maarten.lankhorst@linux.intel.com,tzimmermann@suse.de,linux-kernel@vger.kernel.org,mripard@kernel.org
>
> 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
>
>
> ---
>

Re: 回复: Re: [PATCH 0/5] Revert cleanups for IS_ERR_OR_NULL() usage

[Hongling Zeng]

   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
> *抄送 
> 人：*lyude@redhat.com,rongqianfeng@vivo.com,airlied@gmail.com,kees@kernel.org,simona@ffwll.ch,dri- 
> devel@lists.freedesktop.org,Zhi 
> Wang,nouveau@lists.freedesktop.org,airlied@redhat.com,maarten.lankhorst@linux.intel.com,tzimmermann@suse.de,linux-kernel@vger.kernel.org,mripard@kernel.org
>
> 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
>
>
> ---
>