Skip to content

Commit 849e671

Browse files
gitstergitster
committed
Merge branch 'js/plug-leaks'
Fix memory leaks pointed out by Coverity (and people). * js/plug-leaks: (26 commits) checkout: fix memory leak submodule_uses_worktrees(): plug memory leak show_worktree(): plug memory leak name-rev: avoid leaking memory in the `deref` case remote: plug memory leak in match_explicit() add_reflog_for_walk: avoid memory leak shallow: avoid memory leak line-log: avoid memory leak receive-pack: plug memory leak in update() fast-export: avoid leaking memory in handle_tag() mktree: plug memory leaks reported by Coverity pack-redundant: plug memory leak setup_discovered_git_dir(): plug memory leak setup_bare_git_dir(): help static analysis split_commit_in_progress(): simplify & fix memory leak checkout: fix memory leak cat-file: fix memory leak mailinfo & mailsplit: check for EOF while parsing status: close file descriptor after reading git-rebase-todo difftool: address a couple of resource/memory leaks ...
2 parents 137a261 + 443a12f commit 849e671

23 files changed

+149
-64
lines changed

‎builtin/am.c‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1351,19 +1351,16 @@ static int get_mail_commit_oid(struct object_id *commit_id, const char *mail)
13511351
struct strbuf sb = STRBUF_INIT;
13521352
FILE *fp = xfopen(mail, "r");
13531353
const char *x;
1354+
int ret = 0;
13541355

1355-
if (strbuf_getline_lf(&sb, fp))
1356-
return -1;
1357-
1358-
if (!skip_prefix(sb.buf, "From ", &x))
1359-
return -1;
1360-
1361-
if (get_oid_hex(x, commit_id) < 0)
1362-
return -1;
1356+
if (strbuf_getline_lf(&sb, fp) ||
1357+
!skip_prefix(sb.buf, "From ", &x) ||
1358+
get_oid_hex(x, commit_id) < 0)
1359+
ret = -1;
13631360

13641361
strbuf_release(&sb);
13651362
fclose(fp);
1366-
return 0;
1363+
return ret;
13671364
}
13681365

13691366
/**

‎builtin/cat-file.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
165165
die("git cat-file %s: bad file", obj_name);
166166

167167
write_or_die(1, buf, size);
168+
free(buf);
168169
return 0;
169170
}
170171

‎builtin/checkout.c‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -235,22 +235,24 @@ static int checkout_merged(int pos, const struct checkout *state)
235235
/*
236236
* NEEDSWORK:
237237
* There is absolutely no reason to write this as a blob object
238-
* and create a phony cache entry just to leak. This hack is
239-
* primarily to get to the write_entry() machinery that massages
240-
* the contents to work-tree format and writes out which only
241-
* allows it for a cache entry. The code in write_entry() needs
242-
* to be refactored to allow us to feed a <buffer, size, mode>
243-
* instead of a cache entry. Such a refactoring would help
244-
* merge_recursive as well (it also writes the merge result to the
245-
* object database even when it may contain conflicts).
238+
* and create a phony cache entry. This hack is primarily to get
239+
* to the write_entry() machinery that massages the contents to
240+
* work-tree format and writes out which only allows it for a
241+
* cache entry. The code in write_entry() needs to be refactored
242+
* to allow us to feed a <buffer, size, mode> instead of a cache
243+
* entry. Such a refactoring would help merge_recursive as well
244+
* (it also writes the merge result to the object database even
245+
* when it may contain conflicts).
246246
*/
247247
if (write_sha1_file(result_buf.ptr, result_buf.size,
248248
blob_type, oid.hash))
249249
die(_("Unable to add merge result for '%s'"), path);
250+
free(result_buf.ptr);
250251
ce = make_cache_entry(mode, oid.hash, path, 2, 0);
251252
if (!ce)
252253
die(_("make_cache_entry failed for path '%s'"), path);
253254
status = checkout_entry(ce, state, NULL);
255+
free(ce);
254256
return status;
255257
}
256258

‎builtin/difftool.c‎

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,7 @@ static void changed_files(struct hashmap *result, const char *index_path,
226226
hashmap_entry_init(entry, strhash(buf.buf));
227227
hashmap_add(result, entry);
228228
}
229+
fclose(fp);
229230
if (finish_command(&diff_files))
230231
die("diff-files did not exit properly");
231232
strbuf_release(&index_env);
@@ -439,8 +440,10 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
439440
}
440441

441442
if (lmode && status != 'C') {
442-
if (checkout_path(lmode, &loid, src_path, &lstate))
443-
return error("could not write '%s'", src_path);
443+
if (checkout_path(lmode, &loid, src_path, &lstate)) {
444+
ret = error("could not write '%s'", src_path);
445+
goto finish;
446+
}
444447
}
445448

446449
if (rmode && !S_ISLNK(rmode)) {
@@ -456,9 +459,12 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
456459
hashmap_add(&working_tree_dups, entry);
457460

458461
if (!use_wt_file(workdir, dst_path, &roid)) {
459-
if (checkout_path(rmode, &roid, dst_path, &rstate))
460-
return error("could not write '%s'",
461-
dst_path);
462+
if (checkout_path(rmode, &roid, dst_path,
463+
&rstate)) {
464+
ret = error("could not write '%s'",
465+
dst_path);
466+
goto finish;
467+
}
462468
} else if (!is_null_oid(&roid)) {
463469
/*
464470
* Changes in the working tree need special
@@ -473,10 +479,12 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
473479
ADD_CACHE_JUST_APPEND);
474480

475481
add_path(&rdir, rdir_len, dst_path);
476-
if (ensure_leading_directories(rdir.buf))
477-
return error("could not create "
478-
"directory for '%s'",
479-
dst_path);
482+
if (ensure_leading_directories(rdir.buf)) {
483+
ret = error("could not create "
484+
"directory for '%s'",
485+
dst_path);
486+
goto finish;
487+
}
480488
add_path(&wtdir, wtdir_len, dst_path);
481489
if (symlinks) {
482490
if (symlink(wtdir.buf, rdir.buf)) {
@@ -497,13 +505,15 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
497505
}
498506
}
499507

508+
fclose(fp);
509+
fp = NULL;
500510
if (finish_command(&child)) {
501511
ret = error("error occurred running diff --raw");
502512
goto finish;
503513
}
504514

505515
if (!i)
506-
return 0;
516+
goto finish;
507517

508518
/*
509519
* Changes to submodules require special treatment.This loop writes a
@@ -626,6 +636,9 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
626636
exit_cleanup(tmpdir, rc);
627637

628638
finish:
639+
if (fp)
640+
fclose(fp);
641+
629642
free(lbase_dir);
630643
free(rbase_dir);
631644
strbuf_release(&ldir);

‎builtin/fast-export.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -734,6 +734,7 @@ static void handle_tag(const char *name, struct tag *tag)
734734
oid_to_hex(&tag->object.oid));
735735
case DROP:
736736
/* Ignore this tag altogether */
737+
free(buf);
737738
return;
738739
case REWRITE:
739740
if (tagged->type != OBJ_COMMIT) {
@@ -765,6 +766,7 @@ static void handle_tag(const char *name, struct tag *tag)
765766
(int)(tagger_end - tagger), tagger,
766767
tagger == tagger_end ? "" : "\n",
767768
(int)message_size, (int)message_size, message ? message : "");
769+
free(buf);
768770
}
769771

770772
static struct commit *get_commit(struct rev_cmdline_entry *e, char *full_name)

‎builtin/mailsplit.c‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,16 @@ static int split_mbox(const char *file, const char *dir, int allow_bare,
232232

233233
do {
234234
peek = fgetc(f);
235+
if (peek == EOF) {
236+
if (f == stdin)
237+
/* empty stdin is OK */
238+
ret = skip;
239+
else {
240+
fclose(f);
241+
error(_("empty mbox: '%s'"), file);
242+
}
243+
goto out;
244+
}
235245
} while (isspace(peek));
236246
ungetc(peek, f);
237247

‎builtin/mktree.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ static void mktree_line(char *buf, size_t len, int nul_term_line, int allow_miss
7272
unsigned mode;
7373
enum object_type mode_type; /* object type derived from mode */
7474
enum object_type obj_type; /* object type derived from sha */
75-
char *path;
75+
char *path, *to_free = NULL;
7676
unsigned char sha1[20];
7777

7878
ptr = buf;
@@ -102,7 +102,7 @@ static void mktree_line(char *buf, size_t len, int nul_term_line, int allow_miss
102102
struct strbuf p_uq = STRBUF_INIT;
103103
if (unquote_c_style(&p_uq, path, NULL))
104104
die("invalid quoting");
105-
path = strbuf_detach(&p_uq, NULL);
105+
path = to_free = strbuf_detach(&p_uq, NULL);
106106
}
107107

108108
/*
@@ -136,6 +136,7 @@ static void mktree_line(char *buf, size_t len, int nul_term_line, int allow_miss
136136
}
137137

138138
append_to_tree(mode, sha1, path);
139+
free(to_free);
139140
}
140141

141142
int cmd_mktree(int ac, const char **av, const char *prefix)

‎builtin/name-rev.c‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,15 @@ static void name_rev(struct commit *commit,
2828
struct rev_name *name = (struct rev_name *)commit->util;
2929
struct commit_list *parents;
3030
int parent_number = 1;
31+
char *to_free = NULL;
3132

3233
parse_commit(commit);
3334

3435
if (commit->date < cutoff)
3536
return;
3637

3738
if (deref) {
38-
tip_name = xstrfmt("%s^0", tip_name);
39+
tip_name = to_free = xstrfmt("%s^0", tip_name);
3940

4041
if (generation)
4142
die("generation: %d, but deref?", generation);
@@ -53,8 +54,10 @@ static void name_rev(struct commit *commit,
5354
name->taggerdate = taggerdate;
5455
name->generation = generation;
5556
name->distance = distance;
56-
} else
57+
} else {
58+
free(to_free);
5759
return;
60+
}
5861

5962
for (parents = commit->parents;
6063
parents;

‎builtin/pack-redundant.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,7 @@ static void minimize(struct pack_list **min)
442442
/* return if there are no objects missing from the unique set */
443443
if (missing->size == 0) {
444444
*min = unique;
445+
free(missing);
445446
return;
446447
}
447448

‎builtin/receive-pack.c‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1028,7 +1028,8 @@ static const char *update(struct command *cmd, struct shallow_info *si)
10281028
{
10291029
const char *name = cmd->ref_name;
10301030
struct strbuf namespaced_name_buf = STRBUF_INIT;
1031-
const char *namespaced_name, *ret;
1031+
static char *namespaced_name;
1032+
const char *ret;
10321033
struct object_id *old_oid = &cmd->old_oid;
10331034
struct object_id *new_oid = &cmd->new_oid;
10341035

@@ -1039,6 +1040,7 @@ static const char *update(struct command *cmd, struct shallow_info *si)
10391040
}
10401041

10411042
strbuf_addf(&namespaced_name_buf, "%s%s", get_git_namespace(), name);
1043+
free(namespaced_name);
10421044
namespaced_name = strbuf_detach(&namespaced_name_buf, NULL);
10431045

10441046
if (is_ref_checked_out(namespaced_name)) {

0 commit comments

Comments
 (0)