On Thu, Jun 19, 2014 at 05:58:10PM -0400, Jeff King wrote:

> 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.

Just for fun, that would look something like this (on top):

diff --git a/http-push.c b/http-push.c
index c5c95e8..2425c61 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1016,20 +1016,34 @@ static void remote_ls(const char *path, int flags,
                      void (*userFunc)(struct remote_ls_ctx *ls),
                      void *userData);
 
+static inline int parse_one_hex(unsigned char **to, const char **from)
+{
+       unsigned int val;
+
+       /* avoid reading past end-of-string for from[1] */
+       if (!(*from)[0])
+               return -1;
+       val = (hexval((*from)[0]) << 4) | hexval((*from)[1]);
+       if (val & ~0xff)
+               return -1;
+       *(*to)++ = val;
+       (*from) += 2;
+       return 0;
+}
+
 /* extract hex from sharded "xx/x{40}" filename */
 static int get_sha1_hex_from_objpath(const char *path, unsigned char *sha1)
 {
-       char hex[40];
+       int i;
 
-       if (strlen(path) != 41)
+       if (parse_one_hex(&sha1, &path) < 0)
                return -1;
-
-       memcpy(hex, path, 2);
-       path += 2;
-       path++; /* skip '/' */
-       memcpy(hex, path, 38);
-
-       return get_sha1_hex(hex, sha1);
+       if (*path++ != '/')
+               return -1;
+       for (i = 1; i < 20; i++)
+               if (parse_one_hex(&sha1, &path) < 0)
+                       return -1;
+       return 0;
 }
 
 static void process_ls_object(struct remote_ls_ctx *ls)

In theory we could factor parse_one_hex to share with get_sha1_hex, but
I am hesitant to touch get_sha1_hex, as it is used in a lot of tight
loops.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to