From 344b548475b86f9f95e9fbcd93022f8083918cc7 Mon Sep 17 00:00:00 2001 From: Stefan Beller <sbeller@google.com> Date: Thu, 31 Mar 2016 17:35:43 -0700 Subject: [PATCH 1/4] 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 <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- builtin/notes.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/builtin/notes.c b/builtin/notes.c index 52aa9af74be..afcfa8f5220 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; } From 6eb6078bf5fd08028b21d80b9062a4aed83a2340 Mon Sep 17 00:00:00 2001 From: Stefan Beller <sbeller@google.com> Date: Thu, 31 Mar 2016 17:35:44 -0700 Subject: [PATCH 2/4] 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 <sunshine@sunshineco.com> Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- wt-status.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/wt-status.c b/wt-status.c index bba25960b4e..9f4be333d45 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) From f5ff5fb56485979895cff5954e4db1ea4ff4c9f7 Mon Sep 17 00:00:00 2001 From: Stefan Beller <sbeller@google.com> Date: Thu, 31 Mar 2016 17:35:45 -0700 Subject: [PATCH 3/4] 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 <peff@peff.net> Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- bundle.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/bundle.c b/bundle.c index 506ac49691b..bbf4efa0a0a 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) From 9c60d9faab43f73166d4cebb7c86b1bbf8d8df4b Mon Sep 17 00:00:00 2001 From: Stefan Beller <sbeller@google.com> Date: Thu, 31 Mar 2016 17:35:46 -0700 Subject: [PATCH 4/4] credential-cache, send_request: close fd when done No need to keep it open any further. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- credential-cache.c | 1 + 1 file changed, 1 insertion(+) diff --git a/credential-cache.c b/credential-cache.c index f4afdc6988c..86e21de49be 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; }