Re: [Xen-devel] [PATCH] tools: libxl: Simplify logic in libxl__realloc

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Fri, 2016-02-19 at 15:34 +0000, Ian Jackson wrote:
> Replace the loop exit and separate test for loop overrun with an
> assert in the loop body.
> 
> This simplifies the code.  It also (hopefully) avoids Coverity
> thinking that gc->alloc_maxsize might change, resulting in the loop
> failing to find the right answer but also failing to abort.

It succeeded in this regard, but now coverity thinks that
libxl__gc_is_real(gc) might return false, which I think is reasonable of it
since it cannot possibly tell from this context if gc is NOGC (and hence
has alloc_maxsize==0) or not.

I'm not sure what can be done now, I doubt any kind of check on e.g.
        gc == &gc->owner->no_gc
would be something Coverity could reason about either.

> 
> (gc->alloc_maxsize can't change because gcs are all singlethreaded:
> either they are on the stack of a specific thread, or they belong to
> an ao and are covered by the ctx lock.)
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> ---
>  tools/libxl/libxl_internal.c |    7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
> index fc81130..e7b765b 100644
> --- a/tools/libxl/libxl_internal.c
> +++ b/tools/libxl/libxl_internal.c
> @@ -116,16 +116,13 @@ void *libxl__realloc(libxl__gc *gc, void *ptr,
> size_t new_size)
>      if (ptr == NULL) {
>          libxl__ptr_add(gc, new_ptr);
>      } else if (new_ptr != ptr && libxl__gc_is_real(gc)) {
> -        for (i = 0; i < gc->alloc_maxsize; i++) {
> +        for (i = 0; ; i++) {
> +            assert(i < gc->alloc_maxsize);
>              if (gc->alloc_ptrs[i] == ptr) {
>                  gc->alloc_ptrs[i] = new_ptr;
>                  break;
>              }
>          }
> -        if (i == gc->alloc_maxsize) {
> -            LOG(CRITICAL, "pointer is not tracked by the given gc");
> -            abort();
> -        }
>      }
>  
>      return new_ptr;

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