Hi Peff, On Tue, 29 Oct 2019, Jeff King wrote: > On Tue, Oct 29, 2019 at 03:13:39PM +0100, Johannes Schindelin wrote: > > > > > 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
