Re: [Qemu-devel] [PATCH v3 07/19] linux-user: Relax single exit from "break"

[Laurent Vivier]

Le 12/06/2018 à 02:51, Richard Henderson a écrit :
> Transform outermost "break" to "return ret".  If the immediately
> preceeding statement was an assignment to ret, return the value
> directly.
> 
> Reviewed-by: Laurent Vivier <address@hidden>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  linux-user/syscall.c | 970 +++++++++++++++++--------------------------
>  1 file changed, 390 insertions(+), 580 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index ec3bc1cbe5..efe882612b 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
...
> @@ -10481,7 +10387,7 @@ static abi_long do_syscall1(void *cpu_env, int num, 
> abi_long arg1,
>                  if (arg4) {
>                      unlock_user(target_set, arg4, 0);
>                  }
> -                break;
> +                return ret;
>              }
>  # endif
>  # ifdef TARGET_NR_poll
> @@ -10498,8 +10404,7 @@ static abi_long do_syscall1(void *cpu_env, int num, 
> abi_long arg1,
>                      /* -ve poll() timeout means "infinite" */
>                      pts = NULL;
>                  }
> -                ret = get_errno(safe_ppoll(pfd, nfds, pts, NULL, 0));
> -                break;
> +                return get_errno(safe_ppoll(pfd, nfds, pts, NULL, 0));
>              }
>  # endif
>              default:

You can't change these breaks by returns because they are in an inner
switch() (see the "default:"), that is followed by:

    if (!is_error(ret)) {
        for(i = 0; i < nfds; i++) {
            target_pfd[i].revents = tswap16(pfd[i].revents);
        }
        }
    unlock_user(target_pfd, arg1, sizeof(struct target_pollfd) * nfds);

which are needed to copy the result to the target array.
...
> @@ -12444,7 +12265,6 @@ static abi_long do_syscall1(void *cpu_env, int num, 
> abi_long arg1,
>  
>              ret = get_errno(safe_epoll_pwait(epfd, ep, maxevents, timeout,
>                                               set, SIGSET_T_SIZE));
> -            break;

You can't remove this break because otherwise it falls through the
following case (and you can't put a return because it's in an inner
switch())
>          }
>  #endif
>  #if defined(TARGET_NR_epoll_wait)
> @@ -12468,7 +12288,7 @@ static abi_long do_syscall1(void *cpu_env, int num, 
> abi_long arg1,
>              unlock_user(target_ep, arg2, 0);
>          }
>          g_free(ep);
> -        break;
> +        return ret;
>      }
>  #endif
>  #endif

Thanks,
Laurent