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 8a04066d6..98cbcf94c 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 67d2cd86a..b69060bca 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 98cbcf94c..44d681da0 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 5110c56c4..3d034759b 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 ee93d3a93..e008ee92a 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 b69060bca..0c50d0a26 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 42f4d78e5..5cecef434 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 44d681da0..e530a21e5 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 c6163ff5b..d2e170ce6 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 3d034759b..cfb73500e 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 6b4cf70c6..679adab6a 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);