From ae021d87911da4328157273df24779892cb51277 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 18 Jun 2014 15:47:50 -0400 Subject: use skip_prefix to avoid magic numbers It's a common idiom to match a prefix and then skip past it with a magic number, like: if (starts_with(foo, "bar")) foo += 3; This is easy to get wrong, since you have to count the prefix string yourself, and there's no compiler check if the string changes. We can use skip_prefix to avoid the magic numbers here. Note that some of these conversions could be much shorter. For example: if (starts_with(arg, "--foo=")) { bar = arg + 6; continue; } could become: if (skip_prefix(arg, "--foo=", &bar)) continue; However, I have left it as: if (skip_prefix(arg, "--foo=", &v)) { bar = v; continue; } to visually match nearby cases which need to actually process the string. Like: if (skip_prefix(arg, "--foo=", &v)) { bar = atoi(v); continue; } Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- http-push.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'http-push.c') diff --git a/http-push.c b/http-push.c index de00d1693a..26dfa6724e 100644 --- a/http-push.c +++ b/http-push.c @@ -770,9 +770,9 @@ static void handle_new_lock_ctx(struct xml_ctx *ctx, int tag_closed) lock->owner = xmalloc(strlen(ctx->cdata) + 1); strcpy(lock->owner, ctx->cdata); } else if (!strcmp(ctx->name, DAV_ACTIVELOCK_TIMEOUT)) { - if (starts_with(ctx->cdata, "Second-")) - lock->timeout = - strtol(ctx->cdata + 7, NULL, 10); + const char *arg; + if (skip_prefix(ctx->cdata, "Second-", &arg)) + lock->timeout = strtol(arg, NULL, 10); } else if (!strcmp(ctx->name, DAV_ACTIVELOCK_TOKEN)) { lock->token = xmalloc(strlen(ctx->cdata) + 1); strcpy(lock->token, ctx->cdata); @@ -1561,6 +1561,7 @@ static void fetch_symref(const char *path, char **symref, unsigned char *sha1) { char *url; struct strbuf buffer = STRBUF_INIT; + const char *name; url = xmalloc(strlen(repo->url) + strlen(path) + 1); sprintf(url, "%s%s", repo->url, path); @@ -1578,8 +1579,8 @@ static void fetch_symref(const char *path, char **symref, unsigned char *sha1) return; /* If it's a symref, set the refname; otherwise try for a sha1 */ - if (starts_with((char *)buffer.buf, "ref: ")) { - *symref = xmemdupz((char *)buffer.buf + 5, buffer.len - 6); + if (skip_prefix(buffer.buf, "ref: ", &name)) { + *symref = xmemdupz(name, buffer.len - (name - buffer.buf)); } else { get_sha1_hex(buffer.buf, sha1); } -- cgit v1.2.1 From 67a31f612830e79fe768ac886ed9ef7eadd8fb10 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 19 Jun 2014 17:58:10 -0400 Subject: http-push: refactor parsing of remote object names 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 Signed-off-by: Junio C Hamano --- http-push.c | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) (limited to 'http-push.c') diff --git a/http-push.c b/http-push.c index 26dfa6724e..c5c95e85ea 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) -- cgit v1.2.1