Re: [PATCH v2 1/2] staging: vchiq_core: Bubble up wait_event_interruptible() return value

[Dan Carpenter <dan.carpenter@linaro.org>]

On Wed, Jul 03, 2024 at 06:18:19PM +0200, Stefan Wahren wrote:
> > + *
> > + * Returns: 0 on success, a negative error code on failure

Probably not even worth adding this comment.  It's assumed.

> >    */
> >   static inline int
> >   remote_event_wait(wait_queue_head_t *wq, struct remote_event *event)
> >   {
> > +	int ret = 0;
> > +
> >   	if (!event->fired) {
> >   		event->armed = 1;
> >   		dsb(sy);
> > -		if (wait_event_interruptible(*wq, event->fired)) {
> > +		ret = wait_event_interruptible(*wq, event->fired);
> > +		if (ret) {
> >   			event->armed = 0;
> > -			return 0;
> > +			return ret;
> >   		}
> >   		event->armed = 0;
> >   		/* Ensure that the peer sees that we are not waiting (armed == 0). */
> > @@ -518,7 +523,7 @@ remote_event_wait(wait_queue_head_t *wq, struct remote_event *event)
> >   	}
> > 
> >   	event->fired = 0;
> > -	return 1;
> > +	return ret;
> in general this patch looks good to me. But maybe we better return 0
> directly and reduce the scope of ret.

Heh.  If I could have found some more things to complain about then I
was going to ask that this be changed to "return 0;" as well.  But I
always feel like "ret" should be function scope.  Otherwise you can get
multiple ret declarations in a function and it leads to not setting the
correct error code.

regards,
dan carpenter