Re: [PATCH v5] alloc: fix dangling pointer in alloc_state cleanup

[Junio C Hamano <gitster@pobox.com>]

"ノウラ | Flare via GitGitGadget" <gitgitgadget@gmail.com> writes:

> index 377e80f5dd..3a5d0b2bd8 100644
> --- a/alloc.c
> +++ b/alloc.c
> @@ -36,19 +36,22 @@ struct alloc_state {
>  	int slab_nr, slab_alloc;
>  };
>  
> -struct alloc_state *allocate_alloc_state(void)
> +struct alloc_state *alloc_state_alloc(void)
>  {
>  	return xcalloc(1, sizeof(struct alloc_state));
>  }
>  
> -void clear_alloc_state(struct alloc_state *s)
> +void alloc_state_free_and_null(struct alloc_state **s_)
>  {
> +	struct alloc_state *s = *s_;
> +
>  	while (s->slab_nr > 0) {
>  		s->slab_nr--;
>  		free(s->slabs[s->slab_nr]);
>  	}
>  
>  	FREE_AND_NULL(s->slabs);
> +	FREE_AND_NULL(*s_);
>  }
>  
>  static inline void *alloc_node(struct alloc_state *s, size_t node_size)

Looking good.

> diff --git a/alloc.h b/alloc.h
> index 3f4a0ad310..87a47a9709 100644
> --- a/alloc.h
> +++ b/alloc.h
> @@ -14,7 +14,7 @@ void *alloc_commit_node(struct repository *r);
>  void *alloc_tag_node(struct repository *r);
>  void *alloc_object_node(struct repository *r);
>  
> -struct alloc_state *allocate_alloc_state(void);
> -void clear_alloc_state(struct alloc_state *s);
> +struct alloc_state *alloc_state_alloc(void);
> +void alloc_state_free_and_null(struct alloc_state **s_);
>  
>  #endif
> diff --git a/object.c b/object.c
> index c1553ee433..986114a6db 100644
> --- a/object.c
> +++ b/object.c
> @@ -573,16 +572,11 @@ void parsed_object_pool_clear(struct parsed_object_pool *o)
>  	o->buffer_slab = NULL;
>  
>  	parsed_object_pool_reset_commit_grafts(o);
> -	clear_alloc_state(o->blob_state);
> -	clear_alloc_state(o->tree_state);
> -	clear_alloc_state(o->commit_state);
> -	clear_alloc_state(o->tag_state);
> -	clear_alloc_state(o->object_state);
> +	alloc_state_free_and_null(&o->blob_state);
> +	alloc_state_free_and_null(&o->tree_state);
> +	alloc_state_free_and_null(&o->commit_state);
> +	alloc_state_free_and_null(&o->tag_state);
> +	alloc_state_free_and_null(&o->object_state);
>  	stat_validity_clear(o->shallow_stat);
> -	FREE_AND_NULL(o->blob_state);
> -	FREE_AND_NULL(o->tree_state);
> -	FREE_AND_NULL(o->commit_state);
> -	FREE_AND_NULL(o->tag_state);
> -	FREE_AND_NULL(o->object_state);
>  	FREE_AND_NULL(o->shallow_stat);
>  }

Very nice.

Thanks.  Will queue.