git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Faiz Kothari <faiz.off93@gmail.com>
To: git@vger.kernel.org
Cc: mhagger@alum.mit.edu, Faiz Kothari <faiz.off93@gmail.com>
Subject: [PATCH] rewrite bulk-checkin.c:finish_bulk_checkin() using strbuf
Date: Fri, 28 Feb 2014 13:28:25 +0530	[thread overview]
Message-ID: <1393574305-24015-1-git-send-email-faiz.off93@gmail.com> (raw)

Signed-off-by: Faiz Kothari <faiz.off93@gmail.com>

Notes:
    I finally got what's happening, and why the errors were caused.
    packname is supposed to contain the complete path to the .pack file.
    Packs are stored as /path/to/<SHA1>.pack which I overlooked earlier.
    After inspecting what is happening in pack-write.c:finish_tmp_packfile()
    which indirectly modifies packname by appending the SHA1 and ".pack" to packname
    This is happening in these code snippets:
    	char *end_of_name_prefix = strrchr(name_buffer, 0);
    
    and later
    	sprintf(end_of_name_prefix, "%s.pack", sha1_to_hex(sha1));
    
    name_buffer is packname.buf
    Using const for the first argument of pack-write.c:finish_tmp_packfile()
    doesnot raise any compile time warning or error and not any runtime errors,
    since the packname.buf is on heap and has extra space to which more char can be written.
    If this was not the case,
    	for e.g. passing a constant string and modifying it.
    	This will result in a segmentation fault.
---
 bulk-checkin.c |    8 +++++---
 pack-write.c   |    2 +-
 pack.h         |    2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/bulk-checkin.c b/bulk-checkin.c
index 118c625..bbdf1ec 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -23,7 +23,7 @@ static struct bulk_checkin_state {
 static void finish_bulk_checkin(struct bulk_checkin_state *state)
 {
 	unsigned char sha1[20];
-	char packname[PATH_MAX];
+	struct strbuf packname = STRBUF_INIT;
 	int i;
 
 	if (!state->f)
@@ -42,9 +42,10 @@ static void finish_bulk_checkin(struct bulk_checkin_state *state)
 					 state->offset);
 		close(fd);
 	}
+	strbuf_addf(&packname, "%s/pack/pack-", get_object_directory());
+	strbuf_grow(&packname, 40 + 5);
 
-	sprintf(packname, "%s/pack/pack-", get_object_directory());
-	finish_tmp_packfile(packname, state->pack_tmp_name,
+	finish_tmp_packfile(packname.buf, state->pack_tmp_name,
 			    state->written, state->nr_written,
 			    &state->pack_idx_opts, sha1);
 	for (i = 0; i < state->nr_written; i++)
@@ -53,6 +54,7 @@ static void finish_bulk_checkin(struct bulk_checkin_state *state)
 clear_exit:
 	free(state->written);
 	memset(state, 0, sizeof(*state));
+	strbuf_release(&packname);
 
 	/* Make objects we just wrote available to ourselves */
 	reprepare_packed_git();
diff --git a/pack-write.c b/pack-write.c
index 605d01b..ac38867 100644
--- a/pack-write.c
+++ b/pack-write.c
@@ -336,7 +336,7 @@ struct sha1file *create_tmp_packfile(char **pack_tmp_name)
 	return sha1fd(fd, *pack_tmp_name);
 }
 
-void finish_tmp_packfile(char *name_buffer,
+void finish_tmp_packfile(const char *name_buffer,
 			 const char *pack_tmp_name,
 			 struct pack_idx_entry **written_list,
 			 uint32_t nr_written,
diff --git a/pack.h b/pack.h
index 12d9516..3b9e033 100644
--- a/pack.h
+++ b/pack.h
@@ -91,6 +91,6 @@ extern int encode_in_pack_object_header(enum object_type, uintmax_t, unsigned ch
 extern int read_pack_header(int fd, struct pack_header *);
 
 extern struct sha1file *create_tmp_packfile(char **pack_tmp_name);
-extern void finish_tmp_packfile(char *name_buffer, const char *pack_tmp_name, struct pack_idx_entry **written_list, uint32_t nr_written, struct pack_idx_option *pack_idx_opts, unsigned char sha1[]);
+extern void finish_tmp_packfile(const char *name_buffer, const char *pack_tmp_name, struct pack_idx_entry **written_list, uint32_t nr_written, struct pack_idx_option *pack_idx_opts, unsigned char sha1[]);
 
 #endif
-- 
1.7.9.5

             reply	other threads:[~2014-02-28  7:58 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-28  7:58 Faiz Kothari [this message]
2014-02-28  9:15 ` [PATCH] rewrite bulk-checkin.c:finish_bulk_checkin() using strbuf Eric Sunshine
2014-02-28 18:27   ` Faiz Kothari
2014-02-28 21:59     ` Eric Sunshine
2014-03-01  1:34     ` He Sun

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=1393574305-24015-1-git-send-email-faiz.off93@gmail.com \
    --to=faiz.off93@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=mhagger@alum.mit.edu \
    /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).