Re: [PATCH] convert: fix handling of dashless UTF prefix in validate_encoding()

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 6, 2019, 00:41 UTC
Message-ID: <xmqqeezq66yt.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <c886671c-7753-6ac8-fefc-277e76019cd4@web.de>

René Scharfe <l.s.r@web.de> writes:

> Strip "UTF" and an optional dash from the start of 'upper' without
> passing a NULL pointer to skip_prefix() in the second call, as it cannot
> handle that.

Did the original meant to say "skip UTF- from the beginning of upper and store it to stripped, or if that cannot be done, skip UTF from the beginning of upper and store it to stripped", but made the second one scan the stripped instead of upper by mistake?

Changing it to "skip UTF from upper and store it to stripped, and if the resulting stripped begins with dash, advance stripped to skip that too" is certainly a right fix. Fixing the second one to scan upper would also make the result correct, but there is no point scanning the same thing twice to skip the same leading "UTF" substring.

Thanks.

Show 34 quoted lines

>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
>  convert.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/convert.c b/convert.c
> index deb6f71b2d..25ac525d5f 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -290,8 +290,8 @@ static int validate_encoding(const char *path, const char *enc,
>  			const char *stripped = NULL;
>  			char *upper = xstrdup_toupper(enc);
>  			upper[strlen(upper)-2] = '\0';
> -			if (!skip_prefix(upper, "UTF-", &stripped))
> -				skip_prefix(stripped, "UTF", &stripped);
> +			if (skip_prefix(upper, "UTF", &stripped))
> +				skip_prefix(stripped, "-", &stripped);
>  			advise(advise_msg, path, stripped);
>  			free(upper);
>  			if (die_on_error)
> @@ -310,8 +310,8 @@ static int validate_encoding(const char *path, const char *enc,
>  				"working-tree-encoding.");
>  			const char *stripped = NULL;
>  			char *upper = xstrdup_toupper(enc);
> -			if (!skip_prefix(upper, "UTF-", &stripped))
> -				skip_prefix(stripped, "UTF", &stripped);
> +			if (skip_prefix(upper, "UTF", &stripped))
> +				skip_prefix(stripped, "-", &stripped);
>  			advise(advise_msg, path, stripped, stripped);
>  			free(upper);
>  			if (die_on_error)
> --
> 2.23.0