git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Beller <sbeller@google.com>
To: gitster@pobox.com, peff@peff.net, sunshine@sunshineco.com
Cc: git@vger.kernel.org, Stefan Beller <sbeller@google.com>
Subject: [PATCHv4 0/4] Some cleanups
Date: Thu, 31 Mar 2016 17:35:42 -0700	[thread overview]
Message-ID: <1459470946-16522-1-git-send-email-sbeller@google.com> (raw)

Thanks for the reviews!
Thanks Jeff, Eric, Junio!

This replaces sb/misc-cleanups.

* Revert the builtin/notes fix to the first version as git_config_get_value
  is dangerous, and the memory allocation and free is just a small overhead here
* the bundle code integrates all of the suggestions (i.e. rollback_lock_file
  conditioned on (!bundle_to_stdout).
  
I hope I got everything by now. I will head out for today and tomorrow I'll
be traveling to NY, where there is the Git Merge conference on Mon, Tues and some
vacation afterwards. I'll be online in a very limited fashion. (I plan on taking
my phone only, no laptop), so in case I missed a thing this series will halt
for a while or someone else picks it up.

Thanks,
Stefan

diff to remotes/origin/sb/misc-cleanups (which doesn't contain the bundle fix):
diff --git a/builtin/notes.c b/builtin/notes.c
index 386402d..afcfa8f 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_value(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;
 }
 
diff --git a/bundle.c b/bundle.c
index 506ac49..08e32ca 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,23 @@ 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;
+		goto err;
 
 	if (!bundle_to_stdout) {
 		if (commit_lock_file(&lock))
 			die_errno(_("cannot create '%s'"), path);
 	}
 	return 0;
+err:
+	if (!bundle_to_stdout) {
+		close(bundle_fd);
+		rollback_lock_file(&lock);
+	}
+	return -1;
 }
 
 int unbundle(struct bundle_header *header, int bundle_fd, int flags)


Stefan Beller (4):
  notes: don't leak memory in git_config_get_notes_strategy
  abbrev_sha1_in_line: don't leak memory
  bundle: don't leak an fd in case of early return
  credential-cache, send_request: close fd when done

 builtin/notes.c    |  5 +++--
 bundle.c           | 18 +++++++++++++-----
 credential-cache.c |  1 +
 wt-status.c        |  4 +---
 4 files changed, 18 insertions(+), 10 deletions(-)

-- 
2.5.0.264.gc776916.dirty

             reply	other threads:[~2016-04-01  0:35 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-01  0:35 Stefan Beller [this message]
2016-04-01  0:35 ` [PATCHv4 1/4] notes: don't leak memory in git_config_get_notes_strategy Stefan Beller
2016-04-01  0:46   ` Eric Sunshine
2016-04-01 16:12     ` Junio C Hamano
2016-04-01 17:03       ` Junio C Hamano
2016-04-01 17:14         ` Jeff King
2016-04-01 17:30           ` Eric Sunshine
2016-04-01  0:35 ` [PATCHv4 2/4] abbrev_sha1_in_line: don't leak memory Stefan Beller
2016-04-01  0:35 ` [PATCHv4 3/4] bundle: don't leak an fd in case of early return Stefan Beller
2016-04-01 17:05   ` Junio C Hamano
2016-04-01 17:15     ` Jeff King
2016-04-01 17:29       ` Junio C Hamano
2016-04-01 17:31         ` Jeff King
2016-04-01  0:35 ` [PATCHv4 4/4] credential-cache, send_request: close fd when done Stefan Beller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1459470946-16522-1-git-send-email-sbeller@google.com \
    --to=sbeller@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).