From 344b548475b86f9f95e9fbcd93022f8083918cc7 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Thu, 31 Mar 2016 17:35:43 -0700 Subject: notes: don't leak memory in git_config_get_notes_strategy This function asks for the value of a configuration and after using the value does not have to retain ownership of it. git_config_get_string_const() however is a function to get a copy of the value, but we forget to free it before we return. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- builtin/notes.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/builtin/notes.c b/builtin/notes.c index 52aa9af74b..afcfa8f522 100644 --- a/builtin/notes.c +++ b/builtin/notes.c @@ -741,13 +741,14 @@ static int merge_commit(struct notes_merge_options *o) static int git_config_get_notes_strategy(const char *key, enum notes_merge_strategy *strategy) { - const char *value; + char *value; - if (git_config_get_string_const(key, &value)) + if (git_config_get_string(key, &value)) return 1; if (parse_notes_merge_strategy(value, strategy)) git_die_config(key, "unknown notes merge strategy %s", value); + free(value); return 0; } -- cgit v1.2.3 From 6eb6078bf5fd08028b21d80b9062a4aed83a2340 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Thu, 31 Mar 2016 17:35:44 -0700 Subject: abbrev_sha1_in_line: don't leak memory `split` is of type `struct strbuf **`, and currently we are leaking split itself as well as each element in split[i]. We have a dedicated free function for `struct strbuf **`, which takes care of freeing all related memory. Helped-by: Eric Sunshine Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- wt-status.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/wt-status.c b/wt-status.c index bba25960b4..9f4be333d4 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1063,9 +1063,7 @@ static void abbrev_sha1_in_line(struct strbuf *line) strbuf_addf(line, "%s", split[i]->buf); } } - for (i = 0; split[i]; i++) - strbuf_release(split[i]); - + strbuf_list_free(split); } static void read_rebase_todolist(const char *fname, struct string_list *lines) -- cgit v1.2.3 From f5ff5fb56485979895cff5954e4db1ea4ff4c9f7 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Thu, 31 Mar 2016 17:35:45 -0700 Subject: bundle: don't leak an fd in case of early return In successful operation `write_pack_data` will close the `bundle_fd`, but when we exit early, we need to take care of the file descriptor as well as the lock file ourselves. The lock file may be deleted at the end of running the program, but we are in library code, so we should not rely on that. Helped-by: Jeff King Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- bundle.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/bundle.c b/bundle.c index 506ac49691..bbf4efa0a0 100644 --- a/bundle.c +++ b/bundle.c @@ -435,12 +435,14 @@ int create_bundle(struct bundle_header *header, const char *path, /* write prerequisites */ if (compute_and_write_prerequisites(bundle_fd, &revs, argc, argv)) - return -1; + goto err; argc = setup_revisions(argc, argv, &revs, NULL); - if (argc > 1) - return error(_("unrecognized argument: %s"), argv[1]); + if (argc > 1) { + error(_("unrecognized argument: %s"), argv[1]); + goto err; + } object_array_remove_duplicates(&revs.pending); @@ -448,17 +450,26 @@ int create_bundle(struct bundle_header *header, const char *path, if (!ref_count) die(_("Refusing to create empty bundle.")); else if (ref_count < 0) - return -1; + goto err; /* write pack */ - if (write_pack_data(bundle_fd, &revs)) - return -1; + if (write_pack_data(bundle_fd, &revs)) { + bundle_fd = -1; /* already closed by the above call */ + goto err; + } if (!bundle_to_stdout) { if (commit_lock_file(&lock)) die_errno(_("cannot create '%s'"), path); } return 0; +err: + if (!bundle_to_stdout) { + if (0 <= bundle_fd) + close(bundle_fd); + rollback_lock_file(&lock); + } + return -1; } int unbundle(struct bundle_header *header, int bundle_fd, int flags) -- cgit v1.2.3 From 9c60d9faab43f73166d4cebb7c86b1bbf8d8df4b Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Thu, 31 Mar 2016 17:35:46 -0700 Subject: credential-cache, send_request: close fd when done No need to keep it open any further. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- credential-cache.c | 1 + 1 file changed, 1 insertion(+) diff --git a/credential-cache.c b/credential-cache.c index f4afdc6988..86e21de49b 100644 --- a/credential-cache.c +++ b/credential-cache.c @@ -32,6 +32,7 @@ static int send_request(const char *socket, const struct strbuf *out) write_or_die(1, in, r); got_data = 1; } + close(fd); return got_data; } -- cgit v1.2.3 From 270cd9eaf459b94dee1836de9bf4382afebe1d7b Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Wed, 27 Apr 2016 14:30:39 -0700 Subject: config.c: drop local variable As `ret` is not used for anything except determining an early return, we don't need a variable for that. Drop it. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- config.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/config.c b/config.c index 03544e7a3f..84e437a9b9 100644 --- a/config.c +++ b/config.c @@ -1309,14 +1309,11 @@ static struct config_set_element *configset_find_element(struct config_set *cs, struct config_set_element k; struct config_set_element *found_entry; char *normalized_key; - int ret; /* * `key` may come from the user, so normalize it before using it * for querying entries from the hashmap. */ - ret = git_config_parse_key(key, &normalized_key, NULL); - - if (ret) + if (git_config_parse_key(key, &normalized_key, NULL)) return NULL; hashmap_entry_init(&k, strhash(normalized_key)); -- cgit v1.2.3 From 99dab16863c2eca584a84bdde748b9c113717603 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Wed, 27 Apr 2016 14:30:40 -0700 Subject: submodule-config: don't shadow `cache` Lots of internal functions in submodule-confic.c have a first parameter `struct submodule_cache *cache`, which currently always refers to the global variable `cache` in the file. To avoid confusion rename the global `cache` variable. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- submodule-config.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/submodule-config.c b/submodule-config.c index fe8ceabf30..967bba232a 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -30,7 +30,7 @@ enum lookup_type { lookup_path }; -static struct submodule_cache cache; +static struct submodule_cache the_submodule_cache; static int is_cache_init; static int config_path_cmp(const struct submodule_entry *a, @@ -458,14 +458,14 @@ static void ensure_cache_init(void) if (is_cache_init) return; - cache_init(&cache); + cache_init(&the_submodule_cache); is_cache_init = 1; } int parse_submodule_config_option(const char *var, const char *value) { struct parse_config_parameter parameter; - parameter.cache = &cache; + parameter.cache = &the_submodule_cache; parameter.commit_sha1 = NULL; parameter.gitmodules_sha1 = null_sha1; parameter.overwrite = 1; @@ -478,18 +478,18 @@ const struct submodule *submodule_from_name(const unsigned char *commit_sha1, const char *name) { ensure_cache_init(); - return config_from_name(&cache, commit_sha1, name); + return config_from_name(&the_submodule_cache, commit_sha1, name); } const struct submodule *submodule_from_path(const unsigned char *commit_sha1, const char *path) { ensure_cache_init(); - return config_from_path(&cache, commit_sha1, path); + return config_from_path(&the_submodule_cache, commit_sha1, path); } void submodule_free(void) { - cache_free(&cache); + cache_free(&the_submodule_cache); is_cache_init = 0; } -- cgit v1.2.3