Re: [Xen-devel] [PATCH 07/25] xen (ARM, x86): add errno-returning functions for copy

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Nov 30, 2018 at 05:32:46PM -0800, Christopher Clark wrote:
> Applied to both x86 and ARM headers.
> 
> Signed-off-by: Christopher Clark <christopher.clark6@xxxxxxxxxxxxxx>
> ---
>  xen/include/asm-arm/guest_access.h | 25 +++++++++++++++++++++++++
>  xen/include/asm-x86/guest_access.h | 29 +++++++++++++++++++++++++++++
>  xen/include/xen/guest_access.h     |  3 +++
>  3 files changed, 57 insertions(+)
> 
> diff --git a/xen/include/asm-arm/guest_access.h 
> b/xen/include/asm-arm/guest_access.h
> index 224d2a0..7b6f89c 100644
> --- a/xen/include/asm-arm/guest_access.h
> +++ b/xen/include/asm-arm/guest_access.h
> @@ -24,6 +24,11 @@ int access_guest_memory_by_ipa(struct domain *d, paddr_t 
> ipa, void *buf,
>  #define __raw_copy_from_guest raw_copy_from_guest
>  #define __raw_clear_guest raw_clear_guest
>  
> +#define raw_copy_from_guest_errno(dst, src, len)             \
> +    (raw_copy_from_guest((dst), (src), (len)) ? -EFAULT : 0)
> +#define raw_copy_to_guest_errno(dst, src, len)               \
> +    (raw_copy_to_guest((dst), (src), (len)) ? -EFAULT : 0)

Since the only error that you return is EFAULT, I don't really see the
point in adding all those helpers. You achieve exactly the same by
returning a boolean and doing the translation to EFAULT in the caller
if required.

It might have been nice to have the copy to/from set of functions
return an error value, but adding a new set of helpers that have the
same functionality but just differ in the return value look
redundant.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel