Re: [PATCH 2/2] hex: drop sha1_to_hex()

2 messages, 2 authors, 2019-11-12 · open the first message on its own page

Re: [PATCH 2/2] hex: drop sha1_to_hex()

From: Junio C Hamano <hidden>
Date: 2019-11-12 11:49:51

Jeff King [off-list ref] writes:
quoted
    @@ cache.h: int hex_to_bytes(unsigned char *binary, const char *hex, size_t len);
       * buffers, making it safe to make multiple calls for a single statement, like:
       *
     - *   printf("%s -> %s", sha1_to_hex(one), sha1_to_hex(two));
    -+ *   printf("%s -> %s", oid_to_hex(one), oid_to_hex(two));
    ++ *   printf("%s -> %s", hash_to_hex(one), hash_to_hex(two));
       */
      char *hash_to_hex_algop_r(char *buffer, const unsigned char *hash, const struct git_hash_algo *);
      char *oid_to_hex_r(char *out, const struct object_id *oid);
This one-liner leaves the types of "one" and "two" unspecified. :) So
it's not wrong to use hash_to_hex(), but maybe it's better to be pushing
people towards oid_to_hex() as their first choice? It probably doesn't
matter too much either way.
The pre-context of that comment reads:

 * Convert a binary hash to its hex equivalent. The `_r` variant is reentrant,
 * and writes the NUL-terminated output to the buffer `out`, which must be at
 * least `GIT_MAX_HEXSZ + 1` bytes, and returns a pointer to out for
 * convenience.

so I think the intent of the example that used to use sha1_to_hex()
has been the raw bytestring that is the object name, not its form
that is encapsulated in the "struct object_id()".

Re: [PATCH 2/2] hex: drop sha1_to_hex()

From: Jeff King <hidden>
Date: 2019-11-12 12:15:39

On Tue, Nov 12, 2019 at 08:49:44PM +0900, Junio C Hamano wrote:
Jeff King [off-list ref] writes:
quoted
quoted
    @@ cache.h: int hex_to_bytes(unsigned char *binary, const char *hex, size_t len);
       * buffers, making it safe to make multiple calls for a single statement, like:
       *
     - *   printf("%s -> %s", sha1_to_hex(one), sha1_to_hex(two));
    -+ *   printf("%s -> %s", oid_to_hex(one), oid_to_hex(two));
    ++ *   printf("%s -> %s", hash_to_hex(one), hash_to_hex(two));
       */
      char *hash_to_hex_algop_r(char *buffer, const unsigned char *hash, const struct git_hash_algo *);
      char *oid_to_hex_r(char *out, const struct object_id *oid);
This one-liner leaves the types of "one" and "two" unspecified. :) So
it's not wrong to use hash_to_hex(), but maybe it's better to be pushing
people towards oid_to_hex() as their first choice? It probably doesn't
matter too much either way.
The pre-context of that comment reads:

 * Convert a binary hash to its hex equivalent. The `_r` variant is reentrant,
 * and writes the NUL-terminated output to the buffer `out`, which must be at
 * least `GIT_MAX_HEXSZ + 1` bytes, and returns a pointer to out for
 * convenience.

so I think the intent of the example that used to use sha1_to_hex()
has been the raw bytestring that is the object name, not its form
that is encapsulated in the "struct object_id()".
I guess you're keying on the phrase "binary hash" there (the
"GIT_MAX_HEXZ" bits only apply to the "_r" variants anyway). I'd read it
as encompassing all of the functions below, including oid_to_hex(). But
I'm OK with it either way.

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help