Re: git: 19f073c612af - main - new-bus: Add resource_validate_map_request function

On 11/23/23 13:07, John Baldwin wrote:
> The branch main has been updated by jhb:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=19f073c612afa0111d216e5ccab9525bfc97ec32
> 
> commit 19f073c612afa0111d216e5ccab9525bfc97ec32
> Author:     John Baldwin <jhb@FreeBSD.org>
> AuthorDate: 2023-11-23 17:06:24 +0000
> Commit:     John Baldwin <jhb@FreeBSD.org>
> CommitDate: 2023-11-23 17:06:24 +0000
> 
>      new-bus: Add resource_validate_map_request function
>      
>      This helper function for BUS_MAP_RESOURCE performs common argument
>      validation.
>      
>      Reviewed by:    imp
>      Differential Revision:  https://reviews.freebsd.org/D42723
> ---
>   sys/kern/subr_bus.c | 31 +++++++++++++++++++++++++++++++
>   sys/sys/bus.h       |  6 +++++-
>   2 files changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/sys/kern/subr_bus.c b/sys/kern/subr_bus.c
> index 648394abd026..80fe182eab56 100644
> --- a/sys/kern/subr_bus.c
> +++ b/sys/kern/subr_bus.c
> @@ -2715,6 +2715,37 @@ resource_init_map_request_impl(struct resource_map_request *args, size_t sz)
>   	args->memattr = VM_MEMATTR_DEVICE;
>   }
>   
> +int
> +resource_validate_map_request(struct resource *r,
> +    struct resource_map_request *in, struct resource_map_request *out,
> +    rman_res_t *startp, rman_res_t *lengthp)

Can the function be given a top-level comment stating its purpose? This 
file does a really good job providing this for the majority of its 
public functions.

Sorry to ask post-commit, but I did not see the review.

Mitchell

> +{
> +	rman_res_t end, length, start;
> +
> +	/*
> +	 * This assumes that any callers of this function are compiled
> +	 * into the kernel and use the same version of the structure
> +	 * as this file.
> +	 */
> +	MPASS(out->size == sizeof(struct resource_map_request));
> +
> +	if (in != NULL)
> +		bcopy(in, out, imin(in->size, out->size));
> +	start = rman_get_start(r) + out->offset;
> +	if (out->length == 0)
> +		length = rman_get_size(r);
> +	else
> +		length = out->length;
> +	end = start + length - 1;
> +	if (start > rman_get_end(r) || start < rman_get_start(r))
> +		return (EINVAL);
> +	if (end > rman_get_end(r) || end < start)
> +		return (EINVAL);
> +	*lengthp = length;
> +	*startp = start;
> +	return (0);
> +}
> +
>   /**
>    * @brief Initialise a resource list.
>    *
> diff --git a/sys/sys/bus.h b/sys/sys/bus.h
> index fc07cf70f78a..88ae4000004b 100644
> --- a/sys/sys/bus.h
> +++ b/sys/sys/bus.h
> @@ -317,6 +317,8 @@ struct driver {
>   	KOBJ_CLASS_FIELDS;
>   };
>   
> +struct resource;
> +
>   /**
>    * @brief A resource mapping.
>    */
> @@ -341,12 +343,14 @@ void	resource_init_map_request_impl(struct resource_map_request *_args,
>   	    size_t _sz);
>   #define	resource_init_map_request(rmr) 					\
>   	resource_init_map_request_impl((rmr), sizeof(*(rmr)))
> +int	resource_validate_map_request(struct resource *r,
> +	    struct resource_map_request *in, struct resource_map_request *out,
> +	    rman_res_t *startp, rman_res_t *lengthp);
>   
>   /*
>    * Definitions for drivers which need to keep simple lists of resources
>    * for their child devices.
>    */
> -struct	resource;
>   
>   /**
>    * @brief An entry for a single resource in a resource list.