Skip to content

Commit cdeabf5

Browse files
gitstergitster
committed
Merge branch 'ab/bundle-updates'
Code clean-up and leak plugging in "git bundle". * ab/bundle-updates: bundle: remove "ref_list" in favor of string-list.c API bundle.c: use a temporary variable for OIDs and names bundle cmd: stop leaking memory from parse_options_cmd_bundle()
2 parents f0ade78 + 10b635b commit cdeabf5

File tree

4 files changed

+104
-65
lines changed

4 files changed

+104
-65
lines changed

‎builtin/bundle.c

Lines changed: 47 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ static int parse_options_cmd_bundle(int argc,
4646
const char* prefix,
4747
const char * const usagestr[],
4848
const struct option options[],
49-
const char **bundle_file) {
49+
char **bundle_file) {
5050
int newargc;
5151
newargc = parse_options(argc, argv, NULL, options, usagestr,
5252
PARSE_OPT_STOP_AT_NON_OPTION);
@@ -61,7 +61,7 @@ static int cmd_bundle_create(int argc, const char **argv, const char *prefix) {
6161
int progress = isatty(STDERR_FILENO);
6262
struct strvec pack_opts;
6363
int version = -1;
64-
64+
int ret;
6565
struct option options[] = {
6666
OPT_SET_INT('q', "quiet", &progress,
6767
N_("do not show progress meter"), 0),
@@ -76,7 +76,7 @@ static int cmd_bundle_create(int argc, const char **argv, const char *prefix) {
7676
N_("specify bundle format version")),
7777
OPT_END()
7878
};
79-
const char* bundle_file;
79+
char *bundle_file;
8080

8181
argc = parse_options_cmd_bundle(argc, argv, prefix,
8282
builtin_bundle_create_usage, options, &bundle_file);
@@ -94,75 +94,95 @@ static int cmd_bundle_create(int argc, const char **argv, const char *prefix) {
9494

9595
if (!startup_info->have_repository)
9696
die(_("Need a repository to create a bundle."));
97-
return !!create_bundle(the_repository, bundle_file, argc, argv, &pack_opts, version);
97+
ret = !!create_bundle(the_repository, bundle_file, argc, argv, &pack_opts, version);
98+
free(bundle_file);
99+
return ret;
98100
}
99101

100102
static int cmd_bundle_verify(int argc, const char **argv, const char *prefix) {
101-
struct bundle_header header;
103+
struct bundle_header header = BUNDLE_HEADER_INIT;
102104
int bundle_fd = -1;
103105
int quiet = 0;
104-
106+
int ret;
105107
struct option options[] = {
106108
OPT_BOOL('q', "quiet", &quiet,
107109
N_("do not show bundle details")),
108110
OPT_END()
109111
};
110-
const char* bundle_file;
112+
char *bundle_file;
111113

112114
argc = parse_options_cmd_bundle(argc, argv, prefix,
113115
builtin_bundle_verify_usage, options, &bundle_file);
114116
/* bundle internals use argv[1] as further parameters */
115117

116-
memset(&header, 0, sizeof(header));
117-
if ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0)
118-
return 1;
118+
if ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0) {
119+
ret = 1;
120+
goto cleanup;
121+
}
119122
close(bundle_fd);
120-
if (verify_bundle(the_repository, &header, !quiet))
121-
return 1;
123+
if (verify_bundle(the_repository, &header, !quiet)) {
124+
ret = 1;
125+
goto cleanup;
126+
}
127+
122128
fprintf(stderr, _("%s is okay\n"), bundle_file);
123-
return 0;
129+
ret = 0;
130+
cleanup:
131+
free(bundle_file);
132+
bundle_header_release(&header);
133+
return ret;
124134
}
125135

126136
static int cmd_bundle_list_heads(int argc, const char **argv, const char *prefix) {
127-
struct bundle_header header;
137+
struct bundle_header header = BUNDLE_HEADER_INIT;
128138
int bundle_fd = -1;
129-
139+
int ret;
130140
struct option options[] = {
131141
OPT_END()
132142
};
133-
const char* bundle_file;
143+
char *bundle_file;
134144

135145
argc = parse_options_cmd_bundle(argc, argv, prefix,
136146
builtin_bundle_list_heads_usage, options, &bundle_file);
137147
/* bundle internals use argv[1] as further parameters */
138148

139-
memset(&header, 0, sizeof(header));
140-
if ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0)
141-
return 1;
149+
if ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0) {
150+
ret = 1;
151+
goto cleanup;
152+
}
142153
close(bundle_fd);
143-
return !!list_bundle_refs(&header, argc, argv);
154+
ret = !!list_bundle_refs(&header, argc, argv);
155+
cleanup:
156+
free(bundle_file);
157+
bundle_header_release(&header);
158+
return ret;
144159
}
145160

146161
static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix) {
147-
struct bundle_header header;
162+
struct bundle_header header = BUNDLE_HEADER_INIT;
148163
int bundle_fd = -1;
149-
164+
int ret;
150165
struct option options[] = {
151166
OPT_END()
152167
};
153-
const char* bundle_file;
168+
char *bundle_file;
154169

155170
argc = parse_options_cmd_bundle(argc, argv, prefix,
156171
builtin_bundle_unbundle_usage, options, &bundle_file);
157172
/* bundle internals use argv[1] as further parameters */
158173

159-
memset(&header, 0, sizeof(header));
160-
if ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0)
161-
return 1;
174+
if ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0) {
175+
ret = 1;
176+
goto cleanup;
177+
}
162178
if (!startup_info->have_repository)
163179
die(_("Need a repository to unbundle."));
164-
return !!unbundle(the_repository, &header, bundle_fd, 0) ||
180+
ret = !!unbundle(the_repository, &header, bundle_fd, 0) ||
165181
list_bundle_refs(&header, argc, argv);
182+
bundle_header_release(&header);
183+
cleanup:
184+
free(bundle_file);
185+
return ret;
166186
}
167187

168188
int cmd_bundle(int argc, const char **argv, const char *prefix)

‎bundle.c

Lines changed: 39 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,16 @@ static struct {
2323
{ 3, v3_bundle_signature },
2424
};
2525

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)
2827
{
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);
3336
}
3437

3538
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,
112115
status = -1;
113116
break;
114117
} else {
118+
struct object_id *dup = oiddup(&oid);
115119
if (is_prereq)
116-
add_to_ref_list(&oid, "", &header->prerequisites);
120+
string_list_append(&header->prerequisites, "")->util = dup;
117121
else
118-
add_to_ref_list(&oid, p + 1, &header->references);
122+
string_list_append(&header->references, p + 1)->util = dup;
119123
}
120124
}
121125

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

140144
int is_bundle(const char *path, int quiet)
141145
{
142-
struct bundle_header header;
146+
struct bundle_header header = BUNDLE_HEADER_INIT;
143147
int fd = open(path, O_RDONLY);
144148

145149
if (fd < 0)
146150
return 0;
147-
memset(&header, 0, sizeof(header));
148151
fd = parse_bundle_header(fd, &header, quiet ? NULL : path);
149152
if (fd >= 0)
150153
close(fd);
154+
bundle_header_release(&header);
151155
return (fd >= 0);
152156
}
153157

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)
155159
{
156160
int i;
157161

158162
for (i = 0; i < r->nr; i++) {
163+
struct object_id *oid;
164+
const char *name;
165+
159166
if (argc > 1) {
160167
int j;
161168
for (j = 1; j < argc; j++)
162-
if (!strcmp(r->list[i].name, argv[j]))
169+
if (!strcmp(r->items[i].string, argv[j]))
163170
break;
164171
if (j == argc)
165172
continue;
166173
}
167-
printf("%s %s\n", oid_to_hex(&r->list[i].oid),
168-
r->list[i].name);
174+
175+
oid = r->items[i].util;
176+
name = r->items[i].string;
177+
printf("%s %s\n", oid_to_hex(oid), name);
169178
}
170179
return 0;
171180
}
@@ -181,7 +190,7 @@ int verify_bundle(struct repository *r,
181190
* Do fast check, then if any prereqs are missing then go line by line
182191
* to be verbose about the errors
183192
*/
184-
struct ref_list *p = &header->prerequisites;
193+
struct string_list *p = &header->prerequisites;
185194
struct rev_info revs;
186195
const char *argv[] = {NULL, "--all", NULL};
187196
struct commit *commit;
@@ -193,16 +202,18 @@ int verify_bundle(struct repository *r,
193202

194203
repo_init_revisions(r, &revs, NULL);
195204
for (i = 0; i < p->nr; i++) {
196-
struct ref_list_entry *e = p->list + i;
197-
struct object *o = parse_object(r, &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);
198209
if (o) {
199210
o->flags |= PREREQ_MARK;
200-
add_pending_object(&revs, o, e->name);
211+
add_pending_object(&revs, o, name);
201212
continue;
202213
}
203214
if (++ret == 1)
204215
error("%s", message);
205-
error("%s %s", oid_to_hex(&e->oid), e->name);
216+
error("%s %s", oid_to_hex(oid), name);
206217
}
207218
if (revs.pending.nr != p->nr)
208219
return ret;
@@ -218,26 +229,29 @@ int verify_bundle(struct repository *r,
218229
i--;
219230

220231
for (i = 0; i < p->nr; i++) {
221-
struct ref_list_entry *e = p->list + i;
222-
struct object *o = parse_object(r, &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);
223236
assert(o); /* otherwise we'd have returned early */
224237
if (o->flags & SHOWN)
225238
continue;
226239
if (++ret == 1)
227240
error("%s", message);
228-
error("%s %s", oid_to_hex(&e->oid), e->name);
241+
error("%s %s", oid_to_hex(oid), name);
229242
}
230243

231244
/* Clean up objects used, as they will be reused. */
232245
for (i = 0; i < p->nr; i++) {
233-
struct ref_list_entry *e = p->list + i;
234-
commit = lookup_commit_reference_gently(r, &e->oid, 1);
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);
235249
if (commit)
236250
clear_commit_marks(commit, ALL_REV_FLAGS);
237251
}
238252

239253
if (verbose) {
240-
struct ref_list *r;
254+
struct string_list *r;
241255

242256
r = &header->references;
243257
printf_ln(Q_("The bundle contains this ref:",

‎bundle.h

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,23 @@
33

44
#include "strvec.h"
55
#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"
147

158
struct bundle_header {
169
unsigned version;
17-
struct ref_list prerequisites;
18-
struct ref_list references;
10+
struct string_list prerequisites;
11+
struct string_list references;
1912
const struct git_hash_algo *hash_algo;
2013
};
2114

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+
2223
int is_bundle(const char *path, int quiet);
2324
int read_bundle_header(const char *path, struct bundle_header *header);
2425
int create_bundle(struct repository *r, const char *path,

‎transport.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,9 +147,11 @@ static struct ref *get_refs_from_bundle(struct transport *transport,
147147
transport->hash_algo = data->header.hash_algo;
148148

149149
for (i = 0; i < data->header.references.nr; i++) {
150-
struct ref_list_entry *e = data->header.references.list + i;
151-
struct ref *ref = alloc_ref(e->name);
152-
oidcpy(&ref->old_oid, &e->oid);
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->util;
154+
oidcpy(&ref->old_oid, oid);
153155
ref->next = result;
154156
result = ref;
155157
}
@@ -175,6 +177,7 @@ static int close_bundle(struct transport *transport)
175177
struct bundle_transport_data *data = transport->data;
176178
if (data->fd > 0)
177179
close(data->fd);
180+
bundle_header_release(&data->header);
178181
free(data);
179182
return 0;
180183
}
@@ -1081,6 +1084,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
10811084
die(_("git-over-rsync is no longer supported"));
10821085
} else if (url_is_local_not_ssh(url) && is_file(url) && is_bundle(url, 1)) {
10831086
struct bundle_transport_data *data = xcalloc(1, sizeof(*data));
1087+
bundle_header_init(&data->header);
10841088
transport_check_allowed("file");
10851089
ret->data = data;
10861090
ret->vtable = &bundle_vtable;

0 commit comments

Comments
 (0)