{"thread":{"id":"43996","subject":"[PATCH] introduce hex2chr() for converting two hexadecimal digits to a character","startedAt":"2016-09-03T16:01:15Z","lastAt":"2016-09-06T15:50:10Z","messageCount":3,"participants":["René Scharfe","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"300954","messageId":"ac454d89-e1cc-083e-5cea-fc9751de9a0f@web.de","threadId":"43996","inReplyTo":null,"subject":"[PATCH] introduce hex2chr() for converting two hexadecimal digits to a character","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2016-09-03T15:59:20Z","receivedAt":"2016-09-03T16:01:15Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Add and use a helper function that decodes the char value of two\nhexadecimal digits.  It returns a negative number on error, avoids\nrunning over the end of the given string and doesn't shift negative\nvalues.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n cache.h      | 10 ++++++++++\n hex.c        | 12 ++----------\n pkt-line.c   | 23 ++---------------------\n pretty.c     | 13 +++++--------\n ref-filter.c | 20 +-------------------\n url.c        | 21 +--------------------\n 6 files changed, 21 insertions(+), 78 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex b780a91..b0dae4b 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1139,6 +1139,16 @@ static inline unsigned int hexval(unsigned char c)\n \treturn hexval_table[c];\n }\n \n+/*\n+ * Convert two consecutive hexadecimal digits into a char.  Return a\n+ * negative value on error.  Don't run over the end of short strings.\n+ */\n+static inline int hex2chr(const char *s)\n+{\n+\tint val = hexval(s[0]);\n+\treturn (val < 0) ? val : (val << 4) | hexval(s[1]);\n+}\n+\n /* Convert to/from hex/sha1 representation */\n #define MINIMUM_ABBREV minimum_abbrev\n #define DEFAULT_ABBREV default_abbrev\ndiff --git a/hex.c b/hex.c\nindex 9619b67..ab2610e 100644\n--- a/hex.c\n+++ b/hex.c\n@@ -39,16 +39,8 @@ int get_sha1_hex(const char *hex, unsigned char *sha1)\n {\n \tint i;\n \tfor (i = 0; i < GIT_SHA1_RAWSZ; i++) {\n-\t\tunsigned int val;\n-\t\t/*\n-\t\t * hex[1]=='\\0' is caught when val is checked below,\n-\t\t * but if hex[0] is NUL we have to avoid reading\n-\t\t * past the end of the string:\n-\t\t */\n-\t\tif (!hex[0])\n-\t\t\treturn -1;\n-\t\tval = (hexval(hex[0]) << 4) | hexval(hex[1]);\n-\t\tif (val & ~0xff)\n+\t\tint val = hex2chr(hex);\n+\t\tif (val < 0)\n \t\t\treturn -1;\n \t\t*sha1++ = val;\n \t\thex += 2;\ndiff --git a/pkt-line.c b/pkt-line.c\nindex 62fdb37..30489c6 100644\n--- a/pkt-line.c\n+++ b/pkt-line.c\n@@ -172,27 +172,8 @@ static int get_packet_data(int fd, char **src_buf, size_t *src_size,\n \n static int packet_length(const char *linelen)\n {\n-\tint n;\n-\tint len = 0;\n-\n-\tfor (n = 0; n < 4; n++) {\n-\t\tunsigned char c = linelen[n];\n-\t\tlen <<= 4;\n-\t\tif (c >= '0' && c <= '9') {\n-\t\t\tlen += c - '0';\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (c >= 'a' && c <= 'f') {\n-\t\t\tlen += c - 'a' + 10;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (c >= 'A' && c <= 'F') {\n-\t\t\tlen += c - 'A' + 10;\n-\t\t\tcontinue;\n-\t\t}\n-\t\treturn -1;\n-\t}\n-\treturn len;\n+\tint val = hex2chr(linelen);\n+\treturn (val < 0) ? val : (val << 8) | hex2chr(linelen + 2);\n }\n \n int packet_read(int fd, char **src_buf, size_t *src_len,\ndiff --git a/pretty.c b/pretty.c\nindex 9609afb..9788bd8 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1065,7 +1065,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \tconst struct commit *commit = c->commit;\n \tconst char *msg = c->message;\n \tstruct commit_list *p;\n-\tint h1, h2;\n+\tint ch;\n \n \t/* these are independent of the commit */\n \tswitch (placeholder[0]) {\n@@ -1089,14 +1089,11 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t\treturn 1;\n \tcase 'x':\n \t\t/* %x00 == NUL, %x0a == LF, etc. */\n-\t\tif (0 <= (h1 = hexval_table[0xff & placeholder[1]]) &&\n-\t\t    h1 <= 16 &&\n-\t\t    0 <= (h2 = hexval_table[0xff & placeholder[2]]) &&\n-\t\t    h2 <= 16) {\n-\t\t\tstrbuf_addch(sb, (h1<<4)|h2);\n-\t\t\treturn 3;\n-\t\t} else\n+\t\tch = hex2chr(placeholder + 1);\n+\t\tif (ch < 0)\n \t\t\treturn 0;\n+\t\tstrbuf_addch(sb, ch);\n+\t\treturn 3;\n \tcase 'w':\n \t\tif (placeholder[1] == '(') {\n \t\t\tunsigned long width = 0, indent1 = 0, indent2 = 0;\ndiff --git a/ref-filter.c b/ref-filter.c\nindex bc551a7..9adbb8a 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1576,24 +1576,6 @@ void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)\n \tqsort(array->items, array->nr, sizeof(struct ref_array_item *), compare_refs);\n }\n \n-static int hex1(char ch)\n-{\n-\tif ('0' <= ch && ch <= '9')\n-\t\treturn ch - '0';\n-\telse if ('a' <= ch && ch <= 'f')\n-\t\treturn ch - 'a' + 10;\n-\telse if ('A' <= ch && ch <= 'F')\n-\t\treturn ch - 'A' + 10;\n-\treturn -1;\n-}\n-static int hex2(const char *cp)\n-{\n-\tif (cp[0] && cp[1])\n-\t\treturn (hex1(cp[0]) << 4) | hex1(cp[1]);\n-\telse\n-\t\treturn -1;\n-}\n-\n static void append_literal(const char *cp, const char *ep, struct ref_formatting_state *state)\n {\n \tstruct strbuf *s = &state->stack->output;\n@@ -1603,7 +1585,7 @@ static void append_literal(const char *cp, const char *ep, struct ref_formatting\n \t\t\tif (cp[1] == '%')\n \t\t\t\tcp++;\n \t\t\telse {\n-\t\t\t\tint ch = hex2(cp + 1);\n+\t\t\t\tint ch = hex2chr(cp + 1);\n \t\t\t\tif (0 <= ch) {\n \t\t\t\t\tstrbuf_addch(s, ch);\n \t\t\t\t\tcp += 3;\ndiff --git a/url.c b/url.c\nindex 2d89ad1..eaf4f07 100644\n--- a/url.c\n+++ b/url.c\n@@ -29,25 +29,6 @@ int is_url(const char *url)\n \treturn (url[0] == ':' && url[1] == '/' && url[2] == '/');\n }\n \n-static int url_decode_char(const char *q)\n-{\n-\tint i;\n-\tunsigned char val = 0;\n-\tfor (i = 0; i < 2; i++) {\n-\t\tunsigned char c = *q++;\n-\t\tval <<= 4;\n-\t\tif (c >= '0' && c <= '9')\n-\t\t\tval += c - '0';\n-\t\telse if (c >= 'a' && c <= 'f')\n-\t\t\tval += c - 'a' + 10;\n-\t\telse if (c >= 'A' && c <= 'F')\n-\t\t\tval += c - 'A' + 10;\n-\t\telse\n-\t\t\treturn -1;\n-\t}\n-\treturn val;\n-}\n-\n static char *url_decode_internal(const char **query, int len,\n \t\t\t\t const char *stop_at, struct strbuf *out,\n \t\t\t\t int decode_plus)\n@@ -66,7 +47,7 @@ static char *url_decode_internal(const char **query, int len,\n \t\t}\n \n \t\tif (c == '%') {\n-\t\t\tint val = url_decode_char(q + 1);\n+\t\t\tint val = hex2chr(q + 1);\n \t\t\tif (0 <= val) {\n \t\t\t\tstrbuf_addch(out, val);\n \t\t\t\tq += 3;\n-- \n2.10.0\n\n"},{"id":"300973","messageId":"alpine.DEB.2.20.1609040947211.129229@virtualbox","threadId":"43996","inReplyTo":"ac454d89-e1cc-083e-5cea-fc9751de9a0f@web.de","subject":"Re: [PATCH] introduce hex2chr() for converting two hexadecimal digits to a character","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-09-04T07:49:20Z","receivedAt":"2016-09-04T07:55:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi René,\n\nOn Sat, 3 Sep 2016, René Scharfe wrote:\n\n> Add and use a helper function that decodes the char value of two\n> hexadecimal digits.  It returns a negative number on error, avoids\n> running over the end of the given string and doesn't shift negative\n> values.\n\nI like it! Maybe stress a little bit why this is a good change? Like, DRY\nup code, makes the code safer (bt avoiding shifting negative values)?\n\n>  6 files changed, 21 insertions(+), 78 deletions(-)\n\nVery, very nice!\n\nThanks,\nDscho"},{"id":"301195","messageId":"86d0aa97-f56b-3d6a-c6bc-07697ac1256a@web.de","threadId":"43996","inReplyTo":"alpine.DEB.2.20.1609040947211.129229@virtualbox","subject":"Re: [PATCH] introduce hex2chr() for converting two hexadecimal digits to a character","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2016-09-06T15:48:21Z","receivedAt":"2016-09-06T15:50:10Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 04.09.2016 um 09:49 schrieb Johannes Schindelin:\n> Hi René,\n>\n> On Sat, 3 Sep 2016, René Scharfe wrote:\n>\n>> Add and use a helper function that decodes the char value of two\n>> hexadecimal digits.  It returns a negative number on error, avoids\n>> running over the end of the given string and doesn't shift negative\n>> values.\n>\n> I like it! Maybe stress a little bit why this is a good change? Like, DRY\n> up code, makes the code safer (bt avoiding shifting negative values)?\n>\n>>  6 files changed, 21 insertions(+), 78 deletions(-)\n>\n> Very, very nice!\n\nThat's the main reason: Consistency.  It's intended to be a safe, easy \nto use and reasonably fast replacement for those other (lengthy) \nvariations.\n\nRené\n"}]}