Re: [patch] tree-ssa-dom.c: Use VEC(vrp_element_p,heap) instead ofVARRAY.

[Jeffrey A Law <law at redhat dot com>]

On Wed, 2005-04-27 at 09:39 -0400, Kazu Hirata wrote:
> Hi,
> 
> Attached is a patch to use VEC(vrp_element_p,heap) instead of VARRAY.
> 
> There are two unintuitive changes.
> 
> -	  vrp_records = vrp_hash_elt_p->records;
> -	  if (vrp_records == NULL)
> -	    return NULL;
> +	  vrp_records = &vrp_hash_elt_p->records;
> 
> In this case, we have the following code immediately following the
> code above.
> 
> 	  limit = VEC_length (vrp_element_p, *vrp_records);
> 	  if (limit == 0
> 
> So if vrp_records is NULL, then VEC_length returns 0, so the the limit
> check catches the cases of vrp_records being NULL
This doesn't look right to me.  If you read the comments earlier in the
code it states that two tests must be performed.  First you have to
verify the existance of the varray (or VEC) then you have to look at
the number of active entries.

Your change will cause a segfault in the first case as *vrp_records
would be dereferencing a null pointer.

So my question to you would be is there some reason why it's no longer
possible for vrp_records to be null?

> 
> -      if (*vrp_records_p == NULL)
> -	VARRAY_GENERIC_PTR_INIT (*vrp_records_p, 2, "vrp records");
> -      
> -      VARRAY_PUSH_GENERIC_PTR (*vrp_records_p, element);
> +      VEC_safe_push (vrp_element_p, heap, *vrp_records_p, element);
Similarly here.

> 
> In this case, *vrp_records_p == NULL really means that the vector is
> emoty. 
No, it means the vector does not exist.  There is a difference.


>  VEC_safe_push reserves memory if a pointer holding a vector is
> null or we have not allocated any memory for the vector at all, so we
> don't need to check *vrp_records_p.  It's included in VEC_safe_push.
> 
> Since we are now explicitly managing the memory for these hash
> elements, I created a new function vec_free, which is responsible for
> vrp_hash_elt as well as the vector in it.
You may be trying to explain my objection above -- but I can't really
parse what you're trying to say.  Can you try to reword this to be
clearer?

Jeff