On Tue, May 08, 2018 at 09:42:48AM -0400, Jeff King wrote:
> On Mon, Apr 09, 2018 at 10:32:20AM +0200, Johannes Schindelin wrote:
>
> > +static int store_aux_event(enum config_event_t type,
> > + size_t begin, size_t end, void *data)
> > +{
> > + struct config_store_data *store = data;
> > +
> > + ALLOC_GROW(store->parsed, store->parsed_nr + 1, store->parsed_alloc);
> > + store->parsed[store->parsed_nr].begin = begin;
> > + store->parsed[store->parsed_nr].end = end;
> > + store->parsed[store->parsed_nr].type = type;
> > + store->parsed_nr++;
> > +
> > + if (type == CONFIG_EVENT_SECTION) {
> > + if (cf->var.len < 2 || cf->var.buf[cf->var.len - 1] != '.')
> > + BUG("Invalid section name '%s'", cf->var.buf);
>
> I triggered this BUG today while playing around. Here's a minimal
> reproduction:
>
> echo '[broken' >config
> git config --file=config a.b c
>
> I'm not sure if it should simply be a die() and not a BUG(), since
> it depends on the input. Or if it is a BUG and we expected an earlier
> part of the code (like the event generator) to catch this broken case
> before we get to this function.
By the way, one side effect of BUG() here is that we call abort(), which
means that our atexit handlers don't run. And a crufty "config.lock"
file is left that prevents running the command again.
In our discussion elsewhere of having BUG() just call exit(), I'm not
sure if we'd want it to skip those cleanups or not (it's helpful to
not run them if you're trying to debug, but otherwise is annoying).
-Peff
