From 8009768e89dfe389327654cf9d6868f680ef1f40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 18 Oct 2008 10:37:40 +0200 Subject: [PATCH 1/3] add alloc_ref_with_prefix() In three cases in remote.c, a "raw" ref is allocated using alloc_ref() and then its is constructed using sprintf(). Clean it up by adding a helper function, alloc_ref_with_prefix(), which creates a composite name. Use it in alloc_ref_from_str(), too, as it simplifies the code. Open code alloc_ref() in alloc_ref_with_prefix(), as the former is going to be removed in the patch after the next. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- remote.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/remote.c b/remote.c index 8a04066d61..98cbcf94c3 100644 --- a/remote.c +++ b/remote.c @@ -749,6 +749,16 @@ int remote_find_tracking(struct remote *remote, struct refspec *refspec) return -1; } +static struct ref *alloc_ref_with_prefix(const char *prefix, size_t prefixlen, + const char *name) +{ + size_t len = strlen(name); + struct ref *ref = xcalloc(1, sizeof(struct ref) + prefixlen + len + 1); + memcpy(ref->name, prefix, prefixlen); + memcpy(ref->name + prefixlen, name, len); + return ref; +} + struct ref *alloc_ref(unsigned namelen) { struct ref *ret = xcalloc(1, sizeof(struct ref) + namelen); @@ -757,9 +767,7 @@ struct ref *alloc_ref(unsigned namelen) struct ref *alloc_ref_from_str(const char* str) { - struct ref *ret = alloc_ref(strlen(str) + 1); - strcpy(ret->name, str); - return ret; + return alloc_ref_with_prefix("", 0, str); } static struct ref *copy_ref(const struct ref *ref) @@ -1152,10 +1160,8 @@ static struct ref *get_expanded_map(const struct ref *remote_refs, struct ref *cpy = copy_ref(ref); match = ref->name + remote_prefix_len; - cpy->peer_ref = alloc_ref(local_prefix_len + - strlen(match) + 1); - sprintf(cpy->peer_ref->name, "%s%s", - refspec->dst, match); + cpy->peer_ref = alloc_ref_with_prefix(refspec->dst, + local_prefix_len, match); if (refspec->force) cpy->peer_ref->force = 1; *tail = cpy; @@ -1188,7 +1194,6 @@ struct ref *get_remote_ref(const struct ref *remote_refs, const char *name) static struct ref *get_local_ref(const char *name) { - struct ref *ret; if (!name) return NULL; @@ -1198,15 +1203,10 @@ static struct ref *get_local_ref(const char *name) if (!prefixcmp(name, "heads/") || !prefixcmp(name, "tags/") || - !prefixcmp(name, "remotes/")) { - ret = alloc_ref(strlen(name) + 6); - sprintf(ret->name, "refs/%s", name); - return ret; - } + !prefixcmp(name, "remotes/")) + return alloc_ref_with_prefix("refs/", 5, name); - ret = alloc_ref(strlen(name) + 12); - sprintf(ret->name, "refs/heads/%s", name); - return ret; + return alloc_ref_with_prefix("refs/heads/", 11, name); } int get_fetch_map(const struct ref *remote_refs, From b0b44bc7b26c8c4b4221a377ce6ba174b843cb8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 18 Oct 2008 10:41:33 +0200 Subject: [PATCH 2/3] use alloc_ref_from_str() everywhere Replace pairs of alloc_ref() and strcpy() with alloc_ref_from_str(), simplifying the code. In connect.c, also a pair of alloc_ref() and memcpy() is replaced -- the additional cost of a strlen() call should not have too much of an impact. Consistency and simplicity are more important. In remote.c, the code was allocating 11 bytes more than needed for the name part, but I couldn't see them being used for anything. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- connect.c | 3 +-- remote.c | 3 +-- transport.c | 6 ++---- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/connect.c b/connect.c index 67d2cd86a8..b69060bca0 100644 --- a/connect.c +++ b/connect.c @@ -90,9 +90,8 @@ struct ref **get_remote_heads(int in, struct ref **list, continue; if (nr_match && !path_match(name, nr_match, match)) continue; - ref = alloc_ref(name_len + 1); + ref = alloc_ref_from_str(buffer + 41); hashcpy(ref->old_sha1, old_sha1); - memcpy(ref->name, buffer + 41, name_len + 1); *list = ref; list = &ref->next; } diff --git a/remote.c b/remote.c index 98cbcf94c3..44d681da08 100644 --- a/remote.c +++ b/remote.c @@ -878,8 +878,7 @@ static struct ref *try_explicit_object_name(const char *name) struct ref *ref; if (!*name) { - ref = alloc_ref(20); - strcpy(ref->name, "(delete)"); + ref = alloc_ref_from_str("(delete)"); hashclr(ref->new_sha1); return ref; } diff --git a/transport.c b/transport.c index 5110c56c4e..3d034759bf 100644 --- a/transport.c +++ b/transport.c @@ -75,7 +75,7 @@ static int read_loose_refs(struct strbuf *path, int name_offset, if (fd < 0) continue; - next = alloc_ref(path->len - name_offset + 1); + next = alloc_ref_from_str(path->buf + name_offset); if (read_in_full(fd, buffer, 40) != 40 || get_sha1_hex(buffer, next->old_sha1)) { close(fd); @@ -83,7 +83,6 @@ static int read_loose_refs(struct strbuf *path, int name_offset, continue; } close(fd); - strcpy(next->name, path->buf + name_offset); (*tail)->next = next; *tail = next; } @@ -127,14 +126,13 @@ static void insert_packed_refs(const char *packed_refs, struct ref **list) (*list)->next->name)) > 0) list = &(*list)->next; if (!(*list)->next || cmp < 0) { - struct ref *next = alloc_ref(len - 40); + struct ref *next = alloc_ref_from_str(buffer + 41); buffer[40] = '\0'; if (get_sha1_hex(buffer, next->old_sha1)) { warning ("invalid SHA-1: %s", buffer); free(next); continue; } - strcpy(next->name, buffer + 41); next->next = (*list)->next; (*list)->next = next; list = &(*list)->next; From 59c69c0c656ebce2f7ce870b4913512597a98390 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 18 Oct 2008 10:44:18 +0200 Subject: [PATCH 3/3] make alloc_ref_from_str() the new alloc_ref() With all calls to alloc_ref() gone, we can remove it and then we're free to give alloc_ref_from_str() the shorter name. It's a much nicer interface, as the callers always need to have a name string when they allocate a ref anyway and don't need to calculate and pass its length+1 any more. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- builtin-fetch.c | 4 ++-- connect.c | 2 +- http-push.c | 4 ++-- remote.c | 21 +++++++-------------- remote.h | 4 +--- transport.c | 8 ++++---- walker.c | 2 +- 7 files changed, 18 insertions(+), 27 deletions(-) diff --git a/builtin-fetch.c b/builtin-fetch.c index ee93d3a93d..e008ee92ab 100644 --- a/builtin-fetch.c +++ b/builtin-fetch.c @@ -521,8 +521,8 @@ static void find_non_local_tags(struct transport *transport, will_fetch(head, ref->old_sha1))) { string_list_insert(ref_name, &new_refs); - rm = alloc_ref_from_str(ref_name); - rm->peer_ref = alloc_ref_from_str(ref_name); + rm = alloc_ref(ref_name); + rm->peer_ref = alloc_ref(ref_name); hashcpy(rm->old_sha1, ref_sha1); **tail = rm; diff --git a/connect.c b/connect.c index b69060bca0..0c50d0a26a 100644 --- a/connect.c +++ b/connect.c @@ -90,7 +90,7 @@ struct ref **get_remote_heads(int in, struct ref **list, continue; if (nr_match && !path_match(name, nr_match, match)) continue; - ref = alloc_ref_from_str(buffer + 41); + ref = alloc_ref(buffer + 41); hashcpy(ref->old_sha1, old_sha1); *list = ref; list = &ref->next; diff --git a/http-push.c b/http-push.c index 42f4d78e54..5cecef434a 100644 --- a/http-push.c +++ b/http-push.c @@ -1780,7 +1780,7 @@ static void one_remote_ref(char *refname) struct ref *ref; struct object *obj; - ref = alloc_ref_from_str(refname); + ref = alloc_ref(refname); if (http_fetch_ref(remote->url, ref) != 0) { fprintf(stderr, @@ -1887,7 +1887,7 @@ static void add_remote_info_ref(struct remote_ls_ctx *ls) char *ref_info; struct ref *ref; - ref = alloc_ref_from_str(ls->dentry_name); + ref = alloc_ref(ls->dentry_name); if (http_fetch_ref(remote->url, ref) != 0) { fprintf(stderr, diff --git a/remote.c b/remote.c index 44d681da08..e530a21e5c 100644 --- a/remote.c +++ b/remote.c @@ -759,15 +759,9 @@ static struct ref *alloc_ref_with_prefix(const char *prefix, size_t prefixlen, return ref; } -struct ref *alloc_ref(unsigned namelen) +struct ref *alloc_ref(const char *name) { - struct ref *ret = xcalloc(1, sizeof(struct ref) + namelen); - return ret; -} - -struct ref *alloc_ref_from_str(const char* str) -{ - return alloc_ref_with_prefix("", 0, str); + return alloc_ref_with_prefix("", 0, name); } static struct ref *copy_ref(const struct ref *ref) @@ -878,20 +872,20 @@ static struct ref *try_explicit_object_name(const char *name) struct ref *ref; if (!*name) { - ref = alloc_ref_from_str("(delete)"); + ref = alloc_ref("(delete)"); hashclr(ref->new_sha1); return ref; } if (get_sha1(name, sha1)) return NULL; - ref = alloc_ref_from_str(name); + ref = alloc_ref(name); hashcpy(ref->new_sha1, sha1); return ref; } static struct ref *make_linked_ref(const char *name, struct ref ***tail) { - struct ref *ret = alloc_ref_from_str(name); + struct ref *ret = alloc_ref(name); tail_link_ref(ret, tail); return ret; } @@ -1196,9 +1190,8 @@ static struct ref *get_local_ref(const char *name) if (!name) return NULL; - if (!prefixcmp(name, "refs/")) { - return alloc_ref_from_str(name); - } + if (!prefixcmp(name, "refs/")) + return alloc_ref(name); if (!prefixcmp(name, "heads/") || !prefixcmp(name, "tags/") || diff --git a/remote.h b/remote.h index c6163ff5b1..d2e170ce66 100644 --- a/remote.h +++ b/remote.h @@ -55,9 +55,7 @@ struct refspec { extern const struct refspec *tag_refspec; -struct ref *alloc_ref(unsigned namelen); - -struct ref *alloc_ref_from_str(const char* str); +struct ref *alloc_ref(const char *name); struct ref *copy_ref_list(const struct ref *ref); diff --git a/transport.c b/transport.c index 3d034759bf..cfb73500ec 100644 --- a/transport.c +++ b/transport.c @@ -75,7 +75,7 @@ static int read_loose_refs(struct strbuf *path, int name_offset, if (fd < 0) continue; - next = alloc_ref_from_str(path->buf + name_offset); + next = alloc_ref(path->buf + name_offset); if (read_in_full(fd, buffer, 40) != 40 || get_sha1_hex(buffer, next->old_sha1)) { close(fd); @@ -126,7 +126,7 @@ static void insert_packed_refs(const char *packed_refs, struct ref **list) (*list)->next->name)) > 0) list = &(*list)->next; if (!(*list)->next || cmp < 0) { - struct ref *next = alloc_ref_from_str(buffer + 41); + struct ref *next = alloc_ref(buffer + 41); buffer[40] = '\0'; if (get_sha1_hex(buffer, next->old_sha1)) { warning ("invalid SHA-1: %s", buffer); @@ -499,7 +499,7 @@ static struct ref *get_refs_via_curl(struct transport *transport) strbuf_release(&buffer); - ref = alloc_ref_from_str("HEAD"); + ref = alloc_ref("HEAD"); if (!walker->fetch_ref(walker, ref) && !resolve_remote_symref(ref, refs)) { ref->next = refs; @@ -540,7 +540,7 @@ static struct ref *get_refs_from_bundle(struct transport *transport) die ("Could not read bundle '%s'.", transport->url); for (i = 0; i < data->header.references.nr; i++) { struct ref_list_entry *e = data->header.references.list + i; - struct ref *ref = alloc_ref_from_str(e->name); + struct ref *ref = alloc_ref(e->name); hashcpy(ref->old_sha1, e->sha1); ref->next = result; result = ref; diff --git a/walker.c b/walker.c index 6b4cf70c6a..679adab6a0 100644 --- a/walker.c +++ b/walker.c @@ -191,7 +191,7 @@ static int interpret_target(struct walker *walker, char *target, unsigned char * if (!get_sha1_hex(target, sha1)) return 0; if (!check_ref_format(target)) { - struct ref *ref = alloc_ref_from_str(target); + struct ref *ref = alloc_ref(target); if (!walker->fetch_ref(walker, ref)) { hashcpy(sha1, ref->old_sha1); free(ref);