From: René Scharfe <l.s.r@web.de>
Date: Mon, 30 Oct 2023 16:12:41 GMT
Subject: Re: [PATCH] reflog: fix expire --single-worktree
Message-ID: <82b832cb-da0b-47cf-9b5d-e8011a222151@web.de>
In-Reply-To: <xmqqa5s1hxhh.fsf@gitster.g>

Am 29.10.23 um 23:31 schrieb Junio C Hamano:
> René Scharfe <l.s.r@web.de> writes:
>
>> ... and added a non-printable short flag for it, presumably by
>> accident.
>
> Very well spotted.
>
> FWIW, with the following patch on top of this patch, all tests pass
> (and without your fix, of course this notices the "\001" and breaks
> numerous tests that use "git reflog").  So you seem to have found
> the only one broken instance (among those that are tested, anyway).
>
>  parse-options.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git i/parse-options.c w/parse-options.c
> index 093eaf2db8..be8bedba29 100644
> --- i/parse-options.c
> +++ w/parse-options.c
> @@ -469,7 +469,8 @@ static void parse_options_check(const struct option *opts)
>  			optbug(opts, "uses incompatible flags "
>  			       "LASTARG_DEFAULT and OPTARG");
>  		if (opts->short_name) {
> -			if (0x7F <= opts->short_name)
> +			if (opts->short_name &&
> +			    (opts->short_name < 0x21 || 0x7F <= opts->short_name))

Good idea.  This is equivalent to !isprint(opts->short_name), which I
find to be more readable here.  Seeing why "char short_opts[128];" a
few lines up is big enough would become a bit harder, though.

>  				optbug(opts, "invalid short name");
>  			else if (short_opts[opts->short_name]++)
>  				optbug(opts, "short name already used");