Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge branch 'jk/leakfix'
Code clean-up.

* jk/leakfix:
  submodule--helper: fix leak of core.worktree value
  config: fix leak in git_config_get_expiry_in_days()
  config: drop git_config_get_string_const()
  config: fix leaks from git_config_get_string_const()
  checkout: fix leak of non-existent branch names
  submodule--helper: use strbuf_release() to free strbufs
  clear_pattern_list(): clear embedded hashmaps
  • Loading branch information
gitster committed Aug 27, 2020
2 parents edab8a8 + 55fe225 commit 0d9a8e3
Show file tree
Hide file tree
Showing 17 changed files with 69 additions and 52 deletions.
4 changes: 2 additions & 2 deletions Documentation/MyFirstContribution.txt
Expand Up @@ -319,14 +319,14 @@ function body:
...

git_config(git_default_config, NULL);
if (git_config_get_string_const("user.name", &cfg_name) > 0)
if (git_config_get_string_tmp("user.name", &cfg_name) > 0)
printf(_("No name is found in config\n"));
else
printf(_("Your name: %s\n"), cfg_name);
----

`git_config()` will grab the configuration from config files known to Git and
apply standard precedence rules. `git_config_get_string_const()` will look up
apply standard precedence rules. `git_config_get_string_tmp()` will look up
a specific key ("user.name") and give you the value. There are a number of
single-key lookup functions like this one; you can see them all (and more info
about how to use `git_config()`) in `Documentation/technical/api-config.txt`.
Expand Down
4 changes: 2 additions & 2 deletions apply.c
Expand Up @@ -30,8 +30,8 @@ struct gitdiff_data {

static void git_apply_config(void)
{
git_config_get_string_const("apply.whitespace", &apply_default_whitespace);
git_config_get_string_const("apply.ignorewhitespace", &apply_default_ignorewhitespace);
git_config_get_string("apply.whitespace", &apply_default_whitespace);
git_config_get_string("apply.ignorewhitespace", &apply_default_ignorewhitespace);
git_config(git_xmerge_config, NULL);
}

Expand Down
4 changes: 3 additions & 1 deletion builtin/checkout.c
Expand Up @@ -1120,8 +1120,10 @@ static void setup_new_branch_info_and_source_tree(
if (!check_refname_format(new_branch_info->path, 0) &&
!read_ref(new_branch_info->path, &branch_rev))
oidcpy(rev, &branch_rev);
else
else {
free((char *)new_branch_info->path);
new_branch_info->path = NULL; /* not an existing branch */
}

new_branch_info->commit = lookup_commit_reference_gently(the_repository, rev, 1);
if (!new_branch_info->commit) {
Expand Down
2 changes: 1 addition & 1 deletion builtin/fetch.c
Expand Up @@ -648,7 +648,7 @@ static void prepare_format_display(struct ref *ref_map)
struct ref *rm;
const char *format = "full";

git_config_get_string_const("fetch.output", &format);
git_config_get_string_tmp("fetch.output", &format);
if (!strcasecmp(format, "full"))
compact_format = 0;
else if (!strcasecmp(format, "compact"))
Expand Down
16 changes: 8 additions & 8 deletions builtin/submodule--helper.c
Expand Up @@ -1511,7 +1511,7 @@ static void determine_submodule_update_strategy(struct repository *r,
if (parse_submodule_update_strategy(update, out) < 0)
die(_("Invalid update mode '%s' for submodule path '%s'"),
update, path);
} else if (!repo_config_get_string_const(r, key, &val)) {
} else if (!repo_config_get_string_tmp(r, key, &val)) {
if (parse_submodule_update_strategy(val, out) < 0)
die(_("Invalid update mode '%s' configured for submodule path '%s'"),
val, path);
Expand Down Expand Up @@ -1667,7 +1667,7 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
}

key = xstrfmt("submodule.%s.update", sub->name);
if (!repo_config_get_string_const(the_repository, key, &update_string)) {
if (!repo_config_get_string_tmp(the_repository, key, &update_string)) {
update_type = parse_submodule_update_type(update_string);
} else {
update_type = sub->update_strategy.type;
Expand All @@ -1690,7 +1690,7 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,

strbuf_reset(&sb);
strbuf_addf(&sb, "submodule.%s.url", sub->name);
if (repo_config_get_string_const(the_repository, sb.buf, &url)) {
if (repo_config_get_string_tmp(the_repository, sb.buf, &url)) {
if (starts_with_dot_slash(sub->url) ||
starts_with_dot_dot_slash(sub->url)) {
url = compute_submodule_clone_url(sub->url);
Expand Down Expand Up @@ -1747,8 +1747,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
"--no-single-branch");

cleanup:
strbuf_reset(&displaypath_sb);
strbuf_reset(&sb);
strbuf_release(&displaypath_sb);
strbuf_release(&sb);
if (need_free_url)
free((void*)url);

Expand Down Expand Up @@ -1976,7 +1976,7 @@ static const char *remote_submodule_branch(const char *path)
return NULL;

key = xstrfmt("submodule.%s.branch", sub->name);
if (repo_config_get_string_const(the_repository, key, &branch))
if (repo_config_get_string_tmp(the_repository, key, &branch))
branch = sub->branch;
free(key);

Expand Down Expand Up @@ -2101,7 +2101,7 @@ static int ensure_core_worktree(int argc, const char **argv, const char *prefix)
{
const struct submodule *sub;
const char *path;
char *cw;
const char *cw;
struct repository subrepo;

if (argc != 2)
Expand All @@ -2116,7 +2116,7 @@ static int ensure_core_worktree(int argc, const char **argv, const char *prefix)
if (repo_submodule_init(&subrepo, the_repository, sub))
die(_("could not get a repository handle for submodule '%s'"), path);

if (!repo_config_get_string(&subrepo, "core.worktree", &cw)) {
if (!repo_config_get_string_tmp(&subrepo, "core.worktree", &cw)) {
char *cfg_file, *abs_path;
const char *rel_path;
struct strbuf sb = STRBUF_INIT;
Expand Down
4 changes: 2 additions & 2 deletions cache.h
Expand Up @@ -921,8 +921,8 @@ extern int assume_unchanged;
extern int prefer_symlink_refs;
extern int warn_ambiguous_refs;
extern int warn_on_object_refname_ambiguity;
extern const char *apply_default_whitespace;
extern const char *apply_default_ignorewhitespace;
extern char *apply_default_whitespace;
extern char *apply_default_ignorewhitespace;
extern const char *git_attributes_file;
extern const char *git_hooks_path;
extern int zlib_compression_level;
Expand Down
3 changes: 1 addition & 2 deletions checkout.c
Expand Up @@ -47,15 +47,14 @@ const char *unique_tracking_name(const char *name, struct object_id *oid,
{
struct tracking_name_data cb_data = TRACKING_NAME_DATA_INIT;
const char *default_remote = NULL;
if (!git_config_get_string_const("checkout.defaultremote", &default_remote))
if (!git_config_get_string_tmp("checkout.defaultremote", &default_remote))
cb_data.default_remote = default_remote;
cb_data.src_ref = xstrfmt("refs/heads/%s", name);
cb_data.dst_oid = oid;
for_each_remote(check_tracking_name, &cb_data);
if (dwim_remotes_matched)
*dwim_remotes_matched = cb_data.num_matches;
free(cb_data.src_ref);
free((char *)default_remote);
if (cb_data.num_matches == 1) {
free(cb_data.default_dst_ref);
free(cb_data.default_dst_oid);
Expand Down
47 changes: 30 additions & 17 deletions config.c
Expand Up @@ -2006,18 +2006,27 @@ const struct string_list *git_configset_get_value_multi(struct config_set *cs, c
return e ? &e->value_list : NULL;
}

int git_configset_get_string_const(struct config_set *cs, const char *key, const char **dest)
int git_configset_get_string(struct config_set *cs, const char *key, char **dest)
{
const char *value;
if (!git_configset_get_value(cs, key, &value))
return git_config_string(dest, key, value);
return git_config_string((const char **)dest, key, value);
else
return 1;
}

int git_configset_get_string(struct config_set *cs, const char *key, char **dest)
int git_configset_get_string_tmp(struct config_set *cs, const char *key,
const char **dest)
{
return git_configset_get_string_const(cs, key, (const char **)dest);
const char *value;
if (!git_configset_get_value(cs, key, &value)) {
if (!value)
return config_error_nonbool(key);
*dest = value;
return 0;
} else {
return 1;
}
}

int git_configset_get_int(struct config_set *cs, const char *key, int *dest)
Expand Down Expand Up @@ -2147,22 +2156,26 @@ const struct string_list *repo_config_get_value_multi(struct repository *repo,
return git_configset_get_value_multi(repo->config, key);
}

int repo_config_get_string_const(struct repository *repo,
const char *key, const char **dest)
int repo_config_get_string(struct repository *repo,
const char *key, char **dest)
{
int ret;
git_config_check_init(repo);
ret = git_configset_get_string_const(repo->config, key, dest);
ret = git_configset_get_string(repo->config, key, dest);
if (ret < 0)
git_die_config(key, NULL);
return ret;
}

int repo_config_get_string(struct repository *repo,
const char *key, char **dest)
int repo_config_get_string_tmp(struct repository *repo,
const char *key, const char **dest)
{
int ret;
git_config_check_init(repo);
return repo_config_get_string_const(repo, key, (const char **)dest);
ret = git_configset_get_string_tmp(repo->config, key, dest);
if (ret < 0)
git_die_config(key, NULL);
return ret;
}

int repo_config_get_int(struct repository *repo,
Expand Down Expand Up @@ -2232,14 +2245,14 @@ const struct string_list *git_config_get_value_multi(const char *key)
return repo_config_get_value_multi(the_repository, key);
}

int git_config_get_string_const(const char *key, const char **dest)
int git_config_get_string(const char *key, char **dest)
{
return repo_config_get_string_const(the_repository, key, dest);
return repo_config_get_string(the_repository, key, dest);
}

int git_config_get_string(const char *key, char **dest)
int git_config_get_string_tmp(const char *key, const char **dest)
{
return repo_config_get_string(the_repository, key, dest);
return repo_config_get_string_tmp(the_repository, key, dest);
}

int git_config_get_int(const char *key, int *dest)
Expand Down Expand Up @@ -2274,7 +2287,7 @@ int git_config_get_pathname(const char *key, const char **dest)

int git_config_get_expiry(const char *key, const char **output)
{
int ret = git_config_get_string_const(key, output);
int ret = git_config_get_string(key, (char **)output);
if (ret)
return ret;
if (strcmp(*output, "now")) {
Expand All @@ -2287,11 +2300,11 @@ int git_config_get_expiry(const char *key, const char **output)

int git_config_get_expiry_in_days(const char *key, timestamp_t *expiry, timestamp_t now)
{
char *expiry_string;
const char *expiry_string;
intmax_t days;
timestamp_t when;

if (git_config_get_string(key, &expiry_string))
if (git_config_get_string_tmp(key, &expiry_string))
return 1; /* no such thing */

if (git_parse_signed(expiry_string, &days, maximum_signed_value_of_type(int))) {
Expand Down
15 changes: 8 additions & 7 deletions config.h
Expand Up @@ -458,8 +458,8 @@ void git_configset_clear(struct config_set *cs);
*/
int git_configset_get_value(struct config_set *cs, const char *key, const char **dest);

int git_configset_get_string_const(struct config_set *cs, const char *key, const char **dest);
int git_configset_get_string(struct config_set *cs, const char *key, char **dest);
int git_configset_get_string_tmp(struct config_set *cs, const char *key, const char **dest);
int git_configset_get_int(struct config_set *cs, const char *key, int *dest);
int git_configset_get_ulong(struct config_set *cs, const char *key, unsigned long *dest);
int git_configset_get_bool(struct config_set *cs, const char *key, int *dest);
Expand All @@ -474,10 +474,10 @@ int repo_config_get_value(struct repository *repo,
const char *key, const char **value);
const struct string_list *repo_config_get_value_multi(struct repository *repo,
const char *key);
int repo_config_get_string_const(struct repository *repo,
const char *key, const char **dest);
int repo_config_get_string(struct repository *repo,
const char *key, char **dest);
int repo_config_get_string_tmp(struct repository *repo,
const char *key, const char **dest);
int repo_config_get_int(struct repository *repo,
const char *key, int *dest);
int repo_config_get_ulong(struct repository *repo,
Expand Down Expand Up @@ -529,13 +529,14 @@ void git_config_clear(void);
* error message and returns -1. When the configuration variable `key` is
* not found, returns 1 without touching `dest`.
*/
int git_config_get_string_const(const char *key, const char **dest);
int git_config_get_string(const char *key, char **dest);

/**
* Similar to `git_config_get_string_const`, except that retrieved value
* copied into the `dest` parameter is a mutable string.
* Similar to `git_config_get_string`, but does not allocate any new
* memory; on success `dest` will point to memory owned by the config
* machinery, which could be invalidated if it is discarded and reloaded.
*/
int git_config_get_string(const char *key, char **dest);
int git_config_get_string_tmp(const char *key, const char **dest);

/**
* Finds and parses the value to an integer for the configuration variable
Expand Down
4 changes: 2 additions & 2 deletions connect.c
Expand Up @@ -1052,7 +1052,7 @@ static const char *get_ssh_command(void)
if ((ssh = getenv("GIT_SSH_COMMAND")))
return ssh;

if (!git_config_get_string_const("core.sshcommand", &ssh))
if (!git_config_get_string_tmp("core.sshcommand", &ssh))
return ssh;

return NULL;
Expand All @@ -1071,7 +1071,7 @@ static void override_ssh_variant(enum ssh_variant *ssh_variant)
{
const char *variant = getenv("GIT_SSH_VARIANT");

if (!variant && git_config_get_string_const("ssh.variant", &variant))
if (!variant && git_config_get_string_tmp("ssh.variant", &variant))
return;

if (!strcmp(variant, "auto"))
Expand Down
2 changes: 2 additions & 0 deletions dir.c
Expand Up @@ -921,6 +921,8 @@ void clear_pattern_list(struct pattern_list *pl)
free(pl->patterns[i]);
free(pl->patterns);
free(pl->filebuf);
hashmap_free_entries(&pl->recursive_hashmap, struct pattern_entry, ent);
hashmap_free_entries(&pl->parent_hashmap, struct pattern_entry, ent);

memset(pl, 0, sizeof(*pl));
}
Expand Down
2 changes: 1 addition & 1 deletion editor.c
Expand Up @@ -40,7 +40,7 @@ const char *git_sequence_editor(void)
const char *editor = getenv("GIT_SEQUENCE_EDITOR");

if (!editor)
git_config_get_string_const("sequence.editor", &editor);
git_config_get_string_tmp("sequence.editor", &editor);
if (!editor)
editor = git_editor();

Expand Down
4 changes: 2 additions & 2 deletions environment.c
Expand Up @@ -35,8 +35,8 @@ int repository_format_precious_objects;
int repository_format_worktree_config;
const char *git_commit_encoding;
const char *git_log_output_encoding;
const char *apply_default_whitespace;
const char *apply_default_ignorewhitespace;
char *apply_default_whitespace;
char *apply_default_ignorewhitespace;
const char *git_attributes_file;
const char *git_hooks_path;
int zlib_compression_level = Z_BEST_SPEED;
Expand Down
2 changes: 1 addition & 1 deletion help.c
Expand Up @@ -375,7 +375,7 @@ void list_cmds_by_config(struct string_list *list)
{
const char *cmd_list;

if (git_config_get_string_const("completion.commands", &cmd_list))
if (git_config_get_string_tmp("completion.commands", &cmd_list))
return;

string_list_sort(list);
Expand Down
2 changes: 1 addition & 1 deletion protocol.c
Expand Up @@ -21,7 +21,7 @@ enum protocol_version get_protocol_version_config(void)
const char *git_test_k = "GIT_TEST_PROTOCOL_VERSION";
const char *git_test_v;

if (!git_config_get_string_const("protocol.version", &value)) {
if (!git_config_get_string_tmp("protocol.version", &value)) {
enum protocol_version version = parse_protocol_version(value);

if (version == protocol_unknown_version)
Expand Down
4 changes: 2 additions & 2 deletions submodule.c
Expand Up @@ -194,7 +194,7 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
char *key;

key = xstrfmt("submodule.%s.ignore", submodule->name);
if (repo_config_get_string_const(the_repository, key, &ignore))
if (repo_config_get_string_tmp(the_repository, key, &ignore))
ignore = submodule->ignore;
free(key);

Expand Down Expand Up @@ -1299,7 +1299,7 @@ static int get_fetch_recurse_config(const struct submodule *submodule,

int fetch_recurse = submodule->fetch_recurse;
key = xstrfmt("submodule.%s.fetchRecurseSubmodules", submodule->name);
if (!repo_config_get_string_const(spf->r, key, &value)) {
if (!repo_config_get_string_tmp(spf->r, key, &value)) {
fetch_recurse = parse_fetch_recurse_submodules_arg(key, value);
}
free(key);
Expand Down
2 changes: 1 addition & 1 deletion t/helper/test-config.c
Expand Up @@ -126,7 +126,7 @@ int cmd__config(int argc, const char **argv)
goto exit1;
}
} else if (argc == 3 && !strcmp(argv[1], "get_string")) {
if (!git_config_get_string_const(argv[2], &v)) {
if (!git_config_get_string_tmp(argv[2], &v)) {
printf("%s\n", v);
goto exit0;
} else {
Expand Down

0 comments on commit 0d9a8e3

Please sign in to comment.