@matheusbsbs / matheus-bagliano / commits / 10b635b773

bundle: remove "ref_list" in favor of string-list.c API

Move away from the "struct ref_list" in bundle.c in favor of the almost identical string-list.c API. That API fits this use-case perfectly, but did not exist in its current form when this code was added in 2e0afafebd (Add git-bundle: move objects and references by archive, 2007-02-22), with hindsight we could have used the path-list API, which later got renamed to string-list. See 8fd2cb4069 (Extract helper bits from c-merge-recursive work, 2006-07-25) We need to change "name" to "string" and "oid" to "util" to make this conversion, but other than that the APIs are pretty much identical for what bundle.c made use of. Let's also replace the memset(..,0,...) pattern with a more idiomatic "INIT" macro, and finally add a *_release() function so to free the allocated memory. Before this the add_to_ref_list() would leak memory, now e.g. "bundle list-heads" reports no memory leaks at all under valgrind. In the bundle_header_init() function we're using a clever trick to memcpy() what we'd get from the corresponding BUNDLE_HEADER_INIT. There is a concurrent series to make use of that pattern more generally, see [1]. 1. https://lore.kernel.org/git/cover-0.5-00000000000-20210701T104855Z-avarab@gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Jul 2, 2021 at 11:57 UTC 10b635b77311badcbb5045b7421e6826c4536613

4 files changed +50 -43

builtin/bundle.c

+6 -6

@@ -100,7 +100,7 @@ static int cmd_bundle_create(int argc, const char **argv, const char *prefix) {

100 }

101

102 static int cmd_bundle_verify(int argc, const char **argv, const char *prefix) {

103 - struct bundle_header header;

103 + struct bundle_header header = BUNDLE_HEADER_INIT;

104 int bundle_fd = -1;

105 int quiet = 0;

106 int ret;

@@ -115,7 +115,6 @@ static int cmd_bundle_verify(int argc, const char **argv, const char *prefix) {

115 builtin_bundle_verify_usage, options, &bundle_file);

116 /* bundle internals use argv[1] as further parameters */

117

118 - memset(&header, 0, sizeof(header));

118 if ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0) {

119 ret = 1;

120 goto cleanup;

@@ -130,11 +129,12 @@ static int cmd_bundle_verify(int argc, const char **argv, const char *prefix) {

129 ret = 0;

130 cleanup:

131 free(bundle_file);

132 + bundle_header_release(&header);

133 return ret;

134 }

135

136 static int cmd_bundle_list_heads(int argc, const char **argv, const char *prefix) {

137 - struct bundle_header header;

137 + struct bundle_header header = BUNDLE_HEADER_INIT;

138 int bundle_fd = -1;

139 int ret;

140 struct option options[] = {

@@ -146,7 +146,6 @@ static int cmd_bundle_list_heads(int argc, const char **argv, const char *prefix

146 builtin_bundle_list_heads_usage, options, &bundle_file);

147 /* bundle internals use argv[1] as further parameters */

148

149 - memset(&header, 0, sizeof(header));

149 if ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0) {

150 ret = 1;

151 goto cleanup;

@@ -155,11 +154,12 @@ static int cmd_bundle_list_heads(int argc, const char **argv, const char *prefix

154 ret = !!list_bundle_refs(&header, argc, argv);

155 cleanup:

156 free(bundle_file);

157 + bundle_header_release(&header);

158 return ret;

159 }

160

161 static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix) {

162 - struct bundle_header header;

162 + struct bundle_header header = BUNDLE_HEADER_INIT;

163 int bundle_fd = -1;

164 int ret;

165 struct option options[] = {

@@ -171,7 +171,6 @@ static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix)

171 builtin_bundle_unbundle_usage, options, &bundle_file);

172 /* bundle internals use argv[1] as further parameters */

173

174 - memset(&header, 0, sizeof(header));

174 if ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0) {

175 ret = 1;

176 goto cleanup;

@@ -180,6 +179,7 @@ static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix)

179 die(_("Need a repository to unbundle."));

180 ret = !!unbundle(the_repository, &header, bundle_fd, 0) ||

181 list_bundle_refs(&header, argc, argv);

182 + bundle_header_release(&header);

183 cleanup:

184 free(bundle_file);

185 return ret;

bundle.c

+28 -24

@@ -23,13 +23,16 @@ static struct {

23 { 3, v3_bundle_signature },

24 };

26 -static void add_to_ref_list(const struct object_id *oid, const char *name,

27 - struct ref_list *list)

26 +void bundle_header_init(struct bundle_header *header)

27 {

29 - ALLOC_GROW(list->list, list->nr + 1, list->alloc);

30 - oidcpy(&list->list[list->nr].oid, oid);

31 - list->list[list->nr].name = xstrdup(name);

32 - list->nr++;

28 + struct bundle_header blank = BUNDLE_HEADER_INIT;

29 + memcpy(header, &blank, sizeof(*header));

30 +}

31 +

32 +void bundle_header_release(struct bundle_header *header)

33 +{

34 + string_list_clear(&header->prerequisites, 1);

35 + string_list_clear(&header->references, 1);

36 }

38 static int parse_capability(struct bundle_header *header, const char *capability)

@@ -112,10 +115,11 @@ static int parse_bundle_header(int fd, struct bundle_header *header,

115 status = -1;

116 break;

117 } else {

118 + struct object_id *dup = oiddup(&oid);

119 if (is_prereq)

116 - add_to_ref_list(&oid, "", &header->prerequisites);

120 + string_list_append(&header->prerequisites, "")->util = dup;

121 else

118 - add_to_ref_list(&oid, p + 1, &header->references);

122 + string_list_append(&header->references, p + 1)->util = dup;

123 }

124 }

125

@@ -139,19 +143,19 @@ int read_bundle_header(const char *path, struct bundle_header *header)

143

144 int is_bundle(const char *path, int quiet)

145 {

142 - struct bundle_header header;

146 + struct bundle_header header = BUNDLE_HEADER_INIT;

147 int fd = open(path, O_RDONLY);

148

149 if (fd < 0)

150 return 0;

147 - memset(&header, 0, sizeof(header));

151 fd = parse_bundle_header(fd, &header, quiet ? NULL : path);

152 if (fd >= 0)

153 close(fd);

154 + bundle_header_release(&header);

155 return (fd >= 0);

156 }

157

154 -static int list_refs(struct ref_list *r, int argc, const char **argv)

158 +static int list_refs(struct string_list *r, int argc, const char **argv)

159 {

160 int i;

161

@@ -162,14 +166,14 @@ static int list_refs(struct ref_list *r, int argc, const char **argv)

166 if (argc > 1) {

167 int j;

168 for (j = 1; j < argc; j++)

165 - if (!strcmp(r->list[i].name, argv[j]))

169 + if (!strcmp(r->items[i].string, argv[j]))

170 break;

171 if (j == argc)

172 continue;

173 }

174

171 - oid = &r->list[i].oid;

172 - name = r->list[i].name;

175 + oid = r->items[i].util;

176 + name = r->items[i].string;

177 printf("%s %s\n", oid_to_hex(oid), name);

178 }

179 return 0;

@@ -186,7 +190,7 @@ int verify_bundle(struct repository *r,

190 * Do fast check, then if any prereqs are missing then go line by line

191 * to be verbose about the errors

192 */

189 - struct ref_list *p = &header->prerequisites;

193 + struct string_list *p = &header->prerequisites;

194 struct rev_info revs;

195 const char *argv[] = {NULL, "--all", NULL};

196 struct commit *commit;

@@ -198,9 +202,9 @@ int verify_bundle(struct repository *r,

202

203 repo_init_revisions(r, &revs, NULL);

204 for (i = 0; i < p->nr; i++) {

201 - struct ref_list_entry *e = p->list + i;

202 - const char *name = e->name;

203 - struct object_id *oid = &e->oid;

205 + struct string_list_item *e = p->items + i;

206 + const char *name = e->string;

207 + struct object_id *oid = e->util;

208 struct object *o = parse_object(r, oid);

209 if (o) {

210 o->flags |= PREREQ_MARK;

@@ -225,9 +229,9 @@ int verify_bundle(struct repository *r,

229 i--;

230

231 for (i = 0; i < p->nr; i++) {

228 - struct ref_list_entry *e = p->list + i;

229 - const char *name = e->name;

230 - struct object_id *oid = &e->oid;

232 + struct string_list_item *e = p->items + i;

233 + const char *name = e->string;

234 + const struct object_id *oid = e->util;

235 struct object *o = parse_object(r, oid);

236 assert(o); /* otherwise we'd have returned early */

237 if (o->flags & SHOWN)

@@ -239,15 +243,15 @@ int verify_bundle(struct repository *r,

243

244 /* Clean up objects used, as they will be reused. */

245 for (i = 0; i < p->nr; i++) {

242 - struct ref_list_entry *e = p->list + i;

243 - struct object_id *oid = &e->oid;

246 + struct string_list_item *e = p->items + i;

247 + struct object_id *oid = e->util;

248 commit = lookup_commit_reference_gently(r, oid, 1);

249 if (commit)

250 clear_commit_marks(commit, ALL_REV_FLAGS);

251 }

252

253 if (verbose) {

250 - struct ref_list *r;

254 + struct string_list *r;

255

256 r = &header->references;

257 printf_ln(Q_("The bundle contains this ref:",

bundle.h

+11 -10

@@ -3,22 +3,23 @@

4 #include "strvec.h"

5 #include "cache.h"

6 -

7 -struct ref_list {

8 - unsigned int nr, alloc;

9 - struct ref_list_entry {

10 - struct object_id oid;

11 - char *name;

12 - } *list;

13 -};

6 +#include "string-list.h"

8 struct bundle_header {

9 unsigned version;

17 - struct ref_list prerequisites;

18 - struct ref_list references;

10 + struct string_list prerequisites;

11 + struct string_list references;

12 const struct git_hash_algo *hash_algo;

13 };

15 +#define BUNDLE_HEADER_INIT \

16 +{ \

17 + .prerequisites = STRING_LIST_INIT_DUP, \

18 + .references = STRING_LIST_INIT_DUP, \

19 +}

20 +void bundle_header_init(struct bundle_header *header);

21 +void bundle_header_release(struct bundle_header *header);

22 +

23 int is_bundle(const char *path, int quiet);

24 int read_bundle_header(const char *path, struct bundle_header *header);

25 int create_bundle(struct repository *r, const char *path,

transport.c

+5 -3

@@ -147,10 +147,10 @@ static struct ref *get_refs_from_bundle(struct transport *transport,

147 transport->hash_algo = data->header.hash_algo;

148

149 for (i = 0; i < data->header.references.nr; i++) {

150 - struct ref_list_entry *e = data->header.references.list + i;

151 - const char *name = e->name;

150 + struct string_list_item *e = data->header.references.items + i;

151 + const char *name = e->string;

152 struct ref *ref = alloc_ref(name);

153 - struct object_id *oid = &e->oid;

153 + struct object_id *oid = e->util;

154 oidcpy(&ref->old_oid, oid);

155 ref->next = result;

156 result = ref;

@@ -177,6 +177,7 @@ static int close_bundle(struct transport *transport)

177 struct bundle_transport_data *data = transport->data;

178 if (data->fd > 0)

179 close(data->fd);

180 + bundle_header_release(&data->header);

181 free(data);

182 return 0;

183 }

@@ -1083,6 +1084,7 @@ struct transport *transport_get(struct remote *remote, const char *url)

1084 die(_("git-over-rsync is no longer supported"));

1085 } else if (url_is_local_not_ssh(url) && is_file(url) && is_bundle(url, 1)) {

1086 struct bundle_transport_data *data = xcalloc(1, sizeof(*data));

1087 + bundle_header_init(&data->header);

1088 transport_check_allowed("file");

1089 ret->data = data;

1090 ret->vtable = &bundle_vtable;