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

[Ackerley Tng]

Sean Christopherson <seanjc@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
>  .../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@xxxxxxxxxx>

> +{
> +	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...]
>