Unlike the kernel macros, our __get_user() and __put_user() do not
return a failure code.  Kernel code typically has a style of
  err |= __get_user(...); err |= __get_user(...);
and then checking err at the end.  In sparc64_get_context() our
version of the code dropped the accumulating into err but left the
"if (err) goto do_sigsegv" checks, which will never be taken. Delete
unnecessary if()s.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/sparc/signal.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/linux-user/sparc/signal.c b/linux-user/sparc/signal.c
index e661a769cb1..43dcd137f51 100644
--- a/linux-user/sparc/signal.c
+++ b/linux-user/sparc/signal.c
@@ -555,8 +555,6 @@ void sparc64_get_context(CPUSPARCState *env)
         for (i = 0; i < TARGET_NSIG_WORDS; i++, dst++, src++) {
             __put_user(*src, dst);
         }
-        if (err)
-            goto do_sigsegv;
     }
 
     /* XXX: tstate must be saved properly */
@@ -598,8 +596,6 @@ void sparc64_get_context(CPUSPARCState *env)
      * hidden behind an "if (fenab)" where fenab is always 0).
      */
 
-    if (err)
-        goto do_sigsegv;
     unlock_user_struct(ucp, ucp_addr, 1);
     return;
 do_sigsegv:
-- 
2.20.1

On 11/6/20 7:27 AM, Peter Maydell wrote:
> Unlike the kernel macros, our __get_user() and __put_user() do not
> return a failure code.  Kernel code typically has a style of
>   err |= __get_user(...); err |= __get_user(...);
> and then checking err at the end.  In sparc64_get_context() our
> version of the code dropped the accumulating into err but left the
> "if (err) goto do_sigsegv" checks, which will never be taken. Delete
> unnecessary if()s.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  linux-user/sparc/signal.c | 4 ----
>  1 file changed, 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Le 06/11/2020 à 16:27, Peter Maydell a écrit :
> Unlike the kernel macros, our __get_user() and __put_user() do not
> return a failure code.  Kernel code typically has a style of
>   err |= __get_user(...); err |= __get_user(...);
> and then checking err at the end.  In sparc64_get_context() our
> version of the code dropped the accumulating into err but left the
> "if (err) goto do_sigsegv" checks, which will never be taken. Delete
> unnecessary if()s.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  linux-user/sparc/signal.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/linux-user/sparc/signal.c b/linux-user/sparc/signal.c
> index e661a769cb1..43dcd137f51 100644
> --- a/linux-user/sparc/signal.c
> +++ b/linux-user/sparc/signal.c
> @@ -555,8 +555,6 @@ void sparc64_get_context(CPUSPARCState *env)
>          for (i = 0; i < TARGET_NSIG_WORDS; i++, dst++, src++) {
>              __put_user(*src, dst);
>          }
> -        if (err)
> -            goto do_sigsegv;
>      }
>  
>      /* XXX: tstate must be saved properly */
> @@ -598,8 +596,6 @@ void sparc64_get_context(CPUSPARCState *env)
>       * hidden behind an "if (fenab)" where fenab is always 0).
>       */
>  
> -    if (err)
> -        goto do_sigsegv;
>      unlock_user_struct(ucp, ucp_addr, 1);
>      return;
>  do_sigsegv:
> 

Applied to my linux-user-for-6.0 branch.

Thanks,
Laurent