On Mon, Nov 29 2021, Han-Wen Nienhuys wrote: > On Fri, Nov 26, 2021 at 9:16 AM Junio C Hamano <gitster@xxxxxxxxx> 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? > >> > + trace_printf_key(&trace_refs, >> > + "reflog_ent %s (ret %d): %s -> %s, %s %ld \"%s\"\n", >> > + dbg->refname, ret, o, n, committer, >> > + (long int)timestamp, trimmed.buf); >> > + strbuf_release(&trimmed); >> > return ret; >> > } >> >> Can we use counted bytes in trace_printf()? If we can, it would be >> simpler to just scan "msg" for LF and then show only the span >> between the beginning of the string and the found LF using "%.*s", >> perhaps like this? > > I beg to differ - despite this being fewer lines of code, I think > pointer arithmetic is best avoided if possible. We usually do this with pointer arithmetic, but the %.*s format doesn't require that, just code like: const char *str = "foobar"; size_t len = strlen(str); len -= 1; /* give me less! */ printf("%.*s", (int)len, str); So you can also feed it (len - 1) or whatever if you know it to end with a character you don't want. It's (more simply done as) pointer arithmetic if you're finding that end marker with strstr() or whatever, but you can also bend over backwards and get a "len" instead through other means, and in either case I think it beats reallocating the whole thing (more for readability than any optimization reasons).