@matheusbsbs / matheus-bagliano / commits / af49c6d091

add reentrant variants of sha1_to_hex and find_unique_abbrev

The sha1_to_hex and find_unique_abbrev functions always write into reusable static buffers. There are a few problems with this: - future calls overwrite our result. This is especially annoying with find_unique_abbrev, which does not have a ring of buffers, so you cannot even printf() a result that has two abbreviated sha1s. - if you want to put the result into another buffer, we often strcpy, which looks suspicious when auditing for overflows. This patch introduces sha1_to_hex_r and find_unique_abbrev_r, which write into a user-provided buffer. Of course this is just punting on the overflow-auditing, as the buffer obviously needs to be GIT_SHA1_HEXSZ + 1 bytes. But it is much easier to audit, since that is a well-known size. We retain the non-reentrant forms, which just become thin wrappers around the reentrant ones. This patch also adds a strbuf variant of find_unique_abbrev, which will be handy in later patches. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Sep 24, 2015 at 17:05 UTC af49c6d0918bf04aad89bd885a4eef5767a33d0e

5 files changed +67 -10

cache.h

+30 -1

@@ -785,7 +785,24 @@ extern char *sha1_pack_name(const unsigned char *sha1);

785 */

786 extern char *sha1_pack_index_name(const unsigned char *sha1);

787

788 -extern const char *find_unique_abbrev(const unsigned char *sha1, int);

788 +/*

789 + * Return an abbreviated sha1 unique within this repository's object database.

790 + * The result will be at least `len` characters long, and will be NUL

791 + * terminated.

792 + *

793 + * The non-`_r` version returns a static buffer which will be overwritten by

794 + * subsequent calls.

795 + *

796 + * The `_r` variant writes to a buffer supplied by the caller, which must be at

797 + * least `GIT_SHA1_HEXSZ + 1` bytes. The return value is the number of bytes

798 + * written (excluding the NUL terminator).

799 + *

800 + * Note that while this version avoids the static buffer, it is not fully

801 + * reentrant, as it calls into other non-reentrant git code.

802 + */

803 +extern const char *find_unique_abbrev(const unsigned char *sha1, int len);

804 +extern int find_unique_abbrev_r(char *hex, const unsigned char *sha1, int len);

805 +

806 extern const unsigned char null_sha1[GIT_SHA1_RAWSZ];

807

808 static inline int hashcmp(const unsigned char *sha1, const unsigned char *sha2)

@@ -1067,6 +1084,18 @@ extern int for_each_abbrev(const char *prefix, each_abbrev_fn, void *);

1084 extern int get_sha1_hex(const char *hex, unsigned char *sha1);

1085 extern int get_oid_hex(const char *hex, struct object_id *sha1);

1086

1087 +/*

1088 + * Convert a binary sha1 to its hex equivalent. The `_r` variant is reentrant,

1089 + * and writes the NUL-terminated output to the buffer `out`, which must be at

1090 + * least `GIT_SHA1_HEXSZ + 1` bytes, and returns a pointer to out for

1091 + * convenience.

1092 + *

1093 + * The non-`_r` variant returns a static buffer, but uses a ring of 4

1094 + * buffers, making it safe to make multiple calls for a single statement, like:

1095 + *

1096 + * printf("%s -> %s", sha1_to_hex(one), sha1_to_hex(two));

1097 + */

1098 +extern char *sha1_to_hex_r(char *out, const unsigned char *sha1);

1099 extern char *sha1_to_hex(const unsigned char *sha1); /* static buffer result! */

1100 extern char *oid_to_hex(const struct object_id *oid); /* same static buffer as sha1_to_hex */

1101

hex.c

+9 -4

@@ -61,12 +61,10 @@ int get_oid_hex(const char *hex, struct object_id *oid)

61 return get_sha1_hex(hex, oid->hash);

62 }

64 -char *sha1_to_hex(const unsigned char *sha1)

64 +char *sha1_to_hex_r(char *buffer, const unsigned char *sha1)

65 {

66 - static int bufno;

67 - static char hexbuffer[4][GIT_SHA1_HEXSZ + 1];

66 static const char hex[] = "0123456789abcdef";

69 - char *buffer = hexbuffer[3 & ++bufno], *buf = buffer;

67 + char *buf = buffer;

68 int i;

70 for (i = 0; i < GIT_SHA1_RAWSZ; i++) {

@@ -79,6 +77,13 @@ char *sha1_to_hex(const unsigned char *sha1)

77 return buffer;

78 }

80 +char *sha1_to_hex(const unsigned char *sha1)

81 +{

82 + static int bufno;

83 + static char hexbuffer[4][GIT_SHA1_HEXSZ + 1];

84 + return sha1_to_hex_r(hexbuffer[3 & ++bufno], sha1);

85 +}

86 +

87 char *oid_to_hex(const struct object_id *oid)

88 {

89 return sha1_to_hex(oid->hash);

sha1_name.c

+11 -5

@@ -368,14 +368,13 @@ int for_each_abbrev(const char *prefix, each_abbrev_fn fn, void *cb_data)

368 return ds.ambiguous;

369 }

370

371 -const char *find_unique_abbrev(const unsigned char *sha1, int len)

371 +int find_unique_abbrev_r(char *hex, const unsigned char *sha1, int len)

372 {

373 int status, exists;

374 - static char hex[41];

374

376 - memcpy(hex, sha1_to_hex(sha1), 40);

375 + sha1_to_hex_r(hex, sha1);

376 if (len == 40 || !len)

378 - return hex;

377 + return 40;

378 exists = has_sha1_file(sha1);

379 while (len < 40) {

380 unsigned char sha1_ret[20];

@@ -384,10 +383,17 @@ const char *find_unique_abbrev(const unsigned char *sha1, int len)

383 ? !status

384 : status == SHORT_NAME_NOT_FOUND) {

385 hex[len] = 0;

387 - return hex;

386 + return len;

387 }

388 len++;

389 }

390 + return len;

391 +}

392 +

393 +const char *find_unique_abbrev(const unsigned char *sha1, int len)

394 +{

395 + static char hex[GIT_SHA1_HEXSZ + 1];

396 + find_unique_abbrev_r(hex, sha1, len);

397 return hex;

398 }

399

strbuf.c

@@ -743,3 +743,12 @@ void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm)

743 }

744 strbuf_setlen(sb, sb->len + len);

745 }

746 +

747 +void strbuf_add_unique_abbrev(struct strbuf *sb, const unsigned char *sha1,

748 + int abbrev_len)

749 +{

750 + int r;

751 + strbuf_grow(sb, GIT_SHA1_HEXSZ + 1);

752 + r = find_unique_abbrev_r(sb->buf + sb->len, sha1, abbrev_len);

753 + strbuf_setlen(sb, sb->len + r);

754 +}

strbuf.h

@@ -474,6 +474,14 @@ static inline struct strbuf **strbuf_split(const struct strbuf *sb,

474 */

475 extern void strbuf_list_free(struct strbuf **);

476

477 +/**

478 + * Add the abbreviation, as generated by find_unique_abbrev, of `sha1` to

479 + * the strbuf `sb`.

480 + */

481 +extern void strbuf_add_unique_abbrev(struct strbuf *sb,

482 + const unsigned char *sha1,

483 + int abbrev_len);

484 +

485 /**

486 * Launch the user preferred editor to edit a file and fill the buffer

487 * with the file's contents upon the user completing their editing. The