* [PATCH 1/3] notes: move hex_to_bytes() to hex.c and export it @ 2017-10-31 13:46 René Scharfe 2017-10-31 13:49 ` [PATCH 2/3] http-push: use hex_to_bytes() René Scharfe ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: René Scharfe @ 2017-10-31 13:46 UTC (permalink / raw) To: Git List Cc: Junio C Hamano, Jeff King, brian m. carlson, Michael Haggerty, Thomas Gummerer Make the function for converting pairs of hexadecimal digits to binary available to other call sites. Signed-off-by: Rene Scharfe <l.s.r@web.de> --- cache.h | 7 +++++++ hex.c | 12 ++++++++++++ notes.c | 17 ----------------- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/cache.h b/cache.h index 6440e2bf21..f06bfbaf32 100644 --- a/cache.h +++ b/cache.h @@ -1317,6 +1317,13 @@ extern int set_disambiguate_hint_config(const char *var, const char *value); extern int get_sha1_hex(const char *hex, unsigned char *sha1); extern int get_oid_hex(const char *hex, struct object_id *sha1); +/* + * Read `len` pairs of hexadecimal digits from `hex` and write the + * values to `binary` as `len` bytes. Return 0 on success, or -1 if + * the input does not consist of hex digits). + */ +extern int hex_to_bytes(unsigned char *binary, const char *hex, size_t len); + /* * Convert a binary sha1 to its hex equivalent. The `_r` variant is reentrant, * and writes the NUL-terminated output to the buffer `out`, which must be at diff --git a/hex.c b/hex.c index 28b44118cb..8df2d63728 100644 --- a/hex.c +++ b/hex.c @@ -35,6 +35,18 @@ const signed char hexval_table[256] = { -1, -1, -1, -1, -1, -1, -1, -1, /* f8-ff */ }; +int hex_to_bytes(unsigned char *binary, const char *hex, size_t len) +{ + for (; len; len--, hex += 2) { + unsigned int val = (hexval(hex[0]) << 4) | hexval(hex[1]); + + if (val & ~0xff) + return -1; + *binary++ = val; + } + return 0; +} + int get_sha1_hex(const char *hex, unsigned char *sha1) { int i; diff --git a/notes.c b/notes.c index 5c62862574..04f8c8613c 100644 --- a/notes.c +++ b/notes.c @@ -334,23 +334,6 @@ static void note_tree_free(struct int_node *tree) } } -/* - * Read `len` pairs of hexadecimal digits from `hex` and write the - * values to `binary` as `len` bytes. Return 0 on success, or -1 if - * the input does not consist of hex digits). - */ -static int hex_to_bytes(unsigned char *binary, const char *hex, size_t len) -{ - for (; len; len--, hex += 2) { - unsigned int val = (hexval(hex[0]) << 4) | hexval(hex[1]); - - if (val & ~0xff) - return -1; - *binary++ = val; - } - return 0; -} - static int non_note_cmp(const struct non_note *a, const struct non_note *b) { return strcmp(a->path, b->path); -- 2.15.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] http-push: use hex_to_bytes() 2017-10-31 13:46 [PATCH 1/3] notes: move hex_to_bytes() to hex.c and export it René Scharfe @ 2017-10-31 13:49 ` René Scharfe 2017-11-01 19:55 ` Jeff King 2017-10-31 13:50 ` [PATCH 3/3] sha1_file: " René Scharfe 2017-11-05 2:56 ` [PATCH 1/3] notes: move hex_to_bytes() to hex.c and export it Kevin Daudt 2 siblings, 1 reply; 11+ messages in thread From: René Scharfe @ 2017-10-31 13:49 UTC (permalink / raw) To: Git List Cc: Junio C Hamano, Jeff King, brian m. carlson, Michael Haggerty, Thomas Gummerer The path of a loose object contains its hash value encoded into two substrings of hexadecimal digits, separated by a slash. The current code copies the pieces into a temporary buffer to get rid of the slash and then uses get_oid_hex() to decode the hash value. Avoid the copy by using hex_to_bytes() directly on the substrings. That's shorter and easier. While at it correct the length of the second substring in a comment. Signed-off-by: Rene Scharfe <l.s.r@web.de> --- http-push.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/http-push.c b/http-push.c index 493ee7d719..14435ab65d 100644 --- a/http-push.c +++ b/http-push.c @@ -1007,20 +1007,18 @@ static void remote_ls(const char *path, int flags, void (*userFunc)(struct remote_ls_ctx *ls), void *userData); -/* extract hex from sharded "xx/x{40}" filename */ +/* extract hex from sharded "xx/x{38}" filename */ static int get_oid_hex_from_objpath(const char *path, struct object_id *oid) { - char hex[GIT_MAX_HEXSZ]; - if (strlen(path) != GIT_SHA1_HEXSZ + 1) return -1; - memcpy(hex, path, 2); + if (hex_to_bytes(oid->hash, path, 1)) + return -1; path += 2; path++; /* skip '/' */ - memcpy(hex + 2, path, GIT_SHA1_HEXSZ - 2); - return get_oid_hex(hex, oid); + return hex_to_bytes(oid->hash + 1, path, GIT_SHA1_RAWSZ - 1); } static void process_ls_object(struct remote_ls_ctx *ls) -- 2.15.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] http-push: use hex_to_bytes() 2017-10-31 13:49 ` [PATCH 2/3] http-push: use hex_to_bytes() René Scharfe @ 2017-11-01 19:55 ` Jeff King 2017-11-01 21:59 ` René Scharfe 0 siblings, 1 reply; 11+ messages in thread From: Jeff King @ 2017-11-01 19:55 UTC (permalink / raw) To: René Scharfe Cc: Git List, Junio C Hamano, brian m. carlson, Michael Haggerty, Thomas Gummerer On Tue, Oct 31, 2017 at 02:49:56PM +0100, René Scharfe wrote: > The path of a loose object contains its hash value encoded into two > substrings of hexadecimal digits, separated by a slash. The current > code copies the pieces into a temporary buffer to get rid of the slash > and then uses get_oid_hex() to decode the hash value. > > Avoid the copy by using hex_to_bytes() directly on the substrings. > That's shorter and easier. > > While at it correct the length of the second substring in a comment. I think the patch is correct, but I wonder if this approach will bite us later on when we start dealing with multiple lengths of hashes. The hex_to_bytes() function requires that the caller make sure they have the right number of bytes. But for many callers, I think they'd want to say "parse this oid, which might be truncated; I can't tell what the length is supposed to be". I.e., I wonder if the right primitive is really something like parse_oid_hex(), but with a flag to say "skip over interior slashes". We don't need to deal with that eventuality yet, but I'm on the fence on whether this patch is making that harder down the road or not. The current strategy of "stuff it into a buffer without slashes" would be easier to convert, I think. -Peff ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] http-push: use hex_to_bytes() 2017-11-01 19:55 ` Jeff King @ 2017-11-01 21:59 ` René Scharfe 2017-11-01 22:15 ` Jeff King 0 siblings, 1 reply; 11+ messages in thread From: René Scharfe @ 2017-11-01 21:59 UTC (permalink / raw) To: Jeff King Cc: Git List, Junio C Hamano, brian m. carlson, Michael Haggerty, Thomas Gummerer Am 01.11.2017 um 20:55 schrieb Jeff King: > On Tue, Oct 31, 2017 at 02:49:56PM +0100, René Scharfe wrote: > >> The path of a loose object contains its hash value encoded into two >> substrings of hexadecimal digits, separated by a slash. The current >> code copies the pieces into a temporary buffer to get rid of the slash >> and then uses get_oid_hex() to decode the hash value. >> >> Avoid the copy by using hex_to_bytes() directly on the substrings. >> That's shorter and easier. >> >> While at it correct the length of the second substring in a comment. > > I think the patch is correct, but I wonder if this approach will bite us > later on when we start dealing with multiple lengths of hashes. > > The hex_to_bytes() function requires that the caller make sure they have > the right number of bytes. But for many callers, I think they'd want to > say "parse this oid, which might be truncated; I can't tell what the > length is supposed to be". I'm confused by the word "many". After this series there are three callers of hex_to_bytes() and I don't expect that number to grow. Would loose objects be stored at paths containing only a subset of their new hash value? If they won't then there will be two acceptable lengths instead of the one we have today, which should be easy to handle. > I.e., I wonder if the right primitive is really something like > parse_oid_hex(), but with a flag to say "skip over interior slashes". Hmm, ignoring any slashes is an interesting idea. Is that too loose, I wonder? And I don't think it makes for a good primitive, as slashes only need to be ignored in a single place so far (here in http-push.c). Collecting special cases in a central place, guarded by flags, doesn't reduce the overall complexity. > We don't need to deal with that eventuality yet, but I'm on the fence on > whether this patch is making that harder down the road or not. The > current strategy of "stuff it into a buffer without slashes" would be > easier to convert, I think. How so? If you have a buffer then you need to know the size of the data to copy into it as well, or you'll learn it in the process. The call sites of hex_to_bytes() have to be modified along with the functions in hex.c to support longer hashes, with or without this series. René ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] http-push: use hex_to_bytes() 2017-11-01 21:59 ` René Scharfe @ 2017-11-01 22:15 ` Jeff King 2017-11-04 9:05 ` René Scharfe 0 siblings, 1 reply; 11+ messages in thread From: Jeff King @ 2017-11-01 22:15 UTC (permalink / raw) To: René Scharfe Cc: Git List, Junio C Hamano, brian m. carlson, Michael Haggerty, Thomas Gummerer On Wed, Nov 01, 2017 at 10:59:49PM +0100, René Scharfe wrote: > > The hex_to_bytes() function requires that the caller make sure they have > > the right number of bytes. But for many callers, I think they'd want to > > say "parse this oid, which might be truncated; I can't tell what the > > length is supposed to be". > > I'm confused by the word "many". After this series there are three > callers of hex_to_bytes() and I don't expect that number to grow. I meant only that most callers that parse oids, both in-file and not, would want to stop knowing about the length ahead of time. I think parse_oid_hex() solves that problem for most callers. > Would loose objects be stored at paths containing only a subset of their > new hash value? If they won't then there will be two acceptable lengths > instead of the one we have today, which should be easy to handle. I don't know. TBH, I'm not sure anyone has much interest in making http-push work with new hashes. I'd be OK if it simply doesn't until somebody interested shows up to change that. > > We don't need to deal with that eventuality yet, but I'm on the fence on > > whether this patch is making that harder down the road or not. The > > current strategy of "stuff it into a buffer without slashes" would be > > easier to convert, I think. > > How so? If you have a buffer then you need to know the size of the > data to copy into it as well, or you'll learn it in the process. > > The call sites of hex_to_bytes() have to be modified along with the > functions in hex.c to support longer hashes, with or without this > series. You have to know how big the data you have is, but you don't necessarily know whether that makes a complete hash or not. With a "remove slashes and then parse" strategy, you can do the removing without worrying about how big things are _supposed_ to be, and then the parser can tell you if you have a valid oid or not. The logic for what a hash looks like _and_ how big it must be are both in the parser. With the new code you have here, we have to be a bit more intimate with SHA1_HEXSZ in the calling code. It knows that the hash consists of a certain number of hex bytes. I'm perfectly willing to punt on it for now. I'm not sure we know 100% yet what "new"-style hashes will look like, nor how their loose-object filenames would look. -Peff ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] http-push: use hex_to_bytes() 2017-11-01 22:15 ` Jeff King @ 2017-11-04 9:05 ` René Scharfe 0 siblings, 0 replies; 11+ messages in thread From: René Scharfe @ 2017-11-04 9:05 UTC (permalink / raw) To: Jeff King Cc: Git List, Junio C Hamano, brian m. carlson, Michael Haggerty, Thomas Gummerer Am 01.11.2017 um 23:15 schrieb Jeff King: > On Wed, Nov 01, 2017 at 10:59:49PM +0100, René Scharfe wrote: > >>> The hex_to_bytes() function requires that the caller make sure they have >>> the right number of bytes. But for many callers, I think they'd want to >>> say "parse this oid, which might be truncated; I can't tell what the >>> length is supposed to be". >> >> I'm confused by the word "many". After this series there are three >> callers of hex_to_bytes() and I don't expect that number to grow. > > I meant only that most callers that parse oids, both in-file and not, > would want to stop knowing about the length ahead of time. That's a good idea. > I'm not sure we know 100% > yet what "new"-style hashes will look like, nor how their loose-object > filenames would look. So it's too early to implement a solution, but here's how a patch for reducing dependency on hash lengths could look like today: --- cache.h | 2 ++ hex.c | 13 +++++++++++++ http-push.c | 7 +++---- notes.c | 6 +----- sha1_file.c | 4 +--- 5 files changed, 20 insertions(+), 12 deletions(-) diff --git a/cache.h b/cache.h index f06bfbaf32..acd3804c21 100644 --- a/cache.h +++ b/cache.h @@ -1317,6 +1317,8 @@ extern int set_disambiguate_hint_config(const char *var, const char *value); extern int get_sha1_hex(const char *hex, unsigned char *sha1); extern int get_oid_hex(const char *hex, struct object_id *sha1); +extern int get_oid_hex_tail(const char *hex, struct object_id *oid, size_t offset); + /* * Read `len` pairs of hexadecimal digits from `hex` and write the * values to `binary` as `len` bytes. Return 0 on success, or -1 if diff --git a/hex.c b/hex.c index 8df2d63728..3e6abe4d5e 100644 --- a/hex.c +++ b/hex.c @@ -47,6 +47,19 @@ int hex_to_bytes(unsigned char *binary, const char *hex, size_t len) return 0; } +int get_oid_hex_tail(const char *hex, struct object_id *oid, size_t offset) +{ + for (; offset < GIT_SHA1_RAWSZ; offset++, hex += 2) { + int val = hex2chr(hex); + if (val < 0) + return -1; + oid->hash[offset] = val; + } + if (*hex) + return -1; + return 0; +} + int get_sha1_hex(const char *hex, unsigned char *sha1) { int i; diff --git a/http-push.c b/http-push.c index 14435ab65d..a5512616b9 100644 --- a/http-push.c +++ b/http-push.c @@ -1007,18 +1007,17 @@ static void remote_ls(const char *path, int flags, void (*userFunc)(struct remote_ls_ctx *ls), void *userData); -/* extract hex from sharded "xx/x{38}" filename */ +/* extract hex from sharded "xx/x{N}" filename */ static int get_oid_hex_from_objpath(const char *path, struct object_id *oid) { - if (strlen(path) != GIT_SHA1_HEXSZ + 1) + if (strnlen(path, 3) < 3) return -1; - if (hex_to_bytes(oid->hash, path, 1)) return -1; path += 2; path++; /* skip '/' */ - return hex_to_bytes(oid->hash + 1, path, GIT_SHA1_RAWSZ - 1); + return get_oid_hex_tail(path, oid, 1); } static void process_ls_object(struct remote_ls_ctx *ls) diff --git a/notes.c b/notes.c index 04f8c8613c..ff6ce57022 100644 --- a/notes.c +++ b/notes.c @@ -410,17 +410,13 @@ static void load_subtree(struct notes_tree *t, struct leaf_node *subtree, struct leaf_node *l; size_t path_len = strlen(entry.path); - if (path_len == 2 * (GIT_SHA1_RAWSZ - prefix_len)) { + if (!get_oid_hex_tail(entry.path, &object_oid, prefix_len)) { /* This is potentially the remainder of the SHA-1 */ if (!S_ISREG(entry.mode)) /* notes must be blobs */ goto handle_non_note; - if (hex_to_bytes(object_oid.hash + prefix_len, entry.path, - GIT_SHA1_RAWSZ - prefix_len)) - goto handle_non_note; /* entry.path is not a SHA1 */ - type = PTR_TYPE_NOTE; } else if (path_len == 2) { /* This is potentially an internal node */ diff --git a/sha1_file.c b/sha1_file.c index a3c32d91d1..0486696b0b 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1911,9 +1911,7 @@ int for_each_file_in_obj_subdir(unsigned int subdir_nr, strbuf_setlen(path, baselen); strbuf_addf(path, "/%s", de->d_name); - if (strlen(de->d_name) == GIT_SHA1_HEXSZ - 2 && - !hex_to_bytes(oid.hash + 1, de->d_name, - GIT_SHA1_RAWSZ - 1)) { + if (!get_oid_hex_tail(de->d_name, &oid, 1)) { if (obj_cb) { r = obj_cb(&oid, path->buf, data); if (r) -- 2.15.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] sha1_file: use hex_to_bytes() 2017-10-31 13:46 [PATCH 1/3] notes: move hex_to_bytes() to hex.c and export it René Scharfe 2017-10-31 13:49 ` [PATCH 2/3] http-push: use hex_to_bytes() René Scharfe @ 2017-10-31 13:50 ` René Scharfe 2017-11-01 19:58 ` Jeff King 2017-11-05 2:56 ` [PATCH 1/3] notes: move hex_to_bytes() to hex.c and export it Kevin Daudt 2 siblings, 1 reply; 11+ messages in thread From: René Scharfe @ 2017-10-31 13:50 UTC (permalink / raw) To: Git List Cc: Junio C Hamano, Jeff King, brian m. carlson, Michael Haggerty, Thomas Gummerer The path of a loose object contains its hash value encoded into two substrings of 2 and 38 hexadecimal digits separated by a slash. The first part is handed to for_each_file_in_obj_subdir() in decoded form as subdir_nr. The current code builds a full hexadecimal representation of the hash in a temporary buffer, then uses get_oid_hex() to decode it. Avoid the intermediate step by taking subdir_nr as-is and using hex_to_bytes() directly on the second substring. That's shorter and easier. Signed-off-by: Rene Scharfe <l.s.r@web.de> --- sha1_file.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/sha1_file.c b/sha1_file.c index 10c3a0083d..a3c32d91d1 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1884,6 +1884,7 @@ int for_each_file_in_obj_subdir(unsigned int subdir_nr, DIR *dir; struct dirent *de; int r = 0; + struct object_id oid; if (subdir_nr > 0xff) BUG("invalid loose object subdirectory: %x", subdir_nr); @@ -1901,6 +1902,8 @@ int for_each_file_in_obj_subdir(unsigned int subdir_nr, return r; } + oid.hash[0] = subdir_nr; + while ((de = readdir(dir))) { if (is_dot_or_dotdot(de->d_name)) continue; @@ -1908,20 +1911,15 @@ int for_each_file_in_obj_subdir(unsigned int subdir_nr, strbuf_setlen(path, baselen); strbuf_addf(path, "/%s", de->d_name); - if (strlen(de->d_name) == GIT_SHA1_HEXSZ - 2) { - char hex[GIT_MAX_HEXSZ+1]; - struct object_id oid; - - xsnprintf(hex, sizeof(hex), "%02x%s", - subdir_nr, de->d_name); - if (!get_oid_hex(hex, &oid)) { - if (obj_cb) { - r = obj_cb(&oid, path->buf, data); - if (r) - break; - } - continue; + if (strlen(de->d_name) == GIT_SHA1_HEXSZ - 2 && + !hex_to_bytes(oid.hash + 1, de->d_name, + GIT_SHA1_RAWSZ - 1)) { + if (obj_cb) { + r = obj_cb(&oid, path->buf, data); + if (r) + break; } + continue; } if (cruft_cb) { -- 2.15.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] sha1_file: use hex_to_bytes() 2017-10-31 13:50 ` [PATCH 3/3] sha1_file: " René Scharfe @ 2017-11-01 19:58 ` Jeff King 0 siblings, 0 replies; 11+ messages in thread From: Jeff King @ 2017-11-01 19:58 UTC (permalink / raw) To: René Scharfe Cc: Git List, Junio C Hamano, brian m. carlson, Michael Haggerty, Thomas Gummerer On Tue, Oct 31, 2017 at 02:50:06PM +0100, René Scharfe wrote: > The path of a loose object contains its hash value encoded into two > substrings of 2 and 38 hexadecimal digits separated by a slash. The > first part is handed to for_each_file_in_obj_subdir() in decoded form as > subdir_nr. The current code builds a full hexadecimal representation of > the hash in a temporary buffer, then uses get_oid_hex() to decode it. > > Avoid the intermediate step by taking subdir_nr as-is and using > hex_to_bytes() directly on the second substring. That's shorter and > easier. This raises some of the same questions as the previous one on whether hex_to_bytes() is the ideal abstraction. But as before, I'm on the fence. > @@ -1908,20 +1911,15 @@ int for_each_file_in_obj_subdir(unsigned int subdir_nr, > strbuf_setlen(path, baselen); > strbuf_addf(path, "/%s", de->d_name); > > - if (strlen(de->d_name) == GIT_SHA1_HEXSZ - 2) { > - char hex[GIT_MAX_HEXSZ+1]; > - struct object_id oid; > - > - xsnprintf(hex, sizeof(hex), "%02x%s", > - subdir_nr, de->d_name); > - if (!get_oid_hex(hex, &oid)) { > - if (obj_cb) { > - r = obj_cb(&oid, path->buf, data); > - if (r) > - break; > - } > - continue; > + if (strlen(de->d_name) == GIT_SHA1_HEXSZ - 2 && > + !hex_to_bytes(oid.hash + 1, de->d_name, > + GIT_SHA1_RAWSZ - 1)) { > + if (obj_cb) { > + r = obj_cb(&oid, path->buf, data); > + if (r) > + break; > } > + continue; > } > > if (cruft_cb) { Now that this is one big conditional for "is this a valid object filename", I think we could get rid of the "continue" in favor of: if (...looks like an object...) ...call obj_cb... else if (cruft_cb) ...call cruft_cb... Not a big deal, but it may make the flow more clear (the original had to use a continue because there were multiple independent steps to determining it was an object file, so we had to "break out" from the inner conditional). -Peff ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] notes: move hex_to_bytes() to hex.c and export it 2017-10-31 13:46 [PATCH 1/3] notes: move hex_to_bytes() to hex.c and export it René Scharfe 2017-10-31 13:49 ` [PATCH 2/3] http-push: use hex_to_bytes() René Scharfe 2017-10-31 13:50 ` [PATCH 3/3] sha1_file: " René Scharfe @ 2017-11-05 2:56 ` Kevin Daudt 2017-11-05 16:47 ` René Scharfe 2 siblings, 1 reply; 11+ messages in thread From: Kevin Daudt @ 2017-11-05 2:56 UTC (permalink / raw) To: René Scharfe Cc: Git List, Junio C Hamano, Jeff King, brian m. carlson, Michael Haggerty, Thomas Gummerer On Tue, Oct 31, 2017 at 02:46:49PM +0100, René Scharfe wrote: > Make the function for converting pairs of hexadecimal digits to binary > available to other call sites. > > Signed-off-by: Rene Scharfe <l.s.r@web.de> > --- > cache.h | 7 +++++++ > hex.c | 12 ++++++++++++ > notes.c | 17 ----------------- > 3 files changed, 19 insertions(+), 17 deletions(-) > > diff --git a/cache.h b/cache.h > index 6440e2bf21..f06bfbaf32 100644 > --- a/cache.h > +++ b/cache.h > @@ -1317,6 +1317,13 @@ extern int set_disambiguate_hint_config(const char *var, const char *value); > extern int get_sha1_hex(const char *hex, unsigned char *sha1); > extern int get_oid_hex(const char *hex, struct object_id *sha1); > > +/* > + * Read `len` pairs of hexadecimal digits from `hex` and write the > + * values to `binary` as `len` bytes. Return 0 on success, or -1 if Is it correct to call the result binary? I would say that it's the value that gets stored. To me, this value does not really have a base. Kevin ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] notes: move hex_to_bytes() to hex.c and export it 2017-11-05 2:56 ` [PATCH 1/3] notes: move hex_to_bytes() to hex.c and export it Kevin Daudt @ 2017-11-05 16:47 ` René Scharfe 2017-11-05 19:57 ` Kevin Daudt 0 siblings, 1 reply; 11+ messages in thread From: René Scharfe @ 2017-11-05 16:47 UTC (permalink / raw) To: Kevin Daudt Cc: Git List, Junio C Hamano, Jeff King, brian m. carlson, Michael Haggerty, Thomas Gummerer Am 05.11.2017 um 03:56 schrieb Kevin Daudt: > On Tue, Oct 31, 2017 at 02:46:49PM +0100, René Scharfe wrote: >> Make the function for converting pairs of hexadecimal digits to binary >> available to other call sites. >> >> Signed-off-by: Rene Scharfe <l.s.r@web.de> >> --- >> cache.h | 7 +++++++ >> hex.c | 12 ++++++++++++ >> notes.c | 17 ----------------- >> 3 files changed, 19 insertions(+), 17 deletions(-) >> >> diff --git a/cache.h b/cache.h >> index 6440e2bf21..f06bfbaf32 100644 >> --- a/cache.h >> +++ b/cache.h >> @@ -1317,6 +1317,13 @@ extern int set_disambiguate_hint_config(const char *var, const char *value); >> extern int get_sha1_hex(const char *hex, unsigned char *sha1); >> extern int get_oid_hex(const char *hex, struct object_id *sha1); >> >> +/* >> + * Read `len` pairs of hexadecimal digits from `hex` and write the >> + * values to `binary` as `len` bytes. Return 0 on success, or -1 if > > Is it correct to call the result binary? I would say that it's the value > that gets stored. To me, this value does not really have a base. Here's the full context: /* * Read `len` pairs of hexadecimal digits from `hex` and write the * values to `binary` as `len` bytes. Return 0 on success, or -1 if * the input does not consist of hex digits). */ extern int hex_to_bytes(unsigned char *binary, const char *hex, size_t len); The patch moves the comment verbatim. Words in backticks (`binary`, `hex`, `len`) are parameter names. The function converts pairs of hexadecimal digits (base 16, ASCII encoded) to bytes (base 256). A byte can be seen as an array of bits; thus the output is also binary (base 2) without requiring further conversion. Calling the variable "binary" may seem unspecific, but makes sense in the context of this function. Does any of that help? Thanks, René ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] notes: move hex_to_bytes() to hex.c and export it 2017-11-05 16:47 ` René Scharfe @ 2017-11-05 19:57 ` Kevin Daudt 0 siblings, 0 replies; 11+ messages in thread From: Kevin Daudt @ 2017-11-05 19:57 UTC (permalink / raw) To: René Scharfe Cc: Git List, Junio C Hamano, Jeff King, brian m. carlson, Michael Haggerty, Thomas Gummerer On Sun, Nov 05, 2017 at 05:47:47PM +0100, René Scharfe wrote: > Am 05.11.2017 um 03:56 schrieb Kevin Daudt: > > On Tue, Oct 31, 2017 at 02:46:49PM +0100, René Scharfe wrote: > >> Make the function for converting pairs of hexadecimal digits to binary > >> available to other call sites. > >> > >> Signed-off-by: Rene Scharfe <l.s.r@web.de> > >> --- > >> cache.h | 7 +++++++ > >> hex.c | 12 ++++++++++++ > >> notes.c | 17 ----------------- > >> 3 files changed, 19 insertions(+), 17 deletions(-) > >> > >> diff --git a/cache.h b/cache.h > >> index 6440e2bf21..f06bfbaf32 100644 > >> --- a/cache.h > >> +++ b/cache.h > >> @@ -1317,6 +1317,13 @@ extern int set_disambiguate_hint_config(const char *var, const char *value); > >> extern int get_sha1_hex(const char *hex, unsigned char *sha1); > >> extern int get_oid_hex(const char *hex, struct object_id *sha1); > >> > >> +/* > >> + * Read `len` pairs of hexadecimal digits from `hex` and write the > >> + * values to `binary` as `len` bytes. Return 0 on success, or -1 if > > > > Is it correct to call the result binary? I would say that it's the value > > that gets stored. To me, this value does not really have a base. > > Here's the full context: > > /* > * Read `len` pairs of hexadecimal digits from `hex` and write the > * values to `binary` as `len` bytes. Return 0 on success, or -1 if > * the input does not consist of hex digits). > */ > extern int hex_to_bytes(unsigned char *binary, const char *hex, size_t len); > > The patch moves the comment verbatim. Words in backticks (`binary`, > `hex`, `len`) are parameter names. > > The function converts pairs of hexadecimal digits (base 16, ASCII > encoded) to bytes (base 256). A byte can be seen as an array of bits; > thus the output is also binary (base 2) without requiring further > conversion. > > Calling the variable "binary" may seem unspecific, but makes sense in > the context of this function. > > Does any of that help? > > Thanks, > René Thanks, I have been thinking about it more, and I agree, it does make sense. I had a binary representation in mind, but this is refering to binary data (just like you can have binary files). Kevin ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-11-05 19:57 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-10-31 13:46 [PATCH 1/3] notes: move hex_to_bytes() to hex.c and export it René Scharfe 2017-10-31 13:49 ` [PATCH 2/3] http-push: use hex_to_bytes() René Scharfe 2017-11-01 19:55 ` Jeff King 2017-11-01 21:59 ` René Scharfe 2017-11-01 22:15 ` Jeff King 2017-11-04 9:05 ` René Scharfe 2017-10-31 13:50 ` [PATCH 3/3] sha1_file: " René Scharfe 2017-11-01 19:58 ` Jeff King 2017-11-05 2:56 ` [PATCH 1/3] notes: move hex_to_bytes() to hex.c and export it Kevin Daudt 2017-11-05 16:47 ` René Scharfe 2017-11-05 19:57 ` Kevin Daudt
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.