Subject: Re: [PATCH v4 2/5] notes.c: cleanup for "designated init" and "char ptr init"

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Thu, 12 Jan 2023 10:51:02 +0100

On Thu, Jan 12 2023, Teng Long wrote:

> From: Teng Long <[email-masked]>
>
> Let's do some cleanup for the following two places in "append_edit()".
>
> The first place is "char *logmsg;" need to be initialized with NULL.
> The second place is "struct note_data d = { 0, 0, NULL, STRBUF_INIT };"
> could be replaced with designated init format.
>
> Signed-off-by: Teng Long <[email-masked]>
> ---
>  builtin/notes.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/notes.c b/builtin/notes.c
> index e57f024824..8ca55ec83e 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -566,9 +566,9 @@ static int append_edit(int argc, const char **argv, const char *prefix)
>  	struct notes_tree *t;
>  	struct object_id object, new_note;
>  	const struct object_id *note;
> -	char *logmsg;
> +	char *logmsg = NULL;

This change isn't needed, and the compiler will check that we have it
init'd.

It *is* needed when combined with your 3/5, but even then it's the wrong
solution. In that change you move the commit_notes() into that "if"
branch that needs the "logmsg", and it's correct that you need to
NULL-init it to safely pass it to free().

But let's just declare the "logmsg" in that if block then, and move the
free() over there, that reduces the scope it's in.

>  	const char * const *usage;
> -	struct note_data d = { 0, 0, NULL, STRBUF_INIT };
> +	struct note_data d = { .buf = STRBUF_INIT };

This change is good, but then let's change the other such case in this
file to a designated init too, and make the commit message etc. be about
using designated init here.