Skip to content

Commit 642a400

Browse files
gitstergitster
committed
Merge branch 'ah/plugleaks'
Plug or annotate remaining leaks that trigger while running the very basic set of tests. * ah/plugleaks: transport: also free remote_refs in transport_disconnect() parse-options: don't leak alias help messages parse-options: convert bitfield values to use binary shift init-db: silence template_dir leak when converting to absolute path init: remove git_init_db_config() while fixing leaks worktree: fix leak in dwim_branch() clone: free or UNLEAK further pointers when finished reset: free instead of leaking unneeded ref symbolic-ref: don't leak shortened refname in check_symref()
2 parents 2e36527 + 68ffe09 commit 642a400

File tree

10 files changed

+74
-56
lines changed

10 files changed

+74
-56
lines changed

‎builtin/clone.c‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -964,10 +964,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
964964
{
965965
int is_bundle = 0, is_local;
966966
const char *repo_name, *repo, *work_tree, *git_dir;
967-
char *path, *dir, *display_repo = NULL;
967+
char *path = NULL, *dir, *display_repo = NULL;
968968
int dest_exists, real_dest_exists = 0;
969969
const struct ref *refs, *remote_head;
970-
const struct ref *remote_head_points_at;
970+
struct ref *remote_head_points_at = NULL;
971971
const struct ref *our_head_points_at;
972972
struct ref *mapped_refs;
973973
const struct ref *ref;
@@ -1017,9 +1017,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
10171017
repo_name = argv[0];
10181018

10191019
path = get_repo_path(repo_name, &is_bundle);
1020-
if (path)
1020+
if (path) {
1021+
FREE_AND_NULL(path);
10211022
repo = absolute_pathdup(repo_name);
1022-
else if (strchr(repo_name, ':')) {
1023+
} else if (strchr(repo_name, ':')) {
10231024
repo = repo_name;
10241025
display_repo = transport_anonymize_url(repo);
10251026
} else
@@ -1393,6 +1394,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
13931394
strbuf_release(&reflog_msg);
13941395
strbuf_release(&branch_top);
13951396
strbuf_release(&key);
1397+
free_refs(mapped_refs);
1398+
free_refs(remote_head_points_at);
1399+
free(dir);
1400+
free(path);
1401+
UNLEAK(repo);
13961402
junk_mode = JUNK_LEAVE_ALL;
13971403

13981404
strvec_clear(&transport_ls_refs_options.ref_prefixes);

‎builtin/init-db.c‎

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525

2626
static int init_is_bare_repository = 0;
2727
static int init_shared_repository = -1;
28-
static const char *init_db_template_dir;
2928

3029
static void copy_templates_1(struct strbuf *path, struct strbuf *template_path,
3130
DIR *dir)
@@ -94,7 +93,7 @@ static void copy_templates_1(struct strbuf *path, struct strbuf *template_path,
9493
}
9594
}
9695

97-
static void copy_templates(const char *template_dir)
96+
static void copy_templates(const char *template_dir, const char *init_template_dir)
9897
{
9998
struct strbuf path = STRBUF_INIT;
10099
struct strbuf template_path = STRBUF_INIT;
@@ -107,7 +106,7 @@ static void copy_templates(const char *template_dir)
107106
if (!template_dir)
108107
template_dir = getenv(TEMPLATE_DIR_ENVIRONMENT);
109108
if (!template_dir)
110-
template_dir = init_db_template_dir;
109+
template_dir = init_template_dir;
111110
if (!template_dir)
112111
template_dir = to_free = system_path(DEFAULT_GIT_TEMPLATE_DIR);
113112
if (!template_dir[0]) {
@@ -154,17 +153,6 @@ static void copy_templates(const char *template_dir)
154153
clear_repository_format(&template_format);
155154
}
156155

157-
static int git_init_db_config(const char *k, const char *v, void *cb)
158-
{
159-
if (!strcmp(k, "init.templatedir"))
160-
return git_config_pathname(&init_db_template_dir, k, v);
161-
162-
if (starts_with(k, "core."))
163-
return platform_core_config(k, v, cb);
164-
165-
return 0;
166-
}
167-
168156
/*
169157
* If the git_dir is not directly inside the working tree, then git will not
170158
* find it by default, and we need to set the worktree explicitly.
@@ -212,12 +200,9 @@ static int create_default_files(const char *template_path,
212200
int reinit;
213201
int filemode;
214202
struct strbuf err = STRBUF_INIT;
203+
const char *init_template_dir = NULL;
215204
const char *work_tree = get_git_work_tree();
216205

217-
/* Just look for `init.templatedir` */
218-
init_db_template_dir = NULL; /* re-set in case it was set before */
219-
git_config(git_init_db_config, NULL);
220-
221206
/*
222207
* First copy the templates -- we might have the default
223208
* config file there, in which case we would want to read
@@ -227,7 +212,8 @@ static int create_default_files(const char *template_path,
227212
* values (since we've just potentially changed what's available on
228213
* disk).
229214
*/
230-
copy_templates(template_path);
215+
git_config_get_value("init.templatedir", &init_template_dir);
216+
copy_templates(template_path, init_template_dir);
231217
git_config_clear();
232218
reset_shared_repository();
233219
git_config(git_default_config, NULL);
@@ -422,8 +408,8 @@ int init_db(const char *git_dir, const char *real_git_dir,
422408
}
423409
startup_info->have_repository = 1;
424410

425-
/* Just look for `core.hidedotfiles` */
426-
git_config(git_init_db_config, NULL);
411+
/* Ensure `core.hidedotfiles` is processed */
412+
git_config(platform_core_config, NULL);
427413

428414
safe_create_dir(git_dir, 0);
429415

@@ -575,8 +561,10 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
575561
if (real_git_dir && !is_absolute_path(real_git_dir))
576562
real_git_dir = real_pathdup(real_git_dir, 1);
577563

578-
if (template_dir && *template_dir && !is_absolute_path(template_dir))
564+
if (template_dir && *template_dir && !is_absolute_path(template_dir)) {
579565
template_dir = absolute_pathdup(template_dir);
566+
UNLEAK(template_dir);
567+
}
580568

581569
if (argc == 1) {
582570
int mkdir_tried = 0;

‎builtin/ls-remote.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
124124
int hash_algo = hash_algo_by_ptr(transport_get_hash_algo(transport));
125125
repo_set_hash_algo(the_repository, hash_algo);
126126
}
127-
if (transport_disconnect(transport))
128-
return 1;
129127

130128
if (!dest && !quiet)
131129
fprintf(stderr, "From %s\n", *remote->url);
@@ -151,5 +149,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
151149
}
152150

153151
ref_array_clear(&ref_array);
152+
if (transport_disconnect(transport))
153+
return 1;
154154
return status;
155155
}

‎builtin/remote.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -938,20 +938,19 @@ static int get_remote_ref_states(const char *name,
938938
struct ref_states *states,
939939
int query)
940940
{
941-
struct transport *transport;
942-
const struct ref *remote_refs;
943-
944941
states->remote = remote_get(name);
945942
if (!states->remote)
946943
return error(_("No such remote: '%s'"), name);
947944

948945
read_branches();
949946

950947
if (query) {
948+
struct transport *transport;
949+
const struct ref *remote_refs;
950+
951951
transport = transport_get(states->remote, states->remote->url_nr > 0 ?
952952
states->remote->url[0] : NULL);
953953
remote_refs = transport_get_remote_refs(transport, NULL);
954-
transport_disconnect(transport);
955954

956955
states->queried = 1;
957956
if (query & GET_REF_STATES)
@@ -960,6 +959,7 @@ static int get_remote_ref_states(const char *name,
960959
get_head_names(remote_refs, states);
961960
if (query & GET_PUSH_REF_STATES)
962961
get_push_ref_states(remote_refs, states);
962+
transport_disconnect(transport);
963963
} else {
964964
for_each_ref(append_ref_to_tracked_list, states);
965965
string_list_sort(&states->tracked);

‎builtin/reset.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
425425

426426
dwim_ref(rev, strlen(rev), &dummy, &ref, 0);
427427
if (ref && !starts_with(ref, "refs/"))
428-
ref = NULL;
428+
FREE_AND_NULL(ref);
429429

430430
err = reset_index(ref, &oid, reset_type, quiet);
431431
if (reset_type == KEEP && !err)

‎builtin/symbolic-ref.c‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,11 @@ static int check_symref(const char *HEAD, int quiet, int shorten, int print)
2424
return 1;
2525
}
2626
if (print) {
27+
char *to_free = NULL;
2728
if (shorten)
28-
refname = shorten_unambiguous_ref(refname, 0);
29+
refname = to_free = shorten_unambiguous_ref(refname, 0);
2930
puts(refname);
31+
free(to_free);
3032
}
3133
return 0;
3234
}

‎builtin/worktree.c‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -446,16 +446,18 @@ static void print_preparing_worktree_line(int detach,
446446
static const char *dwim_branch(const char *path, const char **new_branch)
447447
{
448448
int n;
449+
int branch_exists;
449450
const char *s = worktree_basename(path, &n);
450451
const char *branchname = xstrndup(s, n);
451452
struct strbuf ref = STRBUF_INIT;
452453

453454
UNLEAK(branchname);
454-
if (!strbuf_check_branch_ref(&ref, branchname) &&
455-
ref_exists(ref.buf)) {
456-
strbuf_release(&ref);
455+
456+
branch_exists = !strbuf_check_branch_ref(&ref, branchname) &&
457+
ref_exists(ref.buf);
458+
strbuf_release(&ref);
459+
if (branch_exists)
457460
return branchname;
458-
}
459461

460462
*new_branch = branchname;
461463
if (guess_remote) {

‎parse-options.c‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,8 @@ static int show_gitcomp(const struct option *opts, int show_all)
625625
*
626626
* Right now this is only used to preprocess and substitute
627627
* OPTION_ALIAS.
628+
*
629+
* The returned options should be freed using free_preprocessed_options.
628630
*/
629631
static struct option *preprocess_options(struct parse_opt_ctx_t *ctx,
630632
const struct option *options)
@@ -678,6 +680,7 @@ static struct option *preprocess_options(struct parse_opt_ctx_t *ctx,
678680
newopt[i].short_name = short_name;
679681
newopt[i].long_name = long_name;
680682
newopt[i].help = strbuf_detach(&help, NULL);
683+
newopt[i].flags |= PARSE_OPT_FROM_ALIAS;
681684
break;
682685
}
683686

@@ -693,6 +696,20 @@ static struct option *preprocess_options(struct parse_opt_ctx_t *ctx,
693696
return newopt;
694697
}
695698

699+
static void free_preprocessed_options(struct option *options)
700+
{
701+
int i;
702+
703+
if (!options)
704+
return;
705+
706+
for (i = 0; options[i].type != OPTION_END; i++) {
707+
if (options[i].flags & PARSE_OPT_FROM_ALIAS)
708+
free((void *)options[i].help);
709+
}
710+
free(options);
711+
}
712+
696713
static int usage_with_options_internal(struct parse_opt_ctx_t *,
697714
const char * const *,
698715
const struct option *, int, int);
@@ -870,7 +887,7 @@ int parse_options(int argc, const char **argv, const char *prefix,
870887
}
871888

872889
precompose_argv_prefix(argc, argv, NULL);
873-
free(real_options);
890+
free_preprocessed_options(real_options);
874891
free(ctx.alias_groups);
875892
return parse_options_end(&ctx);
876893
}

‎parse-options.h‎

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -28,26 +28,27 @@ enum parse_opt_type {
2828
};
2929

3030
enum parse_opt_flags {
31-
PARSE_OPT_KEEP_DASHDASH = 1,
32-
PARSE_OPT_STOP_AT_NON_OPTION = 2,
33-
PARSE_OPT_KEEP_ARGV0 = 4,
34-
PARSE_OPT_KEEP_UNKNOWN = 8,
35-
PARSE_OPT_NO_INTERNAL_HELP = 16,
36-
PARSE_OPT_ONE_SHOT = 32
31+
PARSE_OPT_KEEP_DASHDASH = 1 << 0,
32+
PARSE_OPT_STOP_AT_NON_OPTION = 1 << 1,
33+
PARSE_OPT_KEEP_ARGV0 = 1 << 2,
34+
PARSE_OPT_KEEP_UNKNOWN = 1 << 3,
35+
PARSE_OPT_NO_INTERNAL_HELP = 1 << 4,
36+
PARSE_OPT_ONE_SHOT = 1 << 5,
3737
};
3838

3939
enum parse_opt_option_flags {
40-
PARSE_OPT_OPTARG = 1,
41-
PARSE_OPT_NOARG = 2,
42-
PARSE_OPT_NONEG = 4,
43-
PARSE_OPT_HIDDEN = 8,
44-
PARSE_OPT_LASTARG_DEFAULT = 16,
45-
PARSE_OPT_NODASH = 32,
46-
PARSE_OPT_LITERAL_ARGHELP = 64,
47-
PARSE_OPT_SHELL_EVAL = 256,
48-
PARSE_OPT_NOCOMPLETE = 512,
49-
PARSE_OPT_COMP_ARG = 1024,
50-
PARSE_OPT_CMDMODE = 2048
40+
PARSE_OPT_OPTARG = 1 << 0,
41+
PARSE_OPT_NOARG = 1 << 1,
42+
PARSE_OPT_NONEG = 1 << 2,
43+
PARSE_OPT_HIDDEN = 1 << 3,
44+
PARSE_OPT_LASTARG_DEFAULT = 1 << 4,
45+
PARSE_OPT_NODASH = 1 << 5,
46+
PARSE_OPT_LITERAL_ARGHELP = 1 << 6,
47+
PARSE_OPT_FROM_ALIAS = 1 << 7,
48+
PARSE_OPT_SHELL_EVAL = 1 << 8,
49+
PARSE_OPT_NOCOMPLETE = 1 << 9,
50+
PARSE_OPT_COMP_ARG = 1 << 10,
51+
PARSE_OPT_CMDMODE = 1 << 11,
5152
};
5253

5354
enum parse_opt_result {

‎transport.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1452,6 +1452,8 @@ int transport_disconnect(struct transport *transport)
14521452
int ret = 0;
14531453
if (transport->vtable->disconnect)
14541454
ret = transport->vtable->disconnect(transport);
1455+
if (transport->got_remote_refs)
1456+
free_refs((void *)transport->remote_refs);
14551457
free(transport);
14561458
return ret;
14571459
}

0 commit comments

Comments
 (0)