git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Subject: [PATCH 17/16] http-push: refactor parsing of remote object names
Date: Thu, 19 Jun 2014 17:58:10 -0400	[thread overview]
Message-ID: <20140619215810.GA29685@sigill.intra.peff.net> (raw)
In-Reply-To: <20140618194117.GA22269@sigill.intra.peff.net>

We get loose object names like "objects/??/..." from the
remote side, and need to convert them to their hex
representation.

The code to do so is rather hard to follow, as it uses some
calculated lengths whose origins are hard to understand and
verify (e.g., the path must be exactly 49 characters long.
why? Why doesn't the strcpy overflow obj_hex, which is the
same length as path?).

We can simplify this a bit by using skip_prefix, using standard
40- and 20-character buffers for hex and binary sha1s, and
adding some comments.

We also drop a totally bogus comment that claims strlcpy
cannot be used because "path" is not NUL-terminated. Right
between a call to strlen(path) and strcpy(path).

Signed-off-by: Jeff King <peff@peff.net>
---
I found this one while doing the xstrfmt series, but it actually doesn't
need xstrfmt, and does need skip_prefix, so it probably goes better on
the skip-prefix topic.

It's still a little more magical than I would like, but I think this is
the best we can do while still building on get_sha1_hex. Parsing it
left-to-right would be better, but we would essentially end up
reimplementing get_sha1_hex.

 http-push.c | 38 +++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/http-push.c b/http-push.c
index 26dfa67..c5c95e8 100644
--- a/http-push.c
+++ b/http-push.c
@@ -719,14 +719,10 @@ static int fetch_indices(void)
 	return ret;
 }
 
-static void one_remote_object(const char *hex)
+static void one_remote_object(const unsigned char *sha1)
 {
-	unsigned char sha1[20];
 	struct object *obj;
 
-	if (get_sha1_hex(hex, sha1) != 0)
-		return;
-
 	obj = lookup_object(sha1);
 	if (!obj)
 		obj = parse_object(sha1);
@@ -1020,26 +1016,38 @@ 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 */
+static int get_sha1_hex_from_objpath(const char *path, unsigned char *sha1)
+{
+	char hex[40];
+
+	if (strlen(path) != 41)
+		return -1;
+
+	memcpy(hex, path, 2);
+	path += 2;
+	path++; /* skip '/' */
+	memcpy(hex, path, 38);
+
+	return get_sha1_hex(hex, sha1);
+}
+
 static void process_ls_object(struct remote_ls_ctx *ls)
 {
 	unsigned int *parent = (unsigned int *)ls->userData;
-	char *path = ls->dentry_name;
-	char *obj_hex;
+	const char *path = ls->dentry_name;
+	unsigned char sha1[20];
 
 	if (!strcmp(ls->path, ls->dentry_name) && (ls->flags & IS_DIR)) {
 		remote_dir_exists[*parent] = 1;
 		return;
 	}
 
-	if (strlen(path) != 49)
+	if (!skip_prefix(path, "objects/", &path) ||
+	    get_sha1_hex_from_objpath(path, sha1))
 		return;
-	path += 8;
-	obj_hex = xmalloc(strlen(path));
-	/* NB: path is not null-terminated, can not use strlcpy here */
-	memcpy(obj_hex, path, 2);
-	strcpy(obj_hex + 2, path + 3);
-	one_remote_object(obj_hex);
-	free(obj_hex);
+
+	one_remote_object(sha1);
 }
 
 static void process_ls_ref(struct remote_ls_ctx *ls)
-- 
2.0.0.566.gfe3e6b2

  parent reply	other threads:[~2014-06-19 21:58 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-18 19:41 [PATCH 0/16] skip_prefix refactoring and cleanups Jeff King
2014-06-18 19:41 ` [PATCH 01/16] parse_diff_color_slot: drop ofs parameter Jeff King
2014-06-18 19:41 ` [PATCH 02/16] daemon: mark some strings as const Jeff King
2014-06-18 19:42 ` [PATCH 03/16] avoid using skip_prefix as a boolean Jeff King
2014-06-18 19:44 ` [PATCH 04/16] refactor skip_prefix to return " Jeff King
2014-06-20  1:59   ` Eric Sunshine
2014-06-20  2:08     ` Jeff King
2014-06-20  2:30       ` Eric Sunshine
2014-06-20  2:38         ` Jeff King
2014-06-23 18:50   ` Junio C Hamano
2014-06-23 21:07     ` Jeff King
2014-06-23 21:32       ` [PATCH] builtin/clone.c: detect a clone starting at a tag correctly Junio C Hamano
2014-06-18 19:45 ` [PATCH 05/16] apply: use skip_prefix instead of raw addition Jeff King
2014-06-18 19:46 ` [PATCH 06/16] fast-import: fix read of uninitialized argv memory Jeff King
2014-06-18 19:47 ` [PATCH 07/16] transport-helper: avoid reading past end-of-string Jeff King
2014-06-18 19:47 ` [PATCH 08/16] use skip_prefix to avoid magic numbers Jeff King
2014-06-23 21:44   ` Junio C Hamano
2014-07-01 17:35     ` Jeff King
2014-06-18 19:48 ` [PATCH 09/16] use skip_prefix to avoid repeating strings Jeff King
2014-06-18 19:49 ` [PATCH 10/16] fast-import: use skip_prefix for parsing input Jeff King
2014-06-20  3:19   ` Eric Sunshine
2014-06-20  5:45     ` Jeff King
2014-06-20  8:59       ` Eric Sunshine
2014-06-18 19:49 ` [PATCH 11/16] daemon: use skip_prefix to avoid magic numbers Jeff King
2014-06-18 19:51 ` [PATCH 12/16] stat_opt: check extra strlen call Jeff King
2014-06-18 19:51 ` [PATCH 13/16] fast-import: refactor parsing of spaces Jeff King
2014-06-18 19:56 ` [PATCH 14/16] fetch-pack: refactor parsing in get_ack Jeff King
2014-06-20  5:15   ` Eric Sunshine
2014-06-18 19:56 ` [PATCH 15/16] git: avoid magic number with skip_prefix Jeff King
2014-06-18 19:57 ` [PATCH 16/16] use skip_prefix to avoid repeated calculations Jeff King
2014-06-19  5:22 ` [PATCH 0/16] skip_prefix refactoring and cleanups Tanay Abhra
2014-06-19 21:58 ` Jeff King [this message]
2014-06-19 22:08   ` [PATCH 17/16] http-push: refactor parsing of remote object names 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

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

  git send-email \
    --in-reply-to=20140619215810.GA29685@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.
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 NNTP newsgroup(s).