| Message ID | 20210422230227.314751-8-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | linux-user: sigaction fixes/cleanups | expand |
On 4/23/21 1:02 AM, Richard Henderson wrote: > Initialize variables instead of elses. > Use an else instead of a goto. > Add braces. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > linux-user/syscall.c | 32 +++++++++++++------------------- > 1 file changed, 13 insertions(+), 19 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Richard Henderson <richard.henderson@linaro.org> writes: > Initialize variables instead of elses. > Use an else instead of a goto. > Add braces. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > linux-user/syscall.c | 32 +++++++++++++------------------- > 1 file changed, 13 insertions(+), 19 deletions(-) > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index 9bcd485423..c7c3257f40 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -9060,32 +9060,26 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1, > target_ulong sigsetsize = arg4; > target_ulong restorer = 0; > #endif > - struct target_sigaction *act; > - struct target_sigaction *oact; > + struct target_sigaction *act = NULL; > + struct target_sigaction *oact = NULL; > > if (sigsetsize != sizeof(target_sigset_t)) { > return -TARGET_EINVAL; > } > - if (arg2) { > - if (!lock_user_struct(VERIFY_READ, act, arg2, 1)) { > - return -TARGET_EFAULT; > - } > - } else { > - act = NULL; > + if (arg2 && !lock_user_struct(VERIFY_READ, act, arg2, 1)) { > + return -TARGET_EFAULT; > } > - if (arg3) { > - if (!lock_user_struct(VERIFY_WRITE, oact, arg3, 0)) { > - ret = -TARGET_EFAULT; > - goto rt_sigaction_fail; > + if (arg3 && !lock_user_struct(VERIFY_WRITE, oact, arg3, 0)) { > + ret = -TARGET_EFAULT; > + } else { > + ret = get_errno(do_sigaction(arg1, act, oact, restorer)); > + if (oact) { > + unlock_user_struct(oact, arg3, 1); > } This does make me idly wonder if there is a way to handle unlocking in a similar way to our autofree and LOCK_GUARD stuff. But that's not getting in the way of approving of this improvement. Reviewed-by: Alex Bennée <alex.bennee@linaro.org> -- Alex Bennée
diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 9bcd485423..c7c3257f40 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -9060,32 +9060,26 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1, target_ulong sigsetsize = arg4; target_ulong restorer = 0; #endif - struct target_sigaction *act; - struct target_sigaction *oact; + struct target_sigaction *act = NULL; + struct target_sigaction *oact = NULL; if (sigsetsize != sizeof(target_sigset_t)) { return -TARGET_EINVAL; } - if (arg2) { - if (!lock_user_struct(VERIFY_READ, act, arg2, 1)) { - return -TARGET_EFAULT; - } - } else { - act = NULL; + if (arg2 && !lock_user_struct(VERIFY_READ, act, arg2, 1)) { + return -TARGET_EFAULT; } - if (arg3) { - if (!lock_user_struct(VERIFY_WRITE, oact, arg3, 0)) { - ret = -TARGET_EFAULT; - goto rt_sigaction_fail; + if (arg3 && !lock_user_struct(VERIFY_WRITE, oact, arg3, 0)) { + ret = -TARGET_EFAULT; + } else { + ret = get_errno(do_sigaction(arg1, act, oact, restorer)); + if (oact) { + unlock_user_struct(oact, arg3, 1); } - } else - oact = NULL; - ret = get_errno(do_sigaction(arg1, act, oact, restorer)); - rt_sigaction_fail: - if (act) + } + if (act) { unlock_user_struct(act, arg2, 0); - if (oact) - unlock_user_struct(oact, arg3, 1); + } } return ret; #ifdef TARGET_NR_sgetmask /* not on alpha */
Initialize variables instead of elses. Use an else instead of a goto. Add braces. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- linux-user/syscall.c | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) -- 2.25.1