{"thread":{"id":"47082","subject":"[PATCH 1/3] notes: move hex_to_bytes() to hex.c and export it","startedAt":"2017-10-31T13:47:16Z","lastAt":"2017-11-05T19:57:56Z","messageCount":11,"participants":["René Scharfe","Jeff King","Kevin Daudt"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"331473","messageId":"508b1b3f-6b55-eee8-110a-c17d572ec27a@web.de","threadId":"47082","inReplyTo":null,"subject":"[PATCH 1/3] notes: move hex_to_bytes() to hex.c and export it","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-10-31T13:46:49Z","receivedAt":"2017-10-31T13:47:16Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Make the function for converting pairs of hexadecimal digits to binary\navailable to other call sites.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n cache.h |  7 +++++++\n hex.c   | 12 ++++++++++++\n notes.c | 17 -----------------\n 3 files changed, 19 insertions(+), 17 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 6440e2bf21..f06bfbaf32 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1317,6 +1317,13 @@ extern int set_disambiguate_hint_config(const char *var, const char *value);\n extern int get_sha1_hex(const char *hex, unsigned char *sha1);\n extern int get_oid_hex(const char *hex, struct object_id *sha1);\n \n+/*\n+ * Read `len` pairs of hexadecimal digits from `hex` and write the\n+ * values to `binary` as `len` bytes. Return 0 on success, or -1 if\n+ * the input does not consist of hex digits).\n+ */\n+extern int hex_to_bytes(unsigned char *binary, const char *hex, size_t len);\n+\n /*\n  * Convert a binary sha1 to its hex equivalent. The `_r` variant is reentrant,\n  * and writes the NUL-terminated output to the buffer `out`, which must be at\ndiff --git a/hex.c b/hex.c\nindex 28b44118cb..8df2d63728 100644\n--- a/hex.c\n+++ b/hex.c\n@@ -35,6 +35,18 @@ const signed char hexval_table[256] = {\n \t -1, -1, -1, -1, -1, -1, -1, -1,\t\t/* f8-ff */\n };\n \n+int hex_to_bytes(unsigned char *binary, const char *hex, size_t len)\n+{\n+\tfor (; len; len--, hex += 2) {\n+\t\tunsigned int val = (hexval(hex[0]) << 4) | hexval(hex[1]);\n+\n+\t\tif (val & ~0xff)\n+\t\t\treturn -1;\n+\t\t*binary++ = val;\n+\t}\n+\treturn 0;\n+}\n+\n int get_sha1_hex(const char *hex, unsigned char *sha1)\n {\n \tint i;\ndiff --git a/notes.c b/notes.c\nindex 5c62862574..04f8c8613c 100644\n--- a/notes.c\n+++ b/notes.c\n@@ -334,23 +334,6 @@ static void note_tree_free(struct int_node *tree)\n \t}\n }\n \n-/*\n- * Read `len` pairs of hexadecimal digits from `hex` and write the\n- * values to `binary` as `len` bytes. Return 0 on success, or -1 if\n- * the input does not consist of hex digits).\n- */\n-static int hex_to_bytes(unsigned char *binary, const char *hex, size_t len)\n-{\n-\tfor (; len; len--, hex += 2) {\n-\t\tunsigned int val = (hexval(hex[0]) << 4) | hexval(hex[1]);\n-\n-\t\tif (val & ~0xff)\n-\t\t\treturn -1;\n-\t\t*binary++ = val;\n-\t}\n-\treturn 0;\n-}\n-\n static int non_note_cmp(const struct non_note *a, const struct non_note *b)\n {\n \treturn strcmp(a->path, b->path);\n-- \n2.15.0\n"},{"id":"331474","messageId":"31ce5c1c-cab3-80aa-623b-43f91ddd2162@web.de","threadId":"47082","inReplyTo":"508b1b3f-6b55-eee8-110a-c17d572ec27a@web.de","subject":"[PATCH 2/3] http-push: use hex_to_bytes()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-10-31T13:49:56Z","receivedAt":"2017-10-31T13:50:19Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"The path of a loose object contains its hash value encoded into two\nsubstrings of hexadecimal digits, separated by a slash.  The current\ncode copies the pieces into a temporary buffer to get rid of the slash\nand then uses get_oid_hex() to decode the hash value.\n\nAvoid the copy by using hex_to_bytes() directly on the substrings.\nThat's shorter and easier.\n\nWhile at it correct the length of the second substring in a comment.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n http-push.c | 10 ++++------\n 1 file changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex 493ee7d719..14435ab65d 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1007,20 +1007,18 @@ static void remote_ls(const char *path, int flags,\n \t\t      void (*userFunc)(struct remote_ls_ctx *ls),\n \t\t      void *userData);\n \n-/* extract hex from sharded \"xx/x{40}\" filename */\n+/* extract hex from sharded \"xx/x{38}\" filename */\n static int get_oid_hex_from_objpath(const char *path, struct object_id *oid)\n {\n-\tchar hex[GIT_MAX_HEXSZ];\n-\n \tif (strlen(path) != GIT_SHA1_HEXSZ + 1)\n \t\treturn -1;\n \n-\tmemcpy(hex, path, 2);\n+\tif (hex_to_bytes(oid->hash, path, 1))\n+\t\treturn -1;\n \tpath += 2;\n \tpath++; /* skip '/' */\n-\tmemcpy(hex + 2, path, GIT_SHA1_HEXSZ - 2);\n \n-\treturn get_oid_hex(hex, oid);\n+\treturn hex_to_bytes(oid->hash + 1, path, GIT_SHA1_RAWSZ - 1);\n }\n \n static void process_ls_object(struct remote_ls_ctx *ls)\n-- \n2.15.0\n"},{"id":"331475","messageId":"d8f73744-b3f5-0fca-d58c-1f60e79214e0@web.de","threadId":"47082","inReplyTo":"508b1b3f-6b55-eee8-110a-c17d572ec27a@web.de","subject":"[PATCH 3/3] sha1_file: use hex_to_bytes()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-10-31T13:50:06Z","receivedAt":"2017-10-31T13:50:26Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"The path of a loose object contains its hash value encoded into two\nsubstrings of 2 and 38 hexadecimal digits separated by a slash.  The\nfirst part is handed to for_each_file_in_obj_subdir() in decoded form as\nsubdir_nr.  The current code builds a full hexadecimal representation of\nthe hash in a temporary buffer, then uses get_oid_hex() to decode it.\n\nAvoid the intermediate step by taking subdir_nr as-is and using\nhex_to_bytes() directly on the second substring.  That's shorter and\neasier.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n sha1_file.c | 24 +++++++++++-------------\n 1 file changed, 11 insertions(+), 13 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 10c3a0083d..a3c32d91d1 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -1884,6 +1884,7 @@ int for_each_file_in_obj_subdir(unsigned int subdir_nr,\n \tDIR *dir;\n \tstruct dirent *de;\n \tint r = 0;\n+\tstruct object_id oid;\n \n \tif (subdir_nr > 0xff)\n \t\tBUG(\"invalid loose object subdirectory: %x\", subdir_nr);\n@@ -1901,6 +1902,8 @@ int for_each_file_in_obj_subdir(unsigned int subdir_nr,\n \t\treturn r;\n \t}\n \n+\toid.hash[0] = subdir_nr;\n+\n \twhile ((de = readdir(dir))) {\n \t\tif (is_dot_or_dotdot(de->d_name))\n \t\t\tcontinue;\n@@ -1908,20 +1911,15 @@ int for_each_file_in_obj_subdir(unsigned int subdir_nr,\n \t\tstrbuf_setlen(path, baselen);\n \t\tstrbuf_addf(path, \"/%s\", de->d_name);\n \n-\t\tif (strlen(de->d_name) == GIT_SHA1_HEXSZ - 2)  {\n-\t\t\tchar hex[GIT_MAX_HEXSZ+1];\n-\t\t\tstruct object_id oid;\n-\n-\t\t\txsnprintf(hex, sizeof(hex), \"%02x%s\",\n-\t\t\t\t  subdir_nr, de->d_name);\n-\t\t\tif (!get_oid_hex(hex, &oid)) {\n-\t\t\t\tif (obj_cb) {\n-\t\t\t\t\tr = obj_cb(&oid, path->buf, data);\n-\t\t\t\t\tif (r)\n-\t\t\t\t\t\tbreak;\n-\t\t\t\t}\n-\t\t\t\tcontinue;\n+\t\tif (strlen(de->d_name) == GIT_SHA1_HEXSZ - 2 &&\n+\t\t    !hex_to_bytes(oid.hash + 1, de->d_name,\n+\t\t\t\t  GIT_SHA1_RAWSZ - 1)) {\n+\t\t\tif (obj_cb) {\n+\t\t\t\tr = obj_cb(&oid, path->buf, data);\n+\t\t\t\tif (r)\n+\t\t\t\t\tbreak;\n \t\t\t}\n+\t\t\tcontinue;\n \t\t}\n \n \t\tif (cruft_cb) {\n-- \n2.15.0\n"},{"id":"331598","messageId":"20171101195530.odscytks7jvgukbs@sigill.intra.peff.net","threadId":"47082","inReplyTo":"31ce5c1c-cab3-80aa-623b-43f91ddd2162@web.de","subject":"Re: [PATCH 2/3] http-push: use hex_to_bytes()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-11-01T19:55:30Z","receivedAt":"2017-11-01T19:55:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 31, 2017 at 02:49:56PM +0100, René Scharfe wrote:\n\n> The path of a loose object contains its hash value encoded into two\n> substrings of hexadecimal digits, separated by a slash.  The current\n> code copies the pieces into a temporary buffer to get rid of the slash\n> and then uses get_oid_hex() to decode the hash value.\n> \n> Avoid the copy by using hex_to_bytes() directly on the substrings.\n> That's shorter and easier.\n> \n> While at it correct the length of the second substring in a comment.\n\nI think the patch is correct, but I wonder if this approach will bite us\nlater on when we start dealing with multiple lengths of hashes.\n\nThe hex_to_bytes() function requires that the caller make sure they have\nthe right number of bytes. But for many callers, I think they'd want to\nsay \"parse this oid, which might be truncated; I can't tell what the\nlength is supposed to be\".\n\nI.e., I wonder if the right primitive is really something like\nparse_oid_hex(), but with a flag to say \"skip over interior slashes\".\n\nWe don't need to deal with that eventuality yet, but I'm on the fence on\nwhether this patch is making that harder down the road or not. The\ncurrent strategy of \"stuff it into a buffer without slashes\" would be\neasier to convert, I think.\n\n-Peff\n"},{"id":"331600","messageId":"20171101195853.tu7wfpzo5cqoqwbo@sigill.intra.peff.net","threadId":"47082","inReplyTo":"d8f73744-b3f5-0fca-d58c-1f60e79214e0@web.de","subject":"Re: [PATCH 3/3] sha1_file: use hex_to_bytes()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-11-01T19:58:53Z","receivedAt":"2017-11-01T19:59:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 31, 2017 at 02:50:06PM +0100, René Scharfe wrote:\n\n> The path of a loose object contains its hash value encoded into two\n> substrings of 2 and 38 hexadecimal digits separated by a slash.  The\n> first part is handed to for_each_file_in_obj_subdir() in decoded form as\n> subdir_nr.  The current code builds a full hexadecimal representation of\n> the hash in a temporary buffer, then uses get_oid_hex() to decode it.\n> \n> Avoid the intermediate step by taking subdir_nr as-is and using\n> hex_to_bytes() directly on the second substring.  That's shorter and\n> easier.\n\nThis raises some of the same questions as the previous one on whether\nhex_to_bytes() is the ideal abstraction. But as before, I'm on the\nfence.\n\n> @@ -1908,20 +1911,15 @@ int for_each_file_in_obj_subdir(unsigned int subdir_nr,\n>  \t\tstrbuf_setlen(path, baselen);\n>  \t\tstrbuf_addf(path, \"/%s\", de->d_name);\n>  \n> -\t\tif (strlen(de->d_name) == GIT_SHA1_HEXSZ - 2)  {\n> -\t\t\tchar hex[GIT_MAX_HEXSZ+1];\n> -\t\t\tstruct object_id oid;\n> -\n> -\t\t\txsnprintf(hex, sizeof(hex), \"%02x%s\",\n> -\t\t\t\t  subdir_nr, de->d_name);\n> -\t\t\tif (!get_oid_hex(hex, &oid)) {\n> -\t\t\t\tif (obj_cb) {\n> -\t\t\t\t\tr = obj_cb(&oid, path->buf, data);\n> -\t\t\t\t\tif (r)\n> -\t\t\t\t\t\tbreak;\n> -\t\t\t\t}\n> -\t\t\t\tcontinue;\n> +\t\tif (strlen(de->d_name) == GIT_SHA1_HEXSZ - 2 &&\n> +\t\t    !hex_to_bytes(oid.hash + 1, de->d_name,\n> +\t\t\t\t  GIT_SHA1_RAWSZ - 1)) {\n> +\t\t\tif (obj_cb) {\n> +\t\t\t\tr = obj_cb(&oid, path->buf, data);\n> +\t\t\t\tif (r)\n> +\t\t\t\t\tbreak;\n>  \t\t\t}\n> +\t\t\tcontinue;\n>  \t\t}\n>  \n>  \t\tif (cruft_cb) {\n\nNow that this is one big conditional for \"is this a valid object\nfilename\", I think we could get rid of the \"continue\" in favor of:\n\n  if (...looks like an object...)\n          ...call obj_cb...\n  else if (cruft_cb)\n          ...call cruft_cb...\n\nNot a big deal, but it may make the flow more clear (the original had to\nuse a continue because there were multiple independent steps to\ndetermining it was an object file, so we had to \"break out\" from the\ninner conditional).\n\n-Peff\n"},{"id":"331617","messageId":"0e7f821a-94ae-3946-fb99-c508a06a6847@web.de","threadId":"47082","inReplyTo":"20171101195530.odscytks7jvgukbs@sigill.intra.peff.net","subject":"Re: [PATCH 2/3] http-push: use hex_to_bytes()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-11-01T21:59:49Z","receivedAt":"2017-11-01T22:00:20Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 01.11.2017 um 20:55 schrieb Jeff King:\n> On Tue, Oct 31, 2017 at 02:49:56PM +0100, René Scharfe wrote:\n> \n>> The path of a loose object contains its hash value encoded into two\n>> substrings of hexadecimal digits, separated by a slash.  The current\n>> code copies the pieces into a temporary buffer to get rid of the slash\n>> and then uses get_oid_hex() to decode the hash value.\n>>\n>> Avoid the copy by using hex_to_bytes() directly on the substrings.\n>> That's shorter and easier.\n>>\n>> While at it correct the length of the second substring in a comment.\n> \n> I think the patch is correct, but I wonder if this approach will bite us\n> later on when we start dealing with multiple lengths of hashes.\n> \n> The hex_to_bytes() function requires that the caller make sure they have\n> the right number of bytes. But for many callers, I think they'd want to\n> say \"parse this oid, which might be truncated; I can't tell what the\n> length is supposed to be\".\n\nI'm confused by the word \"many\".  After this series there are three\ncallers of hex_to_bytes() and I don't expect that number to grow.\n\nWould loose objects be stored at paths containing only a subset of their\nnew hash value?  If they won't then there will be two acceptable lengths\ninstead of the one we have today, which should be easy to handle.\n\n> I.e., I wonder if the right primitive is really something like\n> parse_oid_hex(), but with a flag to say \"skip over interior slashes\".\n\nHmm, ignoring any slashes is an interesting idea.  Is that too loose, I\nwonder?  And I don't think it makes for a good primitive, as slashes\nonly need to be ignored in a single place so far (here in http-push.c).\nCollecting special cases in a central place, guarded by flags, doesn't\nreduce the overall complexity.\n\n> We don't need to deal with that eventuality yet, but I'm on the fence on\n> whether this patch is making that harder down the road or not. The\n> current strategy of \"stuff it into a buffer without slashes\" would be\n> easier to convert, I think.\n\nHow so?  If you have a buffer then you need to know the size of the\ndata to copy into it as well, or you'll learn it in the process.\n\nThe call sites of hex_to_bytes() have to be modified along with the\nfunctions in hex.c to support longer hashes, with or without this\nseries.\n\nRené\n"},{"id":"331620","messageId":"20171101221541.6dwezd4t4hweuis2@sigill.intra.peff.net","threadId":"47082","inReplyTo":"0e7f821a-94ae-3946-fb99-c508a06a6847@web.de","subject":"Re: [PATCH 2/3] http-push: use hex_to_bytes()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-11-01T22:15:42Z","receivedAt":"2017-11-01T22:15:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 01, 2017 at 10:59:49PM +0100, René Scharfe wrote:\n\n> > The hex_to_bytes() function requires that the caller make sure they have\n> > the right number of bytes. But for many callers, I think they'd want to\n> > say \"parse this oid, which might be truncated; I can't tell what the\n> > length is supposed to be\".\n> \n> I'm confused by the word \"many\".  After this series there are three\n> callers of hex_to_bytes() and I don't expect that number to grow.\n\nI meant only that most callers that parse oids, both in-file and not,\nwould want to stop knowing about the length ahead of time. I think\nparse_oid_hex() solves that problem for most callers.\n\n> Would loose objects be stored at paths containing only a subset of their\n> new hash value?  If they won't then there will be two acceptable lengths\n> instead of the one we have today, which should be easy to handle.\n\nI don't know. TBH, I'm not sure anyone has much interest in making\nhttp-push work with new hashes. I'd be OK if it simply doesn't until\nsomebody interested shows up to change that.\n\n> > We don't need to deal with that eventuality yet, but I'm on the fence on\n> > whether this patch is making that harder down the road or not. The\n> > current strategy of \"stuff it into a buffer without slashes\" would be\n> > easier to convert, I think.\n> \n> How so?  If you have a buffer then you need to know the size of the\n> data to copy into it as well, or you'll learn it in the process.\n> \n> The call sites of hex_to_bytes() have to be modified along with the\n> functions in hex.c to support longer hashes, with or without this\n> series.\n\nYou have to know how big the data you have is, but you don't necessarily\nknow whether that makes a complete hash or not. With a \"remove slashes\nand then parse\" strategy, you can do the removing without worrying about\nhow big things are _supposed_ to be, and then the parser can tell you if\nyou have a valid oid or not. The logic for what a hash looks like _and_\nhow big it must be are both in the parser.\n\nWith the new code you have here, we have to be a bit more intimate with\nSHA1_HEXSZ in the calling code. It knows that the hash consists of a\ncertain number of hex bytes.\n\nI'm perfectly willing to punt on it for now. I'm not sure we know 100%\nyet what \"new\"-style hashes will look like, nor how their loose-object\nfilenames would look.\n\n-Peff\n"},{"id":"331819","messageId":"abf7c219-880b-790f-78ef-18c7cdd54ad6@web.de","threadId":"47082","inReplyTo":"20171101221541.6dwezd4t4hweuis2@sigill.intra.peff.net","subject":"Re: [PATCH 2/3] http-push: use hex_to_bytes()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-11-04T09:05:37Z","receivedAt":"2017-11-04T09:06:08Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 01.11.2017 um 23:15 schrieb Jeff King:\n> On Wed, Nov 01, 2017 at 10:59:49PM +0100, René Scharfe wrote:\n> \n>>> The hex_to_bytes() function requires that the caller make sure they have\n>>> the right number of bytes. But for many callers, I think they'd want to\n>>> say \"parse this oid, which might be truncated; I can't tell what the\n>>> length is supposed to be\".\n>>\n>> I'm confused by the word \"many\".  After this series there are three\n>> callers of hex_to_bytes() and I don't expect that number to grow.\n> \n> I meant only that most callers that parse oids, both in-file and not,\n> would want to stop knowing about the length ahead of time.\n\nThat's a good idea.\n\n> I'm not sure we know 100%\n> yet what \"new\"-style hashes will look like, nor how their loose-object\n> filenames would look.\n\nSo it's too early to implement a solution, but here's how a patch for\nreducing dependency on hash lengths could look like today:\n\n---\n cache.h     |  2 ++\n hex.c       | 13 +++++++++++++\n http-push.c |  7 +++----\n notes.c     |  6 +-----\n sha1_file.c |  4 +---\n 5 files changed, 20 insertions(+), 12 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex f06bfbaf32..acd3804c21 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1317,6 +1317,8 @@ extern int set_disambiguate_hint_config(const char *var, const char *value);\n extern int get_sha1_hex(const char *hex, unsigned char *sha1);\n extern int get_oid_hex(const char *hex, struct object_id *sha1);\n \n+extern int get_oid_hex_tail(const char *hex, struct object_id *oid, size_t offset);\n+\n /*\n  * Read `len` pairs of hexadecimal digits from `hex` and write the\n  * values to `binary` as `len` bytes. Return 0 on success, or -1 if\ndiff --git a/hex.c b/hex.c\nindex 8df2d63728..3e6abe4d5e 100644\n--- a/hex.c\n+++ b/hex.c\n@@ -47,6 +47,19 @@ int hex_to_bytes(unsigned char *binary, const char *hex, size_t len)\n \treturn 0;\n }\n \n+int get_oid_hex_tail(const char *hex, struct object_id *oid, size_t offset)\n+{\n+\tfor (; offset < GIT_SHA1_RAWSZ; offset++, hex += 2) {\n+\t\tint val = hex2chr(hex);\n+\t\tif (val < 0)\n+\t\t\treturn -1;\n+\t\toid->hash[offset] = val;\n+\t}\n+\tif (*hex)\n+\t\treturn -1;\n+\treturn 0;\n+}\n+\n int get_sha1_hex(const char *hex, unsigned char *sha1)\n {\n \tint i;\ndiff --git a/http-push.c b/http-push.c\nindex 14435ab65d..a5512616b9 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1007,18 +1007,17 @@ static void remote_ls(const char *path, int flags,\n \t\t      void (*userFunc)(struct remote_ls_ctx *ls),\n \t\t      void *userData);\n \n-/* extract hex from sharded \"xx/x{38}\" filename */\n+/* extract hex from sharded \"xx/x{N}\" filename */\n static int get_oid_hex_from_objpath(const char *path, struct object_id *oid)\n {\n-\tif (strlen(path) != GIT_SHA1_HEXSZ + 1)\n+\tif (strnlen(path, 3) < 3)\n \t\treturn -1;\n-\n \tif (hex_to_bytes(oid->hash, path, 1))\n \t\treturn -1;\n \tpath += 2;\n \tpath++; /* skip '/' */\n \n-\treturn hex_to_bytes(oid->hash + 1, path, GIT_SHA1_RAWSZ - 1);\n+\treturn get_oid_hex_tail(path, oid, 1);\n }\n \n static void process_ls_object(struct remote_ls_ctx *ls)\ndiff --git a/notes.c b/notes.c\nindex 04f8c8613c..ff6ce57022 100644\n--- a/notes.c\n+++ b/notes.c\n@@ -410,17 +410,13 @@ static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,\n \t\tstruct leaf_node *l;\n \t\tsize_t path_len = strlen(entry.path);\n \n-\t\tif (path_len == 2 * (GIT_SHA1_RAWSZ - prefix_len)) {\n+\t\tif (!get_oid_hex_tail(entry.path, &object_oid, prefix_len)) {\n \t\t\t/* This is potentially the remainder of the SHA-1 */\n \n \t\t\tif (!S_ISREG(entry.mode))\n \t\t\t\t/* notes must be blobs */\n \t\t\t\tgoto handle_non_note;\n \n-\t\t\tif (hex_to_bytes(object_oid.hash + prefix_len, entry.path,\n-\t\t\t\t\t GIT_SHA1_RAWSZ - prefix_len))\n-\t\t\t\tgoto handle_non_note; /* entry.path is not a SHA1 */\n-\n \t\t\ttype = PTR_TYPE_NOTE;\n \t\t} else if (path_len == 2) {\n \t\t\t/* This is potentially an internal node */\ndiff --git a/sha1_file.c b/sha1_file.c\nindex a3c32d91d1..0486696b0b 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -1911,9 +1911,7 @@ int for_each_file_in_obj_subdir(unsigned int subdir_nr,\n \t\tstrbuf_setlen(path, baselen);\n \t\tstrbuf_addf(path, \"/%s\", de->d_name);\n \n-\t\tif (strlen(de->d_name) == GIT_SHA1_HEXSZ - 2 &&\n-\t\t    !hex_to_bytes(oid.hash + 1, de->d_name,\n-\t\t\t\t  GIT_SHA1_RAWSZ - 1)) {\n+\t\tif (!get_oid_hex_tail(de->d_name, &oid, 1)) {\n \t\t\tif (obj_cb) {\n \t\t\t\tr = obj_cb(&oid, path->buf, data);\n \t\t\t\tif (r)\n-- \n2.15.0\n"},{"id":"331840","messageId":"20171105025647.GB23275@alpha.vpn.ikke.info","threadId":"47082","inReplyTo":"508b1b3f-6b55-eee8-110a-c17d572ec27a@web.de","subject":"Re: [PATCH 1/3] notes: move hex_to_bytes() to hex.c and export it","fromName":"Kevin Daudt","fromEmail":"me@ikke.info","sentAt":"2017-11-05T02:56:47Z","receivedAt":"2017-11-05T02:56:57Z","isPatch":true,"sender":{"key":"me@ikke.info","avatar":"https://avatars.githubusercontent.com/u/135698?v=4"},"body":"On Tue, Oct 31, 2017 at 02:46:49PM +0100, René Scharfe wrote:\n> Make the function for converting pairs of hexadecimal digits to binary\n> available to other call sites.\n> \n> Signed-off-by: Rene Scharfe <l.s.r@web.de>\n> ---\n>  cache.h |  7 +++++++\n>  hex.c   | 12 ++++++++++++\n>  notes.c | 17 -----------------\n>  3 files changed, 19 insertions(+), 17 deletions(-)\n> \n> diff --git a/cache.h b/cache.h\n> index 6440e2bf21..f06bfbaf32 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -1317,6 +1317,13 @@ extern int set_disambiguate_hint_config(const char *var, const char *value);\n>  extern int get_sha1_hex(const char *hex, unsigned char *sha1);\n>  extern int get_oid_hex(const char *hex, struct object_id *sha1);\n>  \n> +/*\n> + * Read `len` pairs of hexadecimal digits from `hex` and write the\n> + * values to `binary` as `len` bytes. Return 0 on success, or -1 if\n\nIs it correct to call the result binary? I would say that it's the value\nthat gets stored. To me, this value does not really have a base.\n\nKevin\n"},{"id":"331872","messageId":"71257adb-56c0-2e13-c0b1-a7ee4b084207@web.de","threadId":"47082","inReplyTo":"20171105025647.GB23275@alpha.vpn.ikke.info","subject":"Re: [PATCH 1/3] notes: move hex_to_bytes() to hex.c and export it","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-11-05T16:47:47Z","receivedAt":"2017-11-05T16:48:16Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 05.11.2017 um 03:56 schrieb Kevin Daudt:\n> On Tue, Oct 31, 2017 at 02:46:49PM +0100, René Scharfe wrote:\n>> Make the function for converting pairs of hexadecimal digits to binary\n>> available to other call sites.\n>>\n>> Signed-off-by: Rene Scharfe <l.s.r@web.de>\n>> ---\n>>   cache.h |  7 +++++++\n>>   hex.c   | 12 ++++++++++++\n>>   notes.c | 17 -----------------\n>>   3 files changed, 19 insertions(+), 17 deletions(-)\n>>\n>> diff --git a/cache.h b/cache.h\n>> index 6440e2bf21..f06bfbaf32 100644\n>> --- a/cache.h\n>> +++ b/cache.h\n>> @@ -1317,6 +1317,13 @@ extern int set_disambiguate_hint_config(const char *var, const char *value);\n>>   extern int get_sha1_hex(const char *hex, unsigned char *sha1);\n>>   extern int get_oid_hex(const char *hex, struct object_id *sha1);\n>>   \n>> +/*\n>> + * Read `len` pairs of hexadecimal digits from `hex` and write the\n>> + * values to `binary` as `len` bytes. Return 0 on success, or -1 if\n> \n> Is it correct to call the result binary? I would say that it's the value\n> that gets stored. To me, this value does not really have a base.\n\nHere's the full context:\n\n  /*\n   * Read `len` pairs of hexadecimal digits from `hex` and write the\n   * values to `binary` as `len` bytes. Return 0 on success, or -1 if\n   * the input does not consist of hex digits).\n   */\n  extern int hex_to_bytes(unsigned char *binary, const char *hex, size_t len);\n\nThe patch moves the comment verbatim.  Words in backticks (`binary`,\n`hex`, `len`) are parameter names.\n\nThe function converts pairs of hexadecimal digits (base 16, ASCII\nencoded) to bytes (base 256).  A byte can be seen as an array of bits;\nthus the output is also binary (base 2) without requiring further\nconversion.\n\nCalling the variable \"binary\" may seem unspecific, but makes sense in\nthe context of this function.\n\nDoes any of that help?\n\nThanks,\nRené\n"},{"id":"331873","messageId":"20171105195750.GA7647@alpha.vpn.ikke.info","threadId":"47082","inReplyTo":"71257adb-56c0-2e13-c0b1-a7ee4b084207@web.de","subject":"Re: [PATCH 1/3] notes: move hex_to_bytes() to hex.c and export it","fromName":"Kevin Daudt","fromEmail":"me@ikke.info","sentAt":"2017-11-05T19:57:50Z","receivedAt":"2017-11-05T19:57:56Z","isPatch":true,"sender":{"key":"me@ikke.info","avatar":"https://avatars.githubusercontent.com/u/135698?v=4"},"body":"On Sun, Nov 05, 2017 at 05:47:47PM +0100, René Scharfe wrote:\n> Am 05.11.2017 um 03:56 schrieb Kevin Daudt:\n> > On Tue, Oct 31, 2017 at 02:46:49PM +0100, René Scharfe wrote:\n> >> Make the function for converting pairs of hexadecimal digits to binary\n> >> available to other call sites.\n> >>\n> >> Signed-off-by: Rene Scharfe <l.s.r@web.de>\n> >> ---\n> >>   cache.h |  7 +++++++\n> >>   hex.c   | 12 ++++++++++++\n> >>   notes.c | 17 -----------------\n> >>   3 files changed, 19 insertions(+), 17 deletions(-)\n> >>\n> >> diff --git a/cache.h b/cache.h\n> >> index 6440e2bf21..f06bfbaf32 100644\n> >> --- a/cache.h\n> >> +++ b/cache.h\n> >> @@ -1317,6 +1317,13 @@ extern int set_disambiguate_hint_config(const char *var, const char *value);\n> >>   extern int get_sha1_hex(const char *hex, unsigned char *sha1);\n> >>   extern int get_oid_hex(const char *hex, struct object_id *sha1);\n> >>   \n> >> +/*\n> >> + * Read `len` pairs of hexadecimal digits from `hex` and write the\n> >> + * values to `binary` as `len` bytes. Return 0 on success, or -1 if\n> > \n> > Is it correct to call the result binary? I would say that it's the value\n> > that gets stored. To me, this value does not really have a base.\n> \n> Here's the full context:\n> \n>   /*\n>    * Read `len` pairs of hexadecimal digits from `hex` and write the\n>    * values to `binary` as `len` bytes. Return 0 on success, or -1 if\n>    * the input does not consist of hex digits).\n>    */\n>   extern int hex_to_bytes(unsigned char *binary, const char *hex, size_t len);\n> \n> The patch moves the comment verbatim.  Words in backticks (`binary`,\n> `hex`, `len`) are parameter names.\n> \n> The function converts pairs of hexadecimal digits (base 16, ASCII\n> encoded) to bytes (base 256).  A byte can be seen as an array of bits;\n> thus the output is also binary (base 2) without requiring further\n> conversion.\n> \n> Calling the variable \"binary\" may seem unspecific, but makes sense in\n> the context of this function.\n> \n> Does any of that help?\n> \n> Thanks,\n> René\n\nThanks, I have been thinking about it more, and I agree, it does make\nsense. \n\nI had a binary representation in mind, but this is refering to binary\ndata (just like you can have binary files).\n\nKevin\n"}]}