Re: [PATCH v2 5/5] refs/debug: trim trailing LF from reflog message

2 messages, 2 authors, 2021-12-02 · open the first message on its own page

Re: [PATCH v2 5/5] refs/debug: trim trailing LF from reflog message

From: Junio C Hamano <hidden>
Date: 2021-11-29 19:22:08

Han-Wen Nienhuys [off-list ref] writes:
On Fri, Nov 26, 2021 at 9:16 AM Junio C Hamano [off-list ref] wrote:
quoted
The API promises to have only LF, not CRLF, at the end, so
strbuf_trim_trailing_newline() is a bit overkill (and if payload
happened to end with CR, we would lose it).
it would be best if there was a way to escape characters (ie. "\n" =>
"\\n"). Do we have a function for that?
Mere escaping would not work in a backward compatible way, without a
trick.  It was envisioned that we probably could encode *and* signal
the fact that the message is encoded by appending a trailing SP at
the end of the message.  See the log message of 523fa69c (reflog:
cleanse messages in the refs.c layer, 2020-07-10) for details.

Having said that, that is about introducing a whole new reflog
message format (whose use is signalled by the trailing SP), and I
would prefer it to happen

 (1) after we integrate with reftable, and

 (2) implemented as an option in the normalize_reflog_message()
     function, so that no ref backends has to worry about it.

outside this topic.
I beg to differ - despite this being fewer lines of code, I think
pointer arithmetic is best avoided if possible.
I think repeated allocation and deallocation is best avoided, and
that is why I recommended it.  I do not see anything to fear in
poiter arithmetic, as long as it is done clearly (e.g. in a narrow
scope) and correctly.

If trace_printf() does not allow counted bytes "%.*s", the whole
discussion is moot; I didn't go back to check.

Re: [PATCH v2 5/5] refs/debug: trim trailing LF from reflog message

From: Han-Wen Nienhuys <hidden>
Date: 2021-12-02 16:24:42

On Mon, Nov 29, 2021 at 8:20 PM Junio C Hamano [off-list ref] wrote:
Han-Wen Nienhuys [off-list ref] writes:
quoted
On Fri, Nov 26, 2021 at 9:16 AM Junio C Hamano [off-list ref] wrote:
quoted
The API promises to have only LF, not CRLF, at the end, so
strbuf_trim_trailing_newline() is a bit overkill (and if payload
happened to end with CR, we would lose it).
it would be best if there was a way to escape characters (ie. "\n" =>
"\\n"). Do we have a function for that?
Mere escaping would not work in a backward compatible way, without a
trick.  It was envisioned that we probably could encode *and* signal
I'm talking about the debug output, which isn't subject to
compatibility guarantees.

-- 
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--

Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich

Registergericht und -nummer: Hamburg, HRB 86891

Sitz der Gesellschaft: Hamburg

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help