Re: [PATCH 2/4] dmapool: Validate parameters to dma_pool_create

[roel <12o3l@tiscali.nl>]

Matthew Wilcox wrote:
> Check that 'align' is a power of two, like the API specifies.
> Align 'size' to 'align' correctly -- the current code has an off-by-one.
> The ALIGN macro in kernel.h doesn't.
> 
> Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
> ---
>  mm/dmapool.c |   15 ++++++++-------
>  1 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/mm/dmapool.c b/mm/dmapool.c
> index a359b5e..f5d12a7 100644
> --- a/mm/dmapool.c
> +++ b/mm/dmapool.c
> @@ -107,17 +107,18 @@ struct dma_pool *dma_pool_create(const char *name, struct device *dev,
>  {
>  	struct dma_pool *retval;
>  
> -	if (align == 0)
> +	if (align == 0) {
>  		align = 1;
> -	if (size == 0)
> +	} else if (align & (align - 1)) {
>  		return NULL;
> -	else if (size < align)
> -		size = align;
> -	else if ((size % align) != 0) {
> -		size += align + 1;
> -		size &= ~(align - 1);
>  	}
>  
> +	if (size == 0)
> +		return NULL;

The brackets in the first if/else are not required, and you could combine the two statements:

	if (align == 0)
		align = 1;
	else if (align & (align - 1) || size == 0)
		return NULL;
> +
> +	if ((size % align) != 0)
> +		size = ALIGN(size, align);
> +
>  	if (allocation == 0) {
>  		if (PAGE_SIZE < size)
>  			allocation = size;