Skip to content

Commit

Permalink
Merge branch 'ah/plugleaks'
Browse files Browse the repository at this point in the history
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()
  • Loading branch information
gitster committed Apr 7, 2021
2 parents 2e36527 + 68ffe09 commit 642a400
Show file tree
Hide file tree
Showing 10 changed files with 74 additions and 56 deletions.
14 changes: 10 additions & 4 deletions builtin/clone.c
Original file line number Diff line number Diff line change
Expand Up @@ -964,10 +964,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
{
int is_bundle = 0, is_local;
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;
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;
struct ref *mapped_refs;
const struct ref *ref;
Expand Down Expand Up @@ -1017,9 +1017,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
repo_name = argv[0];

path = get_repo_path(repo_name, &is_bundle);
if (path)
if (path) {
FREE_AND_NULL(path);
repo = absolute_pathdup(repo_name);
else if (strchr(repo_name, ':')) {
} else if (strchr(repo_name, ':')) {
repo = repo_name;
display_repo = transport_anonymize_url(repo);
} else
Expand Down Expand Up @@ -1393,6 +1394,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
strbuf_release(&reflog_msg);
strbuf_release(&branch_top);
strbuf_release(&key);
free_refs(mapped_refs);
free_refs(remote_head_points_at);
free(dir);
free(path);
UNLEAK(repo);
junk_mode = JUNK_LEAVE_ALL;

strvec_clear(&transport_ls_refs_options.ref_prefixes);
Expand Down
32 changes: 10 additions & 22 deletions builtin/init-db.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@

static int init_is_bare_repository = 0;
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,
DIR *dir)
Expand Down Expand Up @@ -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 template_path = STRBUF_INIT;
Expand All @@ -107,7 +106,7 @@ static void copy_templates(const char *template_dir)
if (!template_dir)
template_dir = getenv(TEMPLATE_DIR_ENVIRONMENT);
if (!template_dir)
template_dir = init_db_template_dir;
template_dir = init_template_dir;
if (!template_dir)
template_dir = to_free = system_path(DEFAULT_GIT_TEMPLATE_DIR);
if (!template_dir[0]) {
Expand Down Expand Up @@ -154,17 +153,6 @@ static void copy_templates(const char *template_dir)
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
* find it by default, and we need to set the worktree explicitly.
Expand Down Expand Up @@ -212,12 +200,9 @@ static int create_default_files(const char *template_path,
int reinit;
int filemode;
struct strbuf err = STRBUF_INIT;
const char *init_template_dir = NULL;
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
* config file there, in which case we would want to read
Expand All @@ -227,7 +212,8 @@ static int create_default_files(const char *template_path,
* values (since we've just potentially changed what's available on
* disk).
*/
copy_templates(template_path);
git_config_get_value("init.templatedir", &init_template_dir);
copy_templates(template_path, init_template_dir);
git_config_clear();
reset_shared_repository();
git_config(git_default_config, NULL);
Expand Down Expand Up @@ -422,8 +408,8 @@ int init_db(const char *git_dir, const char *real_git_dir,
}
startup_info->have_repository = 1;

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

safe_create_dir(git_dir, 0);

Expand Down Expand Up @@ -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))
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);
UNLEAK(template_dir);
}

if (argc == 1) {
int mkdir_tried = 0;
Expand Down
4 changes: 2 additions & 2 deletions builtin/ls-remote.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));
repo_set_hash_algo(the_repository, hash_algo);
}
if (transport_disconnect(transport))
return 1;

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

ref_array_clear(&ref_array);
if (transport_disconnect(transport))
return 1;
return status;
}
8 changes: 4 additions & 4 deletions builtin/remote.c
Original file line number Diff line number Diff line change
Expand Up @@ -938,20 +938,19 @@ static int get_remote_ref_states(const char *name,
struct ref_states *states,
int query)
{
struct transport *transport;
const struct ref *remote_refs;

states->remote = remote_get(name);
if (!states->remote)
return error(_("No such remote: '%s'"), name);

read_branches();

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

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

states->queried = 1;
if (query & GET_REF_STATES)
Expand All @@ -960,6 +959,7 @@ static int get_remote_ref_states(const char *name,
get_head_names(remote_refs, states);
if (query & GET_PUSH_REF_STATES)
get_push_ref_states(remote_refs, states);
transport_disconnect(transport);
} else {
for_each_ref(append_ref_to_tracked_list, states);
string_list_sort(&states->tracked);
Expand Down
2 changes: 1 addition & 1 deletion builtin/reset.c
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)

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

err = reset_index(ref, &oid, reset_type, quiet);
if (reset_type == KEEP && !err)
Expand Down
4 changes: 3 additions & 1 deletion builtin/symbolic-ref.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,11 @@ static int check_symref(const char *HEAD, int quiet, int shorten, int print)
return 1;
}
if (print) {
char *to_free = NULL;
if (shorten)
refname = shorten_unambiguous_ref(refname, 0);
refname = to_free = shorten_unambiguous_ref(refname, 0);
puts(refname);
free(to_free);
}
return 0;
}
Expand Down
10 changes: 6 additions & 4 deletions builtin/worktree.c
Original file line number Diff line number Diff line change
Expand Up @@ -446,16 +446,18 @@ static void print_preparing_worktree_line(int detach,
static const char *dwim_branch(const char *path, const char **new_branch)
{
int n;
int branch_exists;
const char *s = worktree_basename(path, &n);
const char *branchname = xstrndup(s, n);
struct strbuf ref = STRBUF_INIT;

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

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

*new_branch = branchname;
if (guess_remote) {
Expand Down
19 changes: 18 additions & 1 deletion parse-options.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
* OPTION_ALIAS.
*
* The returned options should be freed using free_preprocessed_options.
*/
static struct option *preprocess_options(struct parse_opt_ctx_t *ctx,
const struct option *options)
Expand Down Expand Up @@ -678,6 +680,7 @@ static struct option *preprocess_options(struct parse_opt_ctx_t *ctx,
newopt[i].short_name = short_name;
newopt[i].long_name = long_name;
newopt[i].help = strbuf_detach(&help, NULL);
newopt[i].flags |= PARSE_OPT_FROM_ALIAS;
break;
}

Expand All @@ -693,6 +696,20 @@ static struct option *preprocess_options(struct parse_opt_ctx_t *ctx,
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 *,
const char * const *,
const struct option *, int, int);
Expand Down Expand Up @@ -870,7 +887,7 @@ int parse_options(int argc, const char **argv, const char *prefix,
}

precompose_argv_prefix(argc, argv, NULL);
free(real_options);
free_preprocessed_options(real_options);
free(ctx.alias_groups);
return parse_options_end(&ctx);
}
Expand Down
35 changes: 18 additions & 17 deletions parse-options.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,26 +28,27 @@ enum parse_opt_type {
};

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

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

enum parse_opt_result {
Expand Down
2 changes: 2 additions & 0 deletions transport.c
Original file line number Diff line number Diff line change
Expand Up @@ -1452,6 +1452,8 @@ int transport_disconnect(struct transport *transport)
int ret = 0;
if (transport->vtable->disconnect)
ret = transport->vtable->disconnect(transport);
if (transport->got_remote_refs)
free_refs((void *)transport->remote_refs);
free(transport);
return ret;
}
Expand Down

0 comments on commit 642a400

Please sign in to comment.