Re: [PATCH v2 1/7] powerpc: properly negate error in syscall_set_return_value()

["Dmitry V. Levin" <ldv@strace.io>]

On Mon, Jan 27, 2025 at 01:04:27PM +0100, Christophe Leroy wrote:
> 
> 
> Le 27/01/2025 à 12:44, Dmitry V. Levin a écrit :
> > On Mon, Jan 27, 2025 at 12:36:53PM +0100, Christophe Leroy wrote:
> >> Le 27/01/2025 à 12:20, Dmitry V. Levin a écrit :
> >>> On Thu, Jan 23, 2025 at 11:07:21PM +0100, Christophe Leroy wrote:
> >>> [...]
> >>>> To add a bit more to the confusion,
> >>>
> >>> Looks like there is no end to it:
> >>>
> >>> static inline long regs_return_value(struct pt_regs *regs)
> >>> {
> >>>           if (trap_is_scv(regs))
> >>>                   return regs->gpr[3];
> >>>
> >>>           if (is_syscall_success(regs))
> >>>                   return regs->gpr[3];
> >>>           else
> >>>                   return -regs->gpr[3];
> >>> }
> >>>
> >>> static inline void regs_set_return_value(struct pt_regs *regs, unsigned long rc)
> >>> {
> >>>           regs->gpr[3] = rc;
> >>> }
> >>>
> >>> This doesn't look consistent, does it?
> >>>
> >>>
> >>
> >> That regs_set_return_value() looks pretty similar to
> >> syscall_get_return_value().
> > 
> > Yes, but here similarities end, and differences begin.
> > 
> >> regs_set_return_value() documentation in asm-generic/syscall.h
> >> explicitely says: This value is meaningless if syscall_get_error()
> >> returned nonzero
> >>
> >> Is it the same with regs_set_return_value(), only meaningfull where
> >> there is no error ?
> > 
> > Did you mean syscall_set_return_value?  No, it explicitly has two
> > arguments, "int error" and "long val", so it can be used to either
> > clear or set the error condition as specified by the caller.
> 
> Sorry, I mean syscall_get_return_value() here.
> 
> static inline long syscall_get_return_value(struct task_struct *task,
> 					    struct pt_regs *regs)
> {
> 	return regs->gpr[3];
> }
> 
> Versus
> 
> static inline void regs_set_return_value(struct pt_regs *regs, unsigned 
> long rc)
> {
> 	regs->gpr[3] = rc;
> }

The asm/syscall.h API provides two functions to obtain the return value:
syscall_get_error() and syscall_get_return_value().  The first one is used
to obtain the error code when the error condition is set.  When the error
condition is not set, it returns 0.  The second function is used to obtain
the return value when the error condition is not set.  When the error
condition is set, its return value is undefined.


-- 
ldv