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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 27, 2023, 18:21 UTC
Message-ID: <xmqq8redkwyr.fsf@gitster.g>
In-Reply-To: <20230427072107.29821-1-tenglong.tl@alibaba-inc.com>

Teng Long <dyroneteng@gmail.com> writes:

Show 14 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
>
>>> +	while (d->msg_nr) {
>>> +		--d->msg_nr;
>>> +		strbuf_release(&d->messages[d->msg_nr]->buf);
>>> +		free(d->messages[d->msg_nr]);
>>> +	}
>>
>>        while (d->msg_nr--) {
>>                strbuf_relesae(...);
>>                free(d->messages[d->msg_nr]);
>>        }
>
> Will fix or optimize.

Not a huge thing and I would be very surprised if you saw a different machine code output. The original stood out like a sore thumb at me, primarily because we rarely if ever do pre-decrement or pre-increment in the void context in our own code.

Show 13 quoted lines

>>Interesting.  
>>
>>I would have expected that where we add to d->messages[] array a new
>>note_msg structure and set its .stripspace member, we already knew
>>if the associated strbuf needs to be stripspace'd and instead of
>>maintaining the .stripspace member for each note_msg, we can just
>>have it contain only a string (not even a strbuf).
>
> That's because the "-C" option, it will not do stripspace for the
> whole note, but support to use together with "-m" or "-F"(they will
> do stripspace). So, for the consistent result with backforward
> compatibility, we have to get the information about whether to do
> stripspace when concating each message.

OK.

Thanks.