From: Jeff King <peff@peff.net>
Date: Thu, 04 Sep 2025 20:49:32 GMT
Subject: Re: [PATCH v5] alloc: fix dangling pointer in alloc_state cleanup
Message-ID: <20250904204932.GD30633@coredump.intra.peff.net>
In-Reply-To: <pull.2040.v5.git.git.1757007856062.gitgitgadget@gmail.com>

On Thu, Sep 04, 2025 at 05:44:16PM +0000, ノウラ | Flare via GitGitGadget wrote:

> -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_);
>  }

It's probably not worth going back and forth on this too much, but I
thought the happy medium was:

  if (!s)
	return;

That is, it is perfectly reasonable and friendly for it to be a noop to
free-and-null a NULL value (either never initialized, or already freed).
The overkill was worrying about whether somebody passed in a NULL
double-pointer. I.e., doing:

  alloc_state_free_and_null(&foo);

is reasonable and should be idempotent but:

  alloc_state_free_and_null(NULL);

is a silly programming error that we do not need to protect against.

-Peff