Re: [Xen-devel] [PATCH 3/4] common/page_alloc: Remove node id ASSERT()s

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.09.13 at 14:10, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> Coverity ID 1055622, 1055630
> 
> node is an unsigned integer, so neccesserily >= 0.  There does not appear to
> be any purpose to these assertions in the first place.

I think assertions should be treated more forgivingly than "normal"
code: The check is very valid is the type of "node" got changed
back to a signed int, as "node" is being used as array index. It is
quite likely that on such type changes people would not remember
to add back respective assertions.

That said - I'm not really opposed to this change, I just think that
assertions like the one here have documentation aspects going
beyond mere correctness checking intentions.

Jan

> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> CC: Keir Fraser <keir@xxxxxxx>
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Tim Deegan <tim@xxxxxxx>
> ---
>  xen/common/page_alloc.c |    2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
> index 41251b2..8cab81b 100644
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -585,7 +585,6 @@ static struct page_info *alloc_heap_pages(
>      }
>      first_node = node;
>  
> -    ASSERT(node >= 0);
>      ASSERT(zone_lo <= zone_hi);
>      ASSERT(zone_hi < NR_ZONES);
>  
> @@ -812,7 +811,6 @@ static void free_heap_pages(
>      unsigned int zone = page_to_zone(pg);
>  
>      ASSERT(order <= MAX_ORDER);
> -    ASSERT(node >= 0);
>  
>      spin_lock(&heap_lock);
>  
> -- 
> 1.7.10.4




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel