Re: [PATCH v10 9/9] convert: add round trip check based on 'core.checkRoundtripEncoding'
- Date: Wed, 07 Mar 2018 11:59:45 -0800
- From: Junio C Hamano <gitster@xxxxxxxxx>
- Subject: Re: [PATCH v10 9/9] convert: add round trip check based on 'core.checkRoundtripEncoding'
> +static int check_roundtrip(const char* enc_name)
The asterisk sticks to the variable, not type.
> + /*
> + * check_roundtrip_encoding contains a string of space and/or
> + * comma separated encodings (eg. "UTF-16, ASCII, CP1125").
> + * Search for the given encoding in that string.
> + */
> + const char *found = strcasestr(check_roundtrip_encoding, enc_name);
> + const char *next;
> + int len;
> + if (!found)
> + return 0;
> + next = found + strlen(enc_name);
> + len = strlen(check_roundtrip_encoding);
> + return (found && (
> + /*
> + * check that the found encoding is at the
> + * beginning of check_roundtrip_encoding or
> + * that it is prefixed with a space or comma
> + */
> + found == check_roundtrip_encoding || (
> + found > check_roundtrip_encoding &&
> + (*(found-1) == ' ' || *(found-1) == ',')
> + )
The second line is unneeded, as we know a non-NULL found either
points at check_roundtrip_encoding or somewhere to the right, and
the first test already checked the "points exactly at" case.
This is defined to be a comma separated list, so it is unnecessary
to accept <cre,en> == <"FOO, SHIFT-JIS, BAR", "SHIFT-JIS">; if you
allow SP, perhaps "isspace(found[-1]) || found[-1] == ','" to also
allow HT may also be appropriate. I think "comma or whitespace
separated list" is fine; in any case, the comment near the beginning
of this function does not match new text in Documentation/config.txt
added by this patch.