Re: [PATCH 08/72] core: dropped actually unused parameter of _cairo_boxes_to_array()

On Tue, Dec 29, 2015 at 10:16:46AM +0100, Enrico Weigelt, metux IT consult wrote:
> When parameter force_allocation is false *and* the box set has
> exactly one chunk, this chunk is returned directly - w/o copying it.
> 
> That mode is never used, and it's highly problematic as it's unclear
> whether we have to free the returnd object or it's still owned
> by somebody else.
> 
> Just dropping the useless parameter / corner case to make the function
> simpler and more robust.

Looks okay, only the two callers from cairo-clip-boxes.c and both are
specifying it to be TRUE.  If nothing calls it with FALSE I agree it
seems unnecessary.  I'm curious if this ever did something else?

Anyway:

Reviewed-by: Bryce Harrington <[email protected]>
 
> Signed-off-by: Enrico Weigelt, metux IT consult <[email protected]>
> ---
>  src/cairo-boxes-private.h | 3 +--
>  src/cairo-boxes.c         | 5 +----
>  src/cairo-clip-boxes.c    | 4 ++--
>  3 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/src/cairo-boxes-private.h b/src/cairo-boxes-private.h
> index d1f9dfc..fd622ae 100644
> --- a/src/cairo-boxes-private.h
> +++ b/src/cairo-boxes-private.h
> @@ -92,8 +92,7 @@ _cairo_boxes_extents (const cairo_boxes_t *boxes,
>  
>  cairo_private cairo_box_t *
>  _cairo_boxes_to_array (const cairo_boxes_t *boxes,
> -		       int *num_boxes,
> -		       cairo_bool_t force_allocation);
> +		       int *num_boxes);
>  
>  cairo_private cairo_status_t
>  _cairo_boxes_intersect (const cairo_boxes_t *a,
> diff --git a/src/cairo-boxes.c b/src/cairo-boxes.c
> index db002ef..6ddf81a 100644
> --- a/src/cairo-boxes.c
> +++ b/src/cairo-boxes.c
> @@ -345,16 +345,13 @@ _cairo_boxes_clear (cairo_boxes_t *boxes)
>   * */
>  cairo_box_t *
>  _cairo_boxes_to_array (const cairo_boxes_t *boxes,
> -		       int *num_boxes,
> -		       cairo_bool_t force_allocation)
> +		       int *num_boxes)
>  {
>      const struct _cairo_boxes_chunk *chunk;
>      cairo_box_t *box;
>      int i, j;
>  
>      *num_boxes = boxes->num_boxes;
> -    if (boxes->chunks.next == NULL && ! force_allocation)
> -	    return boxes->chunks.base;
>  
>      box = _cairo_malloc_ab (boxes->num_boxes, sizeof (cairo_box_t));
>      if (box == NULL) {
> diff --git a/src/cairo-clip-boxes.c b/src/cairo-clip-boxes.c
> index abf634e..b1ff705 100644
> --- a/src/cairo-clip-boxes.c
> +++ b/src/cairo-clip-boxes.c
> @@ -306,7 +306,7 @@ _cairo_clip_intersect_boxes (cairo_clip_t *clip,
>  	clip->boxes[0] = boxes->chunks.base[0];
>  	clip->num_boxes = 1;
>      } else {
> -	clip->boxes = _cairo_boxes_to_array (boxes, &clip->num_boxes, TRUE);
> +	clip->boxes = _cairo_boxes_to_array (boxes, &clip->num_boxes);
>      }
>      _cairo_boxes_extents (boxes, &limits);
>  
> @@ -580,7 +580,7 @@ _cairo_clip_from_boxes (const cairo_boxes_t *boxes)
>  	clip->boxes[0] = boxes->chunks.base[0];
>  	clip->num_boxes = 1;
>      } else {
> -	clip->boxes = _cairo_boxes_to_array (boxes, &clip->num_boxes, TRUE);
> +	clip->boxes = _cairo_boxes_to_array (boxes, &clip->num_boxes);
>  	if (clip->boxes == NULL)
>  	    return _cairo_clip_set_all_clipped (clip);
>      }
> -- 
> 2.6.4.442.g545299f
> 
> -- 
> cairo mailing list
> [email protected]
> http://lists.cairographics.org/mailman/listinfo/cairo
-- 
cairo mailing list
[email protected]
http://lists.cairographics.org/mailman/listinfo/cairo