git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "René Scharfe" <l.s.r@web.de>
To: Duy Nguyen <pclouds@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>, Jeff King <peff@peff.net>
Subject: [PATCH v2 1/2] sha1_file: replace PATH_MAX buffer wirh strbuf in, prepare_packed_git_one()
Date: Sun, 29 Jun 2014 07:43:17 +0200	[thread overview]
Message-ID: <53AFA775.1090900@web.de> (raw)
In-Reply-To: <CACsJy8B_DAjLRcMe4ys2LGkLOcKuW-PL_WNHyFB8Ry3Uv38LCw@mail.gmail.com>

Instead of using strbuf to create a message string in case a path is
too long for our fixed-size buffer, replace that buffer with a strbuf
and thus get rid of the limitation.

Helped-by: Duy Nguyen <pclouds@gmail.com>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
@Duy: Thanks for catching the missing strbuf_release in the opendir
error case.

 sha1_file.c | 42 ++++++++++++++++--------------------------
 1 file changed, 16 insertions(+), 26 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 34d527f..394fa45 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1177,48 +1177,37 @@ static void report_pack_garbage(struct string_list *list)
 
 static void prepare_packed_git_one(char *objdir, int local)
 {
-	/* Ensure that this buffer is large enough so that we can
-	   append "/pack/" without clobbering the stack even if
-	   strlen(objdir) were PATH_MAX.  */
-	char path[PATH_MAX + 1 + 4 + 1 + 1];
-	int len;
+	struct strbuf path = STRBUF_INIT;
+	size_t dirnamelen;
 	DIR *dir;
 	struct dirent *de;
 	struct string_list garbage = STRING_LIST_INIT_DUP;
 
-	sprintf(path, "%s/pack", objdir);
-	len = strlen(path);
-	dir = opendir(path);
+	strbuf_addstr(&path, objdir);
+	strbuf_addstr(&path, "/pack");
+	dir = opendir(path.buf);
 	if (!dir) {
 		if (errno != ENOENT)
 			error("unable to open object pack directory: %s: %s",
-			      path, strerror(errno));
+			      path.buf, strerror(errno));
+		strbuf_release(&path);
 		return;
 	}
-	path[len++] = '/';
+	strbuf_addch(&path, '/');
+	dirnamelen = path.len;
 	while ((de = readdir(dir)) != NULL) {
-		int namelen = strlen(de->d_name);
 		struct packed_git *p;
 
-		if (len + namelen + 1 > sizeof(path)) {
-			if (report_garbage) {
-				struct strbuf sb = STRBUF_INIT;
-				strbuf_addf(&sb, "%.*s/%s", len - 1, path, de->d_name);
-				report_garbage("path too long", sb.buf);
-				strbuf_release(&sb);
-			}
-			continue;
-		}
-
 		if (is_dot_or_dotdot(de->d_name))
 			continue;
 
-		strcpy(path + len, de->d_name);
+		strbuf_setlen(&path, dirnamelen);
+		strbuf_addstr(&path, de->d_name);
 
 		if (has_extension(de->d_name, ".idx")) {
 			/* Don't reopen a pack we already have. */
 			for (p = packed_git; p; p = p->next) {
-				if (!memcmp(path, p->pack_name, len + namelen - 4))
+				if (!memcmp(path.buf, p->pack_name, path.len - 4))
 					break;
 			}
 			if (p == NULL &&
@@ -1226,7 +1215,7 @@ static void prepare_packed_git_one(char *objdir, int local)
 			     * See if it really is a valid .idx file with
 			     * corresponding .pack file that we can map.
 			     */
-			    (p = add_packed_git(path, len + namelen, local)) != NULL)
+			    (p = add_packed_git(path.buf, path.len, local)) != NULL)
 				install_packed_git(p);
 		}
 
@@ -1237,13 +1226,14 @@ static void prepare_packed_git_one(char *objdir, int local)
 		    has_extension(de->d_name, ".pack") ||
 		    has_extension(de->d_name, ".bitmap") ||
 		    has_extension(de->d_name, ".keep"))
-			string_list_append(&garbage, path);
+			string_list_append(&garbage, path.buf);
 		else
-			report_garbage("garbage found", path);
+			report_garbage("garbage found", path.buf);
 	}
 	closedir(dir);
 	report_pack_garbage(&garbage);
 	string_list_clear(&garbage, 0);
+	strbuf_release(&path);
 }
 
 static int sort_pack(const void *a_, const void *b_)
-- 
2.0.0

  reply	other threads:[~2014-06-29  5:54 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-28 14:47 [PATCH 1/2] sha1_file: replace PATH_MAX buffer wirh strbuf in prepare_packed_git_one() René Scharfe
2014-06-28 15:01 ` [PATCH 2/4] sha1_file: use strncmp for string comparison René Scharfe
2014-06-29  1:21 ` [PATCH 1/2] sha1_file: replace PATH_MAX buffer wirh strbuf in prepare_packed_git_one() Duy Nguyen
2014-06-29  5:43   ` René Scharfe [this message]
2014-06-29  5:56     ` [PATCH v2 2/2] sha1_file: use strncmp for string comparison René Scharfe
2014-06-30 13:43       ` Jeff King
2014-06-30 13:59         ` Duy Nguyen
2014-06-30 14:22           ` Jeff King
2014-06-30 16:43             ` René Scharfe
2014-06-30 16:45               ` Jeff King
2014-06-30 16:35         ` René Scharfe
2014-06-30 16:46           ` Jeff King
2014-06-30 16:55         ` [PATCH 0/9] add strip_suffix as an alternative to ends_with Jeff King
2014-06-30 16:55           ` [PATCH 1/9] sha1_file: replace PATH_MAX buffer with strbuf in prepare_packed_git_one() Jeff King
2014-06-30 16:57           ` [PATCH 2/9] add strip_suffix function Jeff King
2014-07-02 15:54             ` Junio C Hamano
2014-07-02 16:38               ` Jeff King
2014-07-02 17:41                 ` Junio C Hamano
2014-06-30 16:58           ` [PATCH 3/9] implement ends_with via strip_suffix Jeff King
2014-06-30 16:58           ` [PATCH 4/9] replace has_extension with ends_with Jeff King
2014-06-30 16:58           ` [PATCH 5/9] use strip_suffix instead of ends_with in simple cases Jeff King
2014-06-30 16:59           ` [PATCH 6/9] index-pack: use strip_suffix to avoid magic numbers Jeff King
2014-06-30 17:01           ` [PATCH 7/9] strbuf: implement strbuf_strip_suffix Jeff King
2014-06-30 17:02           ` [PATCH 8/9] verify-pack: use strbuf_strip_suffix Jeff King
2014-06-30 17:04           ` [PATCH 9/9] prepare_packed_git_one: refactor duplicate-pack check Jeff King
2014-07-01  1:00           ` [PATCH 0/9] add strip_suffix as an alternative to ends_with Junio C Hamano
2014-06-30 13:24     ` [PATCH v2 1/2] sha1_file: replace PATH_MAX buffer wirh strbuf in, prepare_packed_git_one() Jeff King

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=53AFA775.1090900@web.de \
    --to=l.s.r@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    /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).