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()
maint
Junio C Hamano 2021-04-07 16:54:08 -07:00
commit 642a40019c
10 changed files with 74 additions and 56 deletions

View File

@ -964,10 +964,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
{ {
int is_bundle = 0, is_local; int is_bundle = 0, is_local;
const char *repo_name, *repo, *work_tree, *git_dir; const char *repo_name, *repo, *work_tree, *git_dir;
char *path, *dir, *display_repo = NULL; char *path = NULL, *dir, *display_repo = NULL;
int dest_exists, real_dest_exists = 0; int dest_exists, real_dest_exists = 0;
const struct ref *refs, *remote_head; const struct ref *refs, *remote_head;
const struct ref *remote_head_points_at; struct ref *remote_head_points_at = NULL;
const struct ref *our_head_points_at; const struct ref *our_head_points_at;
struct ref *mapped_refs; struct ref *mapped_refs;
const struct ref *ref; const struct ref *ref;
@ -1017,9 +1017,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
repo_name = argv[0]; repo_name = argv[0];


path = get_repo_path(repo_name, &is_bundle); path = get_repo_path(repo_name, &is_bundle);
if (path) if (path) {
FREE_AND_NULL(path);
repo = absolute_pathdup(repo_name); repo = absolute_pathdup(repo_name);
else if (strchr(repo_name, ':')) { } else if (strchr(repo_name, ':')) {
repo = repo_name; repo = repo_name;
display_repo = transport_anonymize_url(repo); display_repo = transport_anonymize_url(repo);
} else } else
@ -1393,6 +1394,11 @@ cleanup:
strbuf_release(&reflog_msg); strbuf_release(&reflog_msg);
strbuf_release(&branch_top); strbuf_release(&branch_top);
strbuf_release(&key); strbuf_release(&key);
free_refs(mapped_refs);
free_refs(remote_head_points_at);
free(dir);
free(path);
UNLEAK(repo);
junk_mode = JUNK_LEAVE_ALL; junk_mode = JUNK_LEAVE_ALL;


strvec_clear(&transport_ls_refs_options.ref_prefixes); strvec_clear(&transport_ls_refs_options.ref_prefixes);

View File

@ -25,7 +25,6 @@


static int init_is_bare_repository = 0; static int init_is_bare_repository = 0;
static int init_shared_repository = -1; static int init_shared_repository = -1;
static const char *init_db_template_dir;


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


static void copy_templates(const char *template_dir) static void copy_templates(const char *template_dir, const char *init_template_dir)
{ {
struct strbuf path = STRBUF_INIT; struct strbuf path = STRBUF_INIT;
struct strbuf template_path = STRBUF_INIT; struct strbuf template_path = STRBUF_INIT;
@ -107,7 +106,7 @@ static void copy_templates(const char *template_dir)
if (!template_dir) if (!template_dir)
template_dir = getenv(TEMPLATE_DIR_ENVIRONMENT); template_dir = getenv(TEMPLATE_DIR_ENVIRONMENT);
if (!template_dir) if (!template_dir)
template_dir = init_db_template_dir; template_dir = init_template_dir;
if (!template_dir) if (!template_dir)
template_dir = to_free = system_path(DEFAULT_GIT_TEMPLATE_DIR); template_dir = to_free = system_path(DEFAULT_GIT_TEMPLATE_DIR);
if (!template_dir[0]) { if (!template_dir[0]) {
@ -154,17 +153,6 @@ free_return:
clear_repository_format(&template_format); clear_repository_format(&template_format);
} }


static int git_init_db_config(const char *k, const char *v, void *cb)
{
if (!strcmp(k, "init.templatedir"))
return git_config_pathname(&init_db_template_dir, k, v);

if (starts_with(k, "core."))
return platform_core_config(k, v, cb);

return 0;
}

/* /*
* If the git_dir is not directly inside the working tree, then git will not * If the git_dir is not directly inside the working tree, then git will not
* find it by default, and we need to set the worktree explicitly. * 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,
int reinit; int reinit;
int filemode; int filemode;
struct strbuf err = STRBUF_INIT; struct strbuf err = STRBUF_INIT;
const char *init_template_dir = NULL;
const char *work_tree = get_git_work_tree(); const char *work_tree = get_git_work_tree();


/* Just look for `init.templatedir` */
init_db_template_dir = NULL; /* re-set in case it was set before */
git_config(git_init_db_config, NULL);

/* /*
* First copy the templates -- we might have the default * First copy the templates -- we might have the default
* config file there, in which case we would want to read * config file there, in which case we would want to read
@ -227,7 +212,8 @@ static int create_default_files(const char *template_path,
* values (since we've just potentially changed what's available on * values (since we've just potentially changed what's available on
* disk). * disk).
*/ */
copy_templates(template_path); git_config_get_value("init.templatedir", &init_template_dir);
copy_templates(template_path, init_template_dir);
git_config_clear(); git_config_clear();
reset_shared_repository(); reset_shared_repository();
git_config(git_default_config, NULL); git_config(git_default_config, NULL);
@ -422,8 +408,8 @@ int init_db(const char *git_dir, const char *real_git_dir,
} }
startup_info->have_repository = 1; startup_info->have_repository = 1;


/* Just look for `core.hidedotfiles` */ /* Ensure `core.hidedotfiles` is processed */
git_config(git_init_db_config, NULL); git_config(platform_core_config, NULL);


safe_create_dir(git_dir, 0); safe_create_dir(git_dir, 0);


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


if (template_dir && *template_dir && !is_absolute_path(template_dir)) if (template_dir && *template_dir && !is_absolute_path(template_dir)) {
template_dir = absolute_pathdup(template_dir); template_dir = absolute_pathdup(template_dir);
UNLEAK(template_dir);
}


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

View File

@ -124,8 +124,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
int hash_algo = hash_algo_by_ptr(transport_get_hash_algo(transport)); int hash_algo = hash_algo_by_ptr(transport_get_hash_algo(transport));
repo_set_hash_algo(the_repository, hash_algo); repo_set_hash_algo(the_repository, hash_algo);
} }
if (transport_disconnect(transport))
return 1;


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


ref_array_clear(&ref_array); ref_array_clear(&ref_array);
if (transport_disconnect(transport))
return 1;
return status; return status;
} }

View File

@ -938,9 +938,6 @@ static int get_remote_ref_states(const char *name,
struct ref_states *states, struct ref_states *states,
int query) int query)
{ {
struct transport *transport;
const struct ref *remote_refs;

states->remote = remote_get(name); states->remote = remote_get(name);
if (!states->remote) if (!states->remote)
return error(_("No such remote: '%s'"), name); return error(_("No such remote: '%s'"), name);
@ -948,10 +945,12 @@ static int get_remote_ref_states(const char *name,
read_branches(); read_branches();


if (query) { if (query) {
struct transport *transport;
const struct ref *remote_refs;

transport = transport_get(states->remote, states->remote->url_nr > 0 ? transport = transport_get(states->remote, states->remote->url_nr > 0 ?
states->remote->url[0] : NULL); states->remote->url[0] : NULL);
remote_refs = transport_get_remote_refs(transport, NULL); remote_refs = transport_get_remote_refs(transport, NULL);
transport_disconnect(transport);


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

View File

@ -425,7 +425,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)


dwim_ref(rev, strlen(rev), &dummy, &ref, 0); dwim_ref(rev, strlen(rev), &dummy, &ref, 0);
if (ref && !starts_with(ref, "refs/")) if (ref && !starts_with(ref, "refs/"))
ref = NULL; FREE_AND_NULL(ref);


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

View File

@ -24,9 +24,11 @@ static int check_symref(const char *HEAD, int quiet, int shorten, int print)
return 1; return 1;
} }
if (print) { if (print) {
char *to_free = NULL;
if (shorten) if (shorten)
refname = shorten_unambiguous_ref(refname, 0); refname = to_free = shorten_unambiguous_ref(refname, 0);
puts(refname); puts(refname);
free(to_free);
} }
return 0; return 0;
} }

View File

@ -446,16 +446,18 @@ static void print_preparing_worktree_line(int detach,
static const char *dwim_branch(const char *path, const char **new_branch) static const char *dwim_branch(const char *path, const char **new_branch)
{ {
int n; int n;
int branch_exists;
const char *s = worktree_basename(path, &n); const char *s = worktree_basename(path, &n);
const char *branchname = xstrndup(s, n); const char *branchname = xstrndup(s, n);
struct strbuf ref = STRBUF_INIT; struct strbuf ref = STRBUF_INIT;


UNLEAK(branchname); UNLEAK(branchname);
if (!strbuf_check_branch_ref(&ref, branchname) &&
ref_exists(ref.buf)) { branch_exists = !strbuf_check_branch_ref(&ref, branchname) &&
strbuf_release(&ref); ref_exists(ref.buf);
strbuf_release(&ref);
if (branch_exists)
return branchname; return branchname;
}


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

View File

@ -625,6 +625,8 @@ static int show_gitcomp(const struct option *opts, int show_all)
* *
* Right now this is only used to preprocess and substitute * Right now this is only used to preprocess and substitute
* OPTION_ALIAS. * OPTION_ALIAS.
*
* The returned options should be freed using free_preprocessed_options.
*/ */
static struct option *preprocess_options(struct parse_opt_ctx_t *ctx, static struct option *preprocess_options(struct parse_opt_ctx_t *ctx,
const struct option *options) const struct option *options)
@ -678,6 +680,7 @@ static struct option *preprocess_options(struct parse_opt_ctx_t *ctx,
newopt[i].short_name = short_name; newopt[i].short_name = short_name;
newopt[i].long_name = long_name; newopt[i].long_name = long_name;
newopt[i].help = strbuf_detach(&help, NULL); newopt[i].help = strbuf_detach(&help, NULL);
newopt[i].flags |= PARSE_OPT_FROM_ALIAS;
break; break;
} }


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


static void free_preprocessed_options(struct option *options)
{
int i;

if (!options)
return;

for (i = 0; options[i].type != OPTION_END; i++) {
if (options[i].flags & PARSE_OPT_FROM_ALIAS)
free((void *)options[i].help);
}
free(options);
}

static int usage_with_options_internal(struct parse_opt_ctx_t *, static int usage_with_options_internal(struct parse_opt_ctx_t *,
const char * const *, const char * const *,
const struct option *, int, int); const struct option *, int, int);
@ -870,7 +887,7 @@ int parse_options(int argc, const char **argv, const char *prefix,
} }


precompose_argv_prefix(argc, argv, NULL); precompose_argv_prefix(argc, argv, NULL);
free(real_options); free_preprocessed_options(real_options);
free(ctx.alias_groups); free(ctx.alias_groups);
return parse_options_end(&ctx); return parse_options_end(&ctx);
} }

View File

@ -28,26 +28,27 @@ enum parse_opt_type {
}; };


enum parse_opt_flags { enum parse_opt_flags {
PARSE_OPT_KEEP_DASHDASH = 1, PARSE_OPT_KEEP_DASHDASH = 1 << 0,
PARSE_OPT_STOP_AT_NON_OPTION = 2, PARSE_OPT_STOP_AT_NON_OPTION = 1 << 1,
PARSE_OPT_KEEP_ARGV0 = 4, PARSE_OPT_KEEP_ARGV0 = 1 << 2,
PARSE_OPT_KEEP_UNKNOWN = 8, PARSE_OPT_KEEP_UNKNOWN = 1 << 3,
PARSE_OPT_NO_INTERNAL_HELP = 16, PARSE_OPT_NO_INTERNAL_HELP = 1 << 4,
PARSE_OPT_ONE_SHOT = 32 PARSE_OPT_ONE_SHOT = 1 << 5,
}; };


enum parse_opt_option_flags { enum parse_opt_option_flags {
PARSE_OPT_OPTARG = 1, PARSE_OPT_OPTARG = 1 << 0,
PARSE_OPT_NOARG = 2, PARSE_OPT_NOARG = 1 << 1,
PARSE_OPT_NONEG = 4, PARSE_OPT_NONEG = 1 << 2,
PARSE_OPT_HIDDEN = 8, PARSE_OPT_HIDDEN = 1 << 3,
PARSE_OPT_LASTARG_DEFAULT = 16, PARSE_OPT_LASTARG_DEFAULT = 1 << 4,
PARSE_OPT_NODASH = 32, PARSE_OPT_NODASH = 1 << 5,
PARSE_OPT_LITERAL_ARGHELP = 64, PARSE_OPT_LITERAL_ARGHELP = 1 << 6,
PARSE_OPT_SHELL_EVAL = 256, PARSE_OPT_FROM_ALIAS = 1 << 7,
PARSE_OPT_NOCOMPLETE = 512, PARSE_OPT_SHELL_EVAL = 1 << 8,
PARSE_OPT_COMP_ARG = 1024, PARSE_OPT_NOCOMPLETE = 1 << 9,
PARSE_OPT_CMDMODE = 2048 PARSE_OPT_COMP_ARG = 1 << 10,
PARSE_OPT_CMDMODE = 1 << 11,
}; };


enum parse_opt_result { enum parse_opt_result {

View File

@ -1452,6 +1452,8 @@ int transport_disconnect(struct transport *transport)
int ret = 0; int ret = 0;
if (transport->vtable->disconnect) if (transport->vtable->disconnect)
ret = transport->vtable->disconnect(transport); ret = transport->vtable->disconnect(transport);
if (transport->got_remote_refs)
free_refs((void *)transport->remote_refs);
free(transport); free(transport);
return ret; return ret;
} }