From: Junio C Hamano Date: Mon, 29 Nov 2021 19:19:59 GMT Subject: Re: [PATCH v2 5/5] refs/debug: trim trailing LF from reflog message Message-ID: In-Reply-To: Han-Wen Nienhuys writes: > On Fri, Nov 26, 2021 at 9:16 AM Junio C Hamano wrote: >> >> 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.