Re: [PATCH 5/6] KVM: selftests: Add wrappers for mmap() and munmap() to assert success

Ackerley Tng
2025-09-29T11:08:44Z

Sean Christopherson <seanjc@google.com> writes:

> Add and use wrappers for mmap() and munmap() that assert success to reduce
> a significant amount of boilerplate code, to ensure all tests assert on
> failure, and to provide consistent error messages on failure.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> .../testing/selftests/kvm/guest_memfd_test.c | 21 +++------
> .../testing/selftests/kvm/include/kvm_util.h | 25 +++++++++++
> tools/testing/selftests/kvm/lib/kvm_util.c | 44 +++++++------------
> tools/testing/selftests/kvm/mmu_stress_test.c | 5 +--
> .../selftests/kvm/s390/ucontrol_test.c | 16 +++----
> .../selftests/kvm/set_memory_region_test.c | 17 ++++---
> 6 files changed, 64 insertions(+), 64 deletions(-)
>
>
> [...snip...]
>
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index 23a506d7eca3..1c68ff0fb3fb 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -278,6 +278,31 @@ static inline bool kvm_has_cap(long cap)
> #define __KVM_SYSCALL_ERROR(_name, _ret) \
> "%s failed, rc: %i errno: %i (%s)", (_name), (_ret), errno, strerror(errno)
>
> +static inline void *__kvm_mmap(size_t size, int prot, int flags, int fd,
> + off_t offset)

Do you have a policy/rationale for putting this in kvm_util.h as opposed
to test_util.h? I like the idea of this wrapper but I thought this is
less of a kvm thing and more of a test utility, and hence it belongs in
test_util.c and test_util.h.

Also, the name kind of associates mmap with KVM too closely IMO, but
test_mmap() is not a great name either.

No strong opinions here.

Reviewed-by: Ackerley Tng <ackerleytng@google.com>

> +{
> + void *mem;
> +
> + mem = mmap(NULL, size, prot, flags, fd, offset);
> + TEST_ASSERT(mem != MAP_FAILED, __KVM_SYSCALL_ERROR("mmap()",
> + (int)(unsigned long)MAP_FAILED));
> +
> + return mem;
> +}
> +
> +static inline void *kvm_mmap(size_t size, int prot, int flags, int fd)
> +{
> + return __kvm_mmap(size, prot, flags, fd, 0);
> +}
> +
>
> [...snip...]
>