git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Subject: [PATCH 08/68] add reentrant variants of sha1_to_hex and find_unique_abbrev
Date: Thu, 24 Sep 2015 17:05:45 -0400	[thread overview]
Message-ID: <20150924210545.GE30946@sigill.intra.peff.net> (raw)
In-Reply-To: <20150924210225.GA23624@sigill.intra.peff.net>

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>
---
 cache.h     | 31 ++++++++++++++++++++++++++++++-
 hex.c       | 13 +++++++++----
 sha1_name.c | 16 +++++++++++-----
 strbuf.c    |  9 +++++++++
 strbuf.h    |  8 ++++++++
 5 files changed, 67 insertions(+), 10 deletions(-)

diff --git a/cache.h b/cache.h
index e231e47..030b880 100644
--- a/cache.h
+++ b/cache.h
@@ -785,7 +785,24 @@ extern char *sha1_pack_name(const unsigned char *sha1);
  */
 extern char *sha1_pack_index_name(const unsigned char *sha1);
 
-extern const char *find_unique_abbrev(const unsigned char *sha1, int);
+/*
+ * Return an abbreviated sha1 unique within this repository's object database.
+ * The result will be at least `len` characters long, and will be NUL
+ * terminated.
+ *
+ * The non-`_r` version returns a static buffer which will be overwritten by
+ * subsequent calls.
+ *
+ * The `_r` variant writes to a buffer supplied by the caller, which must be at
+ * least `GIT_SHA1_HEXSZ + 1` bytes. The return value is the number of bytes
+ * written (excluding the NUL terminator).
+ *
+ * Note that while this version avoids the static buffer, it is not fully
+ * reentrant, as it calls into other non-reentrant git code.
+ */
+extern const char *find_unique_abbrev(const unsigned char *sha1, int len);
+extern int find_unique_abbrev_r(char *hex, const unsigned char *sha1, int len);
+
 extern const unsigned char null_sha1[GIT_SHA1_RAWSZ];
 
 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 *);
 extern int get_sha1_hex(const char *hex, unsigned char *sha1);
 extern int get_oid_hex(const char *hex, struct object_id *sha1);
 
+/*
+ * 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
+ * least `GIT_SHA1_HEXSZ + 1` bytes, and returns a pointer to out for
+ * convenience.
+ *
+ * The non-`_r` variant returns a static buffer, but uses a ring of 4
+ * buffers, making it safe to make multiple calls for a single statement, like:
+ *
+ *   printf("%s -> %s", sha1_to_hex(one), sha1_to_hex(two));
+ */
+extern char *sha1_to_hex_r(char *out, const unsigned char *sha1);
 extern char *sha1_to_hex(const unsigned char *sha1);	/* static buffer result! */
 extern char *oid_to_hex(const struct object_id *oid);	/* same static buffer as sha1_to_hex */
 
diff --git a/hex.c b/hex.c
index 899b74a..0519f85 100644
--- a/hex.c
+++ b/hex.c
@@ -61,12 +61,10 @@ int get_oid_hex(const char *hex, struct object_id *oid)
 	return get_sha1_hex(hex, oid->hash);
 }
 
-char *sha1_to_hex(const unsigned char *sha1)
+char *sha1_to_hex_r(char *buffer, const unsigned char *sha1)
 {
-	static int bufno;
-	static char hexbuffer[4][GIT_SHA1_HEXSZ + 1];
 	static const char hex[] = "0123456789abcdef";
-	char *buffer = hexbuffer[3 & ++bufno], *buf = buffer;
+	char *buf = buffer;
 	int i;
 
 	for (i = 0; i < GIT_SHA1_RAWSZ; i++) {
@@ -79,6 +77,13 @@ char *sha1_to_hex(const unsigned char *sha1)
 	return buffer;
 }
 
+char *sha1_to_hex(const unsigned char *sha1)
+{
+	static int bufno;
+	static char hexbuffer[4][GIT_SHA1_HEXSZ + 1];
+	return sha1_to_hex_r(hexbuffer[3 & ++bufno], sha1);
+}
+
 char *oid_to_hex(const struct object_id *oid)
 {
 	return sha1_to_hex(oid->hash);
diff --git a/sha1_name.c b/sha1_name.c
index da6874c..c58b477 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -368,14 +368,13 @@ int for_each_abbrev(const char *prefix, each_abbrev_fn fn, void *cb_data)
 	return ds.ambiguous;
 }
 
-const char *find_unique_abbrev(const unsigned char *sha1, int len)
+int find_unique_abbrev_r(char *hex, const unsigned char *sha1, int len)
 {
 	int status, exists;
-	static char hex[41];
 
-	memcpy(hex, sha1_to_hex(sha1), 40);
+	sha1_to_hex_r(hex, sha1);
 	if (len == 40 || !len)
-		return hex;
+		return 40;
 	exists = has_sha1_file(sha1);
 	while (len < 40) {
 		unsigned char sha1_ret[20];
@@ -384,10 +383,17 @@ const char *find_unique_abbrev(const unsigned char *sha1, int len)
 		    ? !status
 		    : status == SHORT_NAME_NOT_FOUND) {
 			hex[len] = 0;
-			return hex;
+			return len;
 		}
 		len++;
 	}
+	return len;
+}
+
+const char *find_unique_abbrev(const unsigned char *sha1, int len)
+{
+	static char hex[GIT_SHA1_HEXSZ + 1];
+	find_unique_abbrev_r(hex, sha1, len);
 	return hex;
 }
 
diff --git a/strbuf.c b/strbuf.c
index 29df55b..f3c44fb 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -743,3 +743,12 @@ void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm)
 	}
 	strbuf_setlen(sb, sb->len + len);
 }
+
+void strbuf_add_unique_abbrev(struct strbuf *sb, const unsigned char *sha1,
+			      int abbrev_len)
+{
+	int r;
+	strbuf_grow(sb, GIT_SHA1_HEXSZ + 1);
+	r = find_unique_abbrev_r(sb->buf + sb->len, sha1, abbrev_len);
+	strbuf_setlen(sb, sb->len + r);
+}
diff --git a/strbuf.h b/strbuf.h
index 43f27c3..0f9c8a7 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -475,6 +475,14 @@ static inline struct strbuf **strbuf_split(const struct strbuf *sb,
 extern void strbuf_list_free(struct strbuf **);
 
 /**
+ * Add the abbreviation, as generated by find_unique_abbrev, of `sha1` to
+ * the strbuf `sb`.
+ */
+extern void strbuf_add_unique_abbrev(struct strbuf *sb,
+				     const unsigned char *sha1,
+				     int abbrev_len);
+
+/**
  * Launch the user preferred editor to edit a file and fill the buffer
  * with the file's contents upon the user completing their editing. The
  * third argument can be used to set the environment which the editor is
-- 
2.6.0.rc3.454.g204ad51

  parent reply	other threads:[~2015-09-24 21:06 UTC|newest]

Thread overview: 93+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-24 21:02 [PATCH v2 0/68] war on sprintf Jeff King
2015-09-24 21:02 ` [PATCH 01/68] show-branch: avoid segfault with --reflog of unborn branch Jeff King
2015-09-24 21:03 ` [PATCH 02/68] mailsplit: fix FILE* leak in split_maildir Jeff King
2015-09-24 21:03 ` [PATCH 03/68] archive-tar: fix minor indentation violation Jeff King
2015-09-24 21:05 ` [PATCH 04/68] fsck: don't fsck alternates for connectivity-only check Jeff King
2015-09-24 21:05 ` [PATCH 05/68] add xsnprintf helper function Jeff King
2015-09-24 21:05 ` [PATCH 06/68] add git_path_buf " Jeff King
2015-09-24 21:05 ` [PATCH 07/68] strbuf: make strbuf_complete_line more generic Jeff King
2015-09-24 21:05 ` Jeff King [this message]
2015-09-24 21:05 ` [PATCH 09/68] fsck: use strbuf to generate alternate directories Jeff King
2015-09-24 21:05 ` [PATCH 10/68] mailsplit: make PATH_MAX buffers dynamic Jeff King
2015-09-24 21:05 ` [PATCH 11/68] trace: use strbuf for quote_crnl output Jeff King
2015-09-24 21:05 ` [PATCH 12/68] progress: store throughput display in a strbuf Jeff King
2015-09-24 21:06 ` [PATCH 13/68] test-dump-cache-tree: avoid overflow of cache-tree name Jeff King
2015-09-24 21:06 ` [PATCH 14/68] compat/inet_ntop: fix off-by-one in inet_ntop4 Jeff King
2015-09-24 21:06 ` [PATCH 15/68] convert trivial sprintf / strcpy calls to xsnprintf Jeff King
2015-09-24 21:06 ` [PATCH 16/68] archive-tar: use xsnprintf for trivial formatting Jeff King
2015-09-24 21:06 ` [PATCH 17/68] use xsnprintf for generating git object headers Jeff King
2015-09-24 21:06 ` [PATCH 18/68] find_short_object_filename: convert sprintf to xsnprintf Jeff King
2015-09-24 21:06 ` [PATCH 19/68] stop_progress_msg: " Jeff King
2015-09-24 21:06 ` [PATCH 20/68] compat/hstrerror: convert sprintf to snprintf Jeff King
2015-09-24 21:06 ` [PATCH 21/68] grep: use xsnprintf to format failure message Jeff King
2015-09-24 21:06 ` [PATCH 22/68] entry.c: convert strcpy to xsnprintf Jeff King
2015-09-24 21:06 ` [PATCH 23/68] add_packed_git: convert strcpy into xsnprintf Jeff King
2015-09-24 21:06 ` [PATCH 24/68] http-push: replace strcat with xsnprintf Jeff King
2015-09-24 21:07 ` [PATCH 25/68] receive-pack: convert strncpy to xsnprintf Jeff King
2015-09-24 21:07 ` [PATCH 26/68] replace trivial malloc + sprintf / strcpy calls with xstrfmt Jeff King
2015-09-24 21:07 ` [PATCH 27/68] config: use xstrfmt in normalize_value Jeff King
2015-09-24 21:07 ` [PATCH 28/68] fetch: replace static buffer with xstrfmt Jeff King
2015-09-24 21:07 ` [PATCH 29/68] use strip_suffix and xstrfmt to replace suffix Jeff King
2015-09-24 21:07 ` [PATCH 30/68] ref-filter: drop sprintf and strcpy calls Jeff King
2015-09-24 21:07 ` [PATCH 31/68] help: drop prepend function in favor of xstrfmt Jeff King
2015-09-24 21:07 ` [PATCH 32/68] mailmap: replace strcpy with xstrdup Jeff King
2015-09-24 21:07 ` [PATCH 33/68] read_branches_file: simplify string handling Jeff King
2015-09-24 21:07 ` [PATCH 34/68] read_remotes_file: " Jeff King
2015-09-24 21:07 ` [PATCH 35/68] resolve_ref: use strbufs for internal buffers Jeff King
2015-09-24 21:07 ` [PATCH 36/68] upload-archive: convert sprintf to strbuf Jeff King
2015-09-24 21:07 ` [PATCH 37/68] remote-ext: simplify git pkt-line generation Jeff King
2015-09-24 21:07 ` [PATCH 38/68] http-push: use strbuf instead of fwrite_buffer Jeff King
2015-09-24 21:07 ` [PATCH 39/68] http-walker: store url in a strbuf Jeff King
2015-09-24 21:07 ` [PATCH 40/68] sha1_get_pack_name: use " Jeff King
2015-09-24 21:07 ` [PATCH 41/68] init: use strbufs to store paths Jeff King
2015-09-29 23:50   ` Michael Blume
2015-09-30  0:23     ` Jeff King
2015-09-30 20:00       ` Junio C Hamano
2015-10-01  2:51         ` Jeff King
2015-10-02  6:00           ` Torsten Bögershausen
2015-10-02 15:33             ` Jeff King
2015-10-03  5:58       ` Torsten Bögershausen
2015-10-03 16:54         ` Junio C Hamano
2015-10-03 21:12           ` Torsten Bögershausen
2015-10-04  3:37             ` Jeff King
2015-10-04  6:31               ` Torsten Bögershausen
2015-10-05  3:41                 ` Jeff King
2015-10-05  3:43                   ` [PATCH 1/3] precompose_utf8: drop unused variable Jeff King
2015-10-06  3:24                     ` Torsten Bögershausen
2015-10-05  3:45                   ` [PATCH 2/3] probe_utf8_pathname_composition: use internal strbuf Jeff King
2015-10-05  3:46                   ` [PATCH 3/3] init: use strbufs to store paths Jeff King
2015-09-24 21:07 ` [PATCH 42/68] apply: convert root string to strbuf Jeff King
2015-09-24 21:07 ` [PATCH 43/68] transport: use strbufs for status table "quickref" strings Jeff King
2015-09-24 21:07 ` [PATCH 44/68] merge-recursive: convert malloc / strcpy to strbuf Jeff King
2015-09-24 21:07 ` [PATCH 45/68] enter_repo: convert fixed-size buffers to strbufs Jeff King
2015-09-24 21:07 ` [PATCH 46/68] remove_leading_path: use a strbuf for internal storage Jeff King
2015-09-24 21:07 ` [PATCH 47/68] write_loose_object: convert to strbuf Jeff King
2015-09-24 21:07 ` [PATCH 48/68] diagnose_invalid_index_path: use strbuf to avoid strcpy/strcat Jeff King
2015-09-24 21:07 ` [PATCH 49/68] fetch-pack: use argv_array for index-pack / unpack-objects Jeff King
2015-09-24 21:07 ` [PATCH 50/68] http-push: use an argv_array for setup_revisions Jeff King
2015-09-24 21:07 ` [PATCH 51/68] stat_tracking_info: convert to argv_array Jeff King
2015-09-24 21:08 ` [PATCH 52/68] daemon: use cld->env_array when re-spawning Jeff King
2015-09-24 21:08 ` [PATCH 53/68] use sha1_to_hex_r() instead of strcpy Jeff King
2015-09-24 21:08 ` [PATCH 54/68] drop strcpy in favor of raw sha1_to_hex Jeff King
2015-09-24 23:42   ` Eric Sunshine
2015-09-25  1:36     ` Jeff King
2015-09-24 21:08 ` [PATCH 55/68] color: add overflow checks for parsing colors Jeff King
2015-09-24 21:08 ` [PATCH 56/68] use alloc_ref rather than hand-allocating "struct ref" Jeff King
2015-09-24 21:08 ` [PATCH 57/68] avoid sprintf and strcpy with flex arrays Jeff King
2015-09-24 21:08 ` [PATCH 58/68] receive-pack: simplify keep_arg computation Jeff King
2015-09-24 21:08 ` [PATCH 59/68] help: clean up kfmclient munging Jeff King
2015-09-24 21:08 ` [PATCH 60/68] prefer memcpy to strcpy Jeff King
2015-09-27 11:19   ` René Scharfe
2015-09-27 13:06     ` Torsten Bögershausen
2015-09-27 13:13       ` René Scharfe
2015-09-27 13:24         ` René Scharfe
2015-09-28  7:09   ` Rasmus Villemoes
2015-09-24 21:08 ` [PATCH 61/68] color: add color_set helper for copying raw colors Jeff King
2015-09-24 21:08 ` [PATCH 62/68] notes: document length of fanout path with a constant Jeff King
2015-09-24 21:08 ` [PATCH 63/68] convert strncpy to memcpy Jeff King
2015-09-24 21:08 ` [PATCH 64/68] fsck: drop inode-sorting code Jeff King
2015-09-24 21:08 ` [PATCH 65/68] Makefile: drop D_INO_IN_DIRENT build knob Jeff King
2015-09-24 21:08 ` [PATCH 66/68] fsck: use for_each_loose_file_in_objdir Jeff King
2015-09-26  3:36   ` Jeff King
2015-09-24 21:08 ` [PATCH 67/68] use strbuf_complete to conditionally append slash Jeff King
2015-09-24 21:08 ` [PATCH 68/68] name-rev: use strip_suffix to avoid magic numbers Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150924210545.GE30946@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).