Hi Peff,

On Tue, 29 Oct 2019, Jeff King wrote:

On Tue, Oct 29, 2019 at 03:13:39PM +0100, Johannes Schindelin wrote:

quoted

quoted

I'd disagree here. Any caller sending an arbitrarily-large prefix
is holding it wrong, and we'd probably want to know as soon as
possible (and a BUG() is our best bet there).

How about truncating already inside the prefix, then? It would miss
the entire error message... but at least it would print
_something_...

Yeah, that might be OK. Hopefully missing the whole rest of the error
message would cause some tests to fail.

I am not really worried about that. So far, we only have prefixes like
`"fatal: "` or `"error: "`. The longest prefix is in `BUG_fl()` itself,
when we have access to the file name and the line number of the
triggering line, in which case it is `"BUG: <file>:<line>: "`. So as
long as nobody builds a custom version of Git that includes a file whose
path is insanely long, they should be fine. I am certain that Junio will
never accept such a file into git.git.

Oh, and even if anybody would introduce insanely long paths into their
fork of git.git, they would _still_ be safe because `BUG_fl()` limits
the prefix to 256 bytes, including `NUL`.

You could also abort() after having written if we want to be more
BUG()-like.

As I said, this is really a part of the patch I am not at all concerned
about. The prefixes are well under control, and I only addressed that
potential future breakage (that I believe falls squarely into YAGNI
territory) only because two reviewers seemed to be concerned.

I really believe that the care I put into the patch to safeguard against
overly long prefixes is seriously overkill.

Ciao,
Dscho