On 1/2/24 05:57, Richard Henderson wrote:
> All "goto fail" may be transformed to "return -1".
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> linux-user/mmap.c | 62 ++++++++++++++++++++++++++---------------------
> 1 file changed, 35 insertions(+), 27 deletions(-)
>
> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> index d11f758d07..b4c3cc65aa 100644
> --- a/linux-user/mmap.c
> +++ b/linux-user/mmap.c
> @@ -490,9 +490,9 @@ abi_ulong mmap_find_vma(abi_ulong start, abi_ulong size, abi_ulong align)
> }
> }
>
> -/* NOTE: all the constants are the HOST ones */
> -abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
> - int flags, int fd, off_t offset)
> +static abi_long target_mmap__locked(abi_ulong start, abi_ulong len,
> + int target_prot, int flags,
> + int fd, off_t offset)
> {
> int host_page_size = qemu_real_host_page_size();
> abi_ulong ret, last, real_start, real_last, retaddr, host_len;
> @@ -500,30 +500,27 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
> int page_flags;
> off_t host_offset;
>
> - mmap_lock();
> - trace_target_mmap(start, len, target_prot, flags, fd, offset);
> -
> if (!len) {
> errno = EINVAL;
> - goto fail;
> + return -1;
> }
>
> page_flags = validate_prot_to_pageflags(target_prot);
> if (!page_flags) {
> errno = EINVAL;
> - goto fail;
> + return -1;
> }
>
> /* Also check for overflows... */
> len = TARGET_PAGE_ALIGN(len);
> if (!len) {
> errno = ENOMEM;
> - goto fail;
> + return -1;
> }
>
> if (offset & ~TARGET_PAGE_MASK) {
> errno = EINVAL;
> - goto fail;
> + return -1;
> }
>
> /*
> @@ -553,7 +550,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
> start = mmap_find_vma(real_start, host_len, TARGET_PAGE_SIZE);
> if (start == (abi_ulong)-1) {
> errno = ENOMEM;
> - goto fail;
> + return -1;
> }
> }
>
> @@ -574,7 +571,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
> struct stat sb;
>
> if (fstat(fd, &sb) == -1) {
> - goto fail;
> + return -1;
> }
>
> /* Are we trying to create a map beyond EOF?. */
> @@ -601,7 +598,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
> p = mmap(g2h_untagged(start), host_len, host_prot,
> flags | MAP_FIXED | MAP_ANONYMOUS, -1, 0);
> if (p == MAP_FAILED) {
> - goto fail;
> + return -1;
> }
> /* update start so that it points to the file position at 'offset' */
> host_start = (uintptr_t)p;
> @@ -610,7 +607,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
> flags | MAP_FIXED, fd, host_offset);
> if (p == MAP_FAILED) {
> munmap(g2h_untagged(start), host_len);
> - goto fail;
> + return -1;
> }
> host_start += offset - host_offset;
> }
> @@ -621,7 +618,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
> } else {
> if (start & ~TARGET_PAGE_MASK) {
> errno = EINVAL;
> - goto fail;
> + return -1;
> }
> last = start + len - 1;
> real_last = ROUND_UP(last, host_page_size) - 1;
> @@ -633,14 +630,14 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
> */
> if (last < start || !guest_range_valid_untagged(start, len)) {
> errno = ENOMEM;
> - goto fail;
> + return -1;
> }
>
> if (flags & MAP_FIXED_NOREPLACE) {
> /* Validate that the chosen range is empty. */
> if (!page_check_range_empty(start, last)) {
> errno = EEXIST;
> - goto fail;
> + return -1;
> }
>
> /*
> @@ -671,17 +668,17 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
> if ((flags & MAP_TYPE) == MAP_SHARED
> && (target_prot & PROT_WRITE)) {
> errno = EINVAL;
> - goto fail;
> + return -1;
> }
> retaddr = target_mmap(start, len, target_prot | PROT_WRITE,
> (flags & (MAP_FIXED | MAP_FIXED_NOREPLACE))
> | MAP_PRIVATE | MAP_ANONYMOUS,
> -1, 0);
> if (retaddr == -1) {
> - goto fail;
> + return -1;
> }
> if (pread(fd, g2h_untagged(start), len, offset) == -1) {
> - goto fail;
> + return -1;
> }
> if (!(target_prot & PROT_WRITE)) {
> ret = target_mprotect(start, len, target_prot);
> @@ -696,14 +693,14 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
> /* one single host page */
> if (!mmap_frag(real_start, start, last,
> target_prot, flags, fd, offset)) {
> - goto fail;
> + return -1;
> }
> goto the_end1;
> }
> if (!mmap_frag(real_start, start,
> real_start + host_page_size - 1,
> target_prot, flags, fd, offset)) {
> - goto fail;
> + return -1;
> }
> real_start += host_page_size;
> }
> @@ -713,7 +710,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
> if (!mmap_frag(real_page, real_page, last,
> target_prot, flags, fd,
> offset + real_page - start)) {
> - goto fail;
> + return -1;
> }
> real_last -= host_page_size;
> }
> @@ -739,7 +736,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
> munmap(p, len1);
> errno = EEXIST;
> }
> - goto fail;
> + return -1;
> }
> passthrough_start = real_start;
> passthrough_last = real_last;
> @@ -773,11 +770,22 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
> qemu_log_unlock(f);
> }
> }
> - mmap_unlock();
> return start;
> -fail:
> +}
> +
> +/* NOTE: all the constants are the HOST ones */
> +abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
> + int flags, int fd, off_t offset)
> +{
> + abi_long ret;
> +
> + trace_target_mmap(start, len, target_prot, flags, fd, offset);
> + mmap_lock();
> +
> + ret = target_mmap__locked(start, len, target_prot, flags, fd, offset);
> +
> mmap_unlock();
> - return -1;
> + return ret;
> }
>
> static int mmap_reserve_or_unmap(abi_ulong start, abi_ulong len)
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>