From: Phillip Wood <phillip.wood123@gmail.com>
Date: Fri, 01 Apr 2022 15:33:21 GMT
Subject: Re: [PATCH v4 10/27] stash: always have the owner of "stash_info" free it
Message-ID: <3b08ab38-0d91-a361-544c-c3af68715e79@gmail.com>
In-Reply-To: <patch-v4-10.27-145a0f74b6a-20220331T005325Z-avarab@gmail.com>


Hi Ævar

On 31/03/2022 02:11, Ævar Arnfjörð Bjarmason wrote:
> Change the initialization of the "revision" member of "struct
> stash_info" to be initialized vi a macro, and more importantly that
> that initializing function be tasked to free it, usually via a "goto
> cleanup" pattern.
> 
> Despite the "revision" name (and the topic of the series containing
> this commit) the "stash info" has nothing to do with the "struct
> rev_info". I'm making this change because in the subsequent commit
> when we do want to free the "struct rev_info" via a "goto cleanup"
> pattern we'd otherwise free() uninitialized memory in some cases, as
> we only strbuf_init() the string in get_stash_info().
> 
> So while it's the smallest possible change, let's convert all users of
> this pattern in the file while we're at it.
> 
> A good follow-up to this change would be to change all the "ret = -1;
> goto done;" in this file to instead use a "goto cleanup", and
> initialize "int ret = -1" at the start of the relevant functions. That
> would allow us to drop a lot of needless brace verbosity on two-line
> "if" statements, but let's leave that alone for now.
>[...]   
> @@ -912,8 +912,11 @@ static int show_stash(int argc, const char **argv, const char *prefix)
>   	}
>   	log_tree_diff_flush(&rev);
>   
> +	ret = diff_result_code(&rev.diffopt, 0);;

Extra ';'

Best Wishes

Phillip