{"thread":{"id":"52222","subject":"[PATCH] convert: use skip_iprefix() in validate_encoding()","startedAt":"2019-11-08T20:27:39Z","lastAt":"2019-11-08T22:19:51Z","messageCount":2,"participants":["René Scharfe","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"385808","messageId":"a61b60e4-77f4-10a9-65ff-f78348d4c4b7@web.de","threadId":"52222","inReplyTo":null,"subject":"[PATCH] convert: use skip_iprefix() in validate_encoding()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2019-11-08T20:27:34Z","receivedAt":"2019-11-08T20:27:39Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Use skip_iprefix() to parse \"UTF\" case-insensitively instead of checking\nwith istarts_with(), building an upper-case version and then using\nskip_prefix() on it.  This gets rid of duplicate code and of a small\nallocation.\n\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\n convert.c | 22 ++++++++--------------\n 1 file changed, 8 insertions(+), 14 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex 25ac525d5f..99f684bf3e 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -270,8 +270,12 @@ static int will_convert_lf_to_crlf(struct text_stat *stats,\n static int validate_encoding(const char *path, const char *enc,\n \t\t      const char *data, size_t len, int die_on_error)\n {\n+\tconst char *stripped;\n+\n \t/* We only check for UTF here as UTF?? can be an alias for UTF-?? */\n-\tif (istarts_with(enc, \"UTF\")) {\n+\tif (skip_iprefix(enc, \"UTF\", &stripped)) {\n+\t\tskip_prefix(stripped, \"-\", &stripped);\n+\n \t\t/*\n \t\t * Check for detectable errors in UTF encodings\n \t\t */\n@@ -285,15 +289,10 @@ static int validate_encoding(const char *path, const char *enc,\n \t\t\t */\n \t\t\tconst char *advise_msg = _(\n \t\t\t\t\"The file '%s' contains a byte order \"\n-\t\t\t\t\"mark (BOM). Please use UTF-%s as \"\n+\t\t\t\t\"mark (BOM). Please use UTF-%.*s as \"\n \t\t\t\t\"working-tree-encoding.\");\n-\t\t\tconst char *stripped = NULL;\n-\t\t\tchar *upper = xstrdup_toupper(enc);\n-\t\t\tupper[strlen(upper)-2] = '\\0';\n-\t\t\tif (skip_prefix(upper, \"UTF\", &stripped))\n-\t\t\t\tskip_prefix(stripped, \"-\", &stripped);\n-\t\t\tadvise(advise_msg, path, stripped);\n-\t\t\tfree(upper);\n+\t\t\tint stripped_len = strlen(stripped) - strlen(\"BE\");\n+\t\t\tadvise(advise_msg, path, stripped_len, stripped);\n \t\t\tif (die_on_error)\n \t\t\t\tdie(error_msg, path, enc);\n \t\t\telse {\n@@ -308,12 +307,7 @@ static int validate_encoding(const char *path, const char *enc,\n \t\t\t\t\"mark (BOM). Please use UTF-%sBE or UTF-%sLE \"\n \t\t\t\t\"(depending on the byte order) as \"\n \t\t\t\t\"working-tree-encoding.\");\n-\t\t\tconst char *stripped = NULL;\n-\t\t\tchar *upper = xstrdup_toupper(enc);\n-\t\t\tif (skip_prefix(upper, \"UTF\", &stripped))\n-\t\t\t\tskip_prefix(stripped, \"-\", &stripped);\n \t\t\tadvise(advise_msg, path, stripped, stripped);\n-\t\t\tfree(upper);\n \t\t\tif (die_on_error)\n \t\t\t\tdie(error_msg, path, enc);\n \t\t\telse {\n--\n2.24.0\n"},{"id":"385818","messageId":"20191108221949.GA18519@sigill.intra.peff.net","threadId":"52222","inReplyTo":"a61b60e4-77f4-10a9-65ff-f78348d4c4b7@web.de","subject":"Re: [PATCH] convert: use skip_iprefix() in validate_encoding()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-08T22:19:49Z","receivedAt":"2019-11-08T22:19:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 08, 2019 at 09:27:34PM +0100, René Scharfe wrote:\n\n> @@ -285,15 +289,10 @@ static int validate_encoding(const char *path, const char *enc,\n>  \t\t\t */\n>  \t\t\tconst char *advise_msg = _(\n>  \t\t\t\t\"The file '%s' contains a byte order \"\n> -\t\t\t\t\"mark (BOM). Please use UTF-%s as \"\n> +\t\t\t\t\"mark (BOM). Please use UTF-%.*s as \"\n>  \t\t\t\t\"working-tree-encoding.\");\n> -\t\t\tconst char *stripped = NULL;\n> -\t\t\tchar *upper = xstrdup_toupper(enc);\n> -\t\t\tupper[strlen(upper)-2] = '\\0';\n> -\t\t\tif (skip_prefix(upper, \"UTF\", &stripped))\n> -\t\t\t\tskip_prefix(stripped, \"-\", &stripped);\n> -\t\t\tadvise(advise_msg, path, stripped);\n> -\t\t\tfree(upper);\n> +\t\t\tint stripped_len = strlen(stripped) - strlen(\"BE\");\n> +\t\t\tadvise(advise_msg, path, stripped_len, stripped);\n\nI think we could drop xstrdup_toupper() after this.\n\nOf course it _could_ come in handy in another spot, but in most cases\nyou can use xstrdup_tolower() interchangeably (e.g., that would have\nworked here). And even the tolower() variant is seldom used.\n\n-Peff\n"}]}