Re: [PATCH v2 13/16] parse-options.c: turn some die() to BUG()

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 6, 2018, 03:27 UTC
Message-ID: <xmqqh8guga1x.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20181105192059.20303-14-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

Show 12 quoted lines

> diff --git a/parse-options.c b/parse-options.c
> index 0bf817193d..3f5f985c1e 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -197,7 +197,7 @@ static int get_value(struct parse_opt_ctx_t *p,
>  		return 0;
>  
>  	default:
> -		die("should not happen, someone must be hit on the forehead");
> +		BUG("opt->type %d should not happen", opt->type);
>  	}
>  }

OK, this should not happen.

Show 6 quoted lines

> @@ -424,7 +424,7 @@ void parse_options_start(struct parse_opt_ctx_t *ctx,
>  	ctx->flags = flags;
>  	if ((flags & PARSE_OPT_KEEP_UNKNOWN) &&
>  	    (flags & PARSE_OPT_STOP_AT_NON_OPTION))
> -		die("STOP_AT_NON_OPTION and KEEP_UNKNOWN don't go together");
> +		BUG("STOP_AT_NON_OPTION and KEEP_UNKNOWN don't go together");

The correctness of this conversion was not immediately obvious, as "git rev-parse --parse-options" could allow end-users to specify flags, and when these two flags came together from that codepath, it is an end-user error that should be diagnosed with die(), not BUG().

It turns out that stop-at-non-option can be passed, but keep-unknown is not (yet) available to the codepath, so this conversion to BUG() is correct---an end user futzing with Git correctly compiled from a bug-free source should not be able to trigger this.