Re: [PATCH v9 4/6] notes.c: introduce '--separator=<paragraph-break>' option

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 28, 2023, 20:44 UTC
Message-ID: <xmqqsfcjbuud.fsf@gitster.g>
In-Reply-To: <ed930ef4f795f30792bc14d9c1939484e4976db8.1682671758.git.dyroneteng@gmail.com>

Teng Long <dyroneteng@gmail.com> writes:

> +static char *separator = "\n";

The only two ways this pointer gains a non-NULL value are with this initialization and parsing the command line "--separator=<value>" option with OPT_STRING(). Neither of them allocate new storage but points an existing string that we do not "own" (and cannot free) with the pointer. So it probably is safer to make it a pointer to a const string, i.e.

	static const char *separator = "\n";

Show 18 quoted lines

> @@ -213,65 +229,96 @@ static void write_note_data(struct note_data *d, struct object_id *oid)
>  	}
>  }
>  
> +static void insert_separator(struct strbuf *message, size_t pos)
> +{
> +	if (separator[strlen(separator) - 1] == '\n')
> +		strbuf_addstr(message, separator);
> +	else
> +		strbuf_insertf(message, pos, "%s%s", separator, "\n");
> +}
> +
> +static void concat_messages(struct note_data *d)
> +{
> +	struct strbuf msg = STRBUF_INIT;
> +
> +	size_t i;
> +	for (i = 0; i < d->msg_nr ; i++) {

Wrong placement of the blank line that separates the declaration and the first statement.