Re: [PATCH 41/68] init: use strbufs to store paths

9 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 41/68] init: use strbufs to store paths

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:06:45

Torsten Bögershausen [off-list ref] writes:
On 30.09.15 02:23, Jeff King wrote:
quoted
On Tue, Sep 29, 2015 at 04:50:39PM -0700, Michael Blume wrote:
quoted
I see compile errors on my mac:
This is my attempt, passing the test, but not fully polished.
Thanks.
quoted hunk
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 89f2c05..60b559c 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -276,7 +276,9 @@ static int create_default_files(const char *template_path)
 		path = git_path_buf(&buf, "CoNfIg");
 		if (!access(path, F_OK))
 			git_config_set("core.ignorecase", "true");
-		probe_utf8_pathname_composition(path);
+		/* Probe utf-8 normalization withou mangling CoNfIG */
+		path = git_path_buf(&buf, "config");
+		probe_utf8_pathname_composition(path, strlen(path));
Hmph, Peff's quick-fix passed the original "CoNfIg" in &buf directly
to probe_utf8_pathname_composition() without changing its signature.

What is the reason behind these two changes?  i.e. why is it
inappropriate to use "CoNfIg" (and append the auml to it to use for
the checking) and why does the function need to take pointer + len,
only to store it in another strbuf itself?
quoted hunk
diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c
index b4dd3c7..37172a4 100644
--- a/compat/precompose_utf8.c
+++ b/compat/precompose_utf8.c
@@ -8,6 +8,7 @@
 #include "cache.h"
 #include "utf8.h"
 #include "precompose_utf8.h"
+#include "strbuf.h"
 
 typedef char *iconv_ibp;
 static const char *repo_encoding = "UTF-8";
@@ -36,28 +37,33 @@ static size_t has_non_ascii(const char *s, size_t maxlen, size_t *strlen_c)
 }
 
 
-void probe_utf8_pathname_composition(struct strbuf *path)
+void probe_utf8_pathname_composition(char *path, int len)
 {
 	static const char *auml_nfc = "\xc3\xa4";
 	static const char *auml_nfd = "\x61\xcc\x88";
-	size_t baselen = path->len;
+	struct strbuf sbuf;
 	int output_fd;
 	if (precomposed_unicode != -1)
 		return; /* We found it defined in the global config, respect it */
-	strbuf_addstr(path, auml_nfc);
-	output_fd = open(path, O_CREAT|O_EXCL|O_RDWR, 0600);
+	strbuf_init(&sbuf, len+3);
+	strbuf_add(&sbuf, path, len);
+	strbuf_addstr(&sbuf, auml_nfc);
+	output_fd = open(sbuf.buf, O_CREAT|O_EXCL|O_RDWR, 0600);
+	fprintf(stderr, "%s/%s:%d sbuf.buf=%s\n",
+					__FILE__, __FUNCTION__, __LINE__, sbuf.buf);
 	if (output_fd >= 0) {
 		close(output_fd);
-		strbuf_setlen(path, baselen);
-		strbuf_addstr(path, auml_nfd);
+		strbuf_setlen(&sbuf, len);
+		strbuf_addstr(&sbuf, auml_nfd);
+	fprintf(stderr, "%s/%s:%d sbuf.buf=%s\n",
+					__FILE__, __FUNCTION__, __LINE__, sbuf.buf);
 		precomposed_unicode = access(path, R_OK) ? 0 : 1;
 		git_config_set("core.precomposeunicode", precomposed_unicode ? "true" : "false");
-		strbuf_setlen(path, baselen);
-		strbuf_addstr(path, auml_nfc);
+		strcpy(path + len, auml_nfc);
 		if (unlink(path))
 			die_errno(_("failed to unlink '%s'"), path);
 	}
-	strbuf_setlen(path, baselen);
+	strbuf_release(&sbuf);
 }
 
 
diff --git a/compat/precompose_utf8.h b/compat/precompose_utf8.h
index 7fc7be5..3b73585 100644
--- a/compat/precompose_utf8.h
+++ b/compat/precompose_utf8.h
@@ -27,7 +27,7 @@ typedef struct {
 } PREC_DIR;
 
 void precompose_argv(int argc, const char **argv);
-void probe_utf8_pathname_composition(struct strbuf *path);
+void probe_utf8_pathname_composition(char *, int);
 
 PREC_DIR *precompose_utf8_opendir(const char *dirname);
 struct dirent_prec_psx *precompose_utf8_readdir(PREC_DIR *dirp);

Re: [PATCH 41/68] init: use strbufs to store paths

From: Torsten Bögershausen <hidden>
Date: 2016-06-15 23:06:45

On 03.10.15 18:54, Junio C Hamano wrote:
Torsten Bögershausen [off-list ref] writes:
quoted
On 30.09.15 02:23, Jeff King wrote:
quoted
On Tue, Sep 29, 2015 at 04:50:39PM -0700, Michael Blume wrote:
quoted
I see compile errors on my mac:
This is my attempt, passing the test, but not fully polished.
Thanks.
quoted
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 89f2c05..60b559c 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -276,7 +276,9 @@ static int create_default_files(const char *template_path)
 		path = git_path_buf(&buf, "CoNfIg");
 		if (!access(path, F_OK))
 			git_config_set("core.ignorecase", "true");
-		probe_utf8_pathname_composition(path);
+		/* Probe utf-8 normalization withou mangling CoNfIG */
+		path = git_path_buf(&buf, "config");
+		probe_utf8_pathname_composition(path, strlen(path));
Hmph, Peff's quick-fix passed the original "CoNfIg" in &buf directly
to probe_utf8_pathname_composition() without changing its signature.
True, ( I was thinking that the test did only work on case insensitive FS).
We can skip that change.

Beside that, I later realized, that a better signature could be:
+void probe_utf8_pathname_composition(const char *path, size_t len)

I can send a proper patch the next days.

Re: [PATCH 41/68] init: use strbufs to store paths

From: Jeff King <hidden>
Date: 2016-06-15 23:06:45

On Sat, Oct 03, 2015 at 11:12:13PM +0200, Torsten Bögershausen wrote:
quoted
Hmph, Peff's quick-fix passed the original "CoNfIg" in &buf directly
to probe_utf8_pathname_composition() without changing its signature.
True, ( I was thinking that the test did only work on case insensitive FS).
We can skip that change.

Beside that, I later realized, that a better signature could be:
+void probe_utf8_pathname_composition(const char *path, size_t len)

I can send a proper patch the next days.
That is the original signature, before my sprintf series. I do not mind
leaving that as-is, and simply cleaning up probe_utf8_pathname_composition
by using a strbuf internally there. Though I have to wonder if it even
needs us to pass _anything_ at that point. It could just call
git_path_buf("config%s", auml_nfd) itself. The whole reason to pass
anything was to let it reuse the buffer the caller had.

-Peff

Re: [PATCH 41/68] init: use strbufs to store paths

From: Torsten Bögershausen <hidden>
Date: 2016-06-15 23:06:45


On 2015-10-04 05.37, Jeff King wrote:
On Sat, Oct 03, 2015 at 11:12:13PM +0200, Torsten Bögershausen wrote:
quoted
quoted
Hmph, Peff's quick-fix passed the original "CoNfIg" in &buf directly
to probe_utf8_pathname_composition() without changing its signature.
True, ( I was thinking that the test did only work on case insensitive FS).
We can skip that change.

Beside that, I later realized, that a better signature could be:
+void probe_utf8_pathname_composition(const char *path, size_t len)

I can send a proper patch the next days.
That is the original signature, before my sprintf series. I do not mind
leaving that as-is, and simply cleaning up probe_utf8_pathname_composition
by using a strbuf internally there. Though I have to wonder if it even
needs us to pass _anything_ at that point. It could just call
git_path_buf("config%s", auml_nfd) itself. The whole reason to pass
anything was to let it reuse the buffer the caller had.

-Peff
Makes sense, here is V2:
 git diff  07690109b6a252ac7cbede
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 89f2c05..4892579 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -276,7 +276,7 @@ static int create_default_files(const char *template_path)
         path = git_path_buf(&buf, "CoNfIg");
         if (!access(path, F_OK))
             git_config_set("core.ignorecase", "true");
-        probe_utf8_pathname_composition(path);
+        probe_utf8_pathname_composition();
     }
 
     strbuf_release(&buf);
diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c
index b4dd3c7..64b85f2 100644
--- a/compat/precompose_utf8.c
+++ b/compat/precompose_utf8.c
@@ -8,6 +8,7 @@
 #include "cache.h"
 #include "utf8.h"
 #include "precompose_utf8.h"
+#include "strbuf.h"
 
 typedef char *iconv_ibp;
 static const char *repo_encoding = "UTF-8";
@@ -36,28 +37,27 @@ static size_t has_non_ascii(const char *s, size_t maxlen,
size_t *strlen_c)
 }
 
 
-void probe_utf8_pathname_composition(struct strbuf *path)
+void probe_utf8_pathname_composition(void)
 {
+    struct strbuf sbuf = STRBUF_INIT;
     static const char *auml_nfc = "\xc3\xa4";
     static const char *auml_nfd = "\x61\xcc\x88";
-    size_t baselen = path->len;
+    const char *path;
     int output_fd;
     if (precomposed_unicode != -1)
         return; /* We found it defined in the global config, respect it */
-    strbuf_addstr(path, auml_nfc);
+    path = git_path_buf(&sbuf, "%s", auml_nfc);
     output_fd = open(path, O_CREAT|O_EXCL|O_RDWR, 0600);
     if (output_fd >= 0) {
         close(output_fd);
-        strbuf_setlen(path, baselen);
-        strbuf_addstr(path, auml_nfd);
+        path = git_path_buf(&sbuf, "%s", auml_nfd);
         precomposed_unicode = access(path, R_OK) ? 0 : 1;
         git_config_set("core.precomposeunicode", precomposed_unicode ? "true" :
"false");
-        strbuf_setlen(path, baselen);
-        strbuf_addstr(path, auml_nfc);
+        path = git_path_buf(&sbuf, "%s", auml_nfc);
         if (unlink(path))
             die_errno(_("failed to unlink '%s'"), path);
     }
-    strbuf_setlen(path, baselen);
+    strbuf_release(&sbuf);
 }
 
 
diff --git a/compat/precompose_utf8.h b/compat/precompose_utf8.h
index 7fc7be5..a94e7c4 100644
--- a/compat/precompose_utf8.h
+++ b/compat/precompose_utf8.h
@@ -27,7 +27,7 @@ typedef struct {
 } PREC_DIR;
 
 void precompose_argv(int argc, const char **argv);
-void probe_utf8_pathname_composition(struct strbuf *path);
+void probe_utf8_pathname_composition(void);
 
 PREC_DIR *precompose_utf8_opendir(const char *dirname);
 struct dirent_prec_psx *precompose_utf8_readdir(PREC_DIR *dirp);

============================

And this is fix for David:

diff --git a/refs.h b/refs.h
index f499093..7dee497 100644
--- a/refs.h
+++ b/refs.h
@@ -670,7 +670,6 @@ typedef int (*ref_transaction_verify_fn)(struct
ref_transaction *transaction,
         unsigned int flags, struct strbuf *err);
 typedef int (*ref_transaction_commit_fn)(struct ref_transaction *transaction,
                      struct strbuf *err);
-typedef void (*ref_transaction_free_fn)(struct ref_transaction *transaction);
 
 /* reflog functions */
 typedef int (*for_each_reflog_ent_fn)(const char *refname,

Re: [PATCH 41/68] init: use strbufs to store paths

From: Jeff King <hidden>
Date: 2016-06-15 23:06:45

On Sun, Oct 04, 2015 at 08:31:31AM +0200, Torsten Bögershausen wrote:
quoted
That is the original signature, before my sprintf series. I do not mind
leaving that as-is, and simply cleaning up probe_utf8_pathname_composition
by using a strbuf internally there. Though I have to wonder if it even
needs us to pass _anything_ at that point. It could just call
git_path_buf("config%s", auml_nfd) itself. The whole reason to pass
anything was to let it reuse the buffer the caller had.

-Peff
Makes sense, here is V2:
Yeah, I think this is much nicer.

And because it decouples the interface between init-db.c and the
precompose code, it is easy to do it as a separate patch before the
init-db one.
quoted hunk
diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c
index b4dd3c7..64b85f2 100644
--- a/compat/precompose_utf8.c
+++ b/compat/precompose_utf8.c
@@ -8,6 +8,7 @@
 #include "cache.h"
 #include "utf8.h"
 #include "precompose_utf8.h"
+#include "strbuf.h"
I think this is actually redundant; it is part of cache.h included above
(and the precompose_utf8.h header file does not need to care anymore,
since the strbuf is not part of the interface).
-void probe_utf8_pathname_composition(struct strbuf *path)
+void probe_utf8_pathname_composition(void)
 {
+    struct strbuf sbuf = STRBUF_INIT;
     static const char *auml_nfc = "\xc3\xa4";
     static const char *auml_nfd = "\x61\xcc\x88";
-    size_t baselen = path->len;
+    const char *path;
I don't think we need this separate "path"; we can just access the
strbuf directly (that makes the diff a little noisier, but I think the
end result is simpler).
quoted hunk
diff --git a/compat/precompose_utf8.h b/compat/precompose_utf8.h
index 7fc7be5..a94e7c4 100644
--- a/compat/precompose_utf8.h
+++ b/compat/precompose_utf8.h
@@ -27,7 +27,7 @@ typedef struct {
 } PREC_DIR;
 
 void precompose_argv(int argc, const char **argv);
-void probe_utf8_pathname_composition(struct strbuf *path);
+void probe_utf8_pathname_composition(void);
I think we need a similar fix for the compat macro to build on non-Mac
platforms.

Here's a mini-series I came up with, which I hope is polished enough for
Junio to apply as a drop-in replacement for the "init: use strbufs"
patch from my original series. I compiled-tested it on Linux, with and
without precompose_utf8.o support hacked in. I don't have access to an
OS X machine to test on, so I'd appreciate confirmation that t3910 still
passes there.

  [1/3]: precompose_utf8: drop unused variable
  [2/3]: probe_utf8_pathname_composition: use internal strbuf
  [3/3]: init: use strbufs to store paths

-Peff

[PATCH 1/3] precompose_utf8: drop unused variable

From: Jeff King <hidden>
Date: 2016-06-15 23:06:45

The result of iconv is assigned to a variable, but we never
use it (instead, we check errno and whether the function
consumed all bytes). Let's drop the assignment, as it
triggers gcc's -Wunused-but-set-variable.

Signed-off-by: Jeff King <redacted>
---
This is obviously completely optional; I just needed it to get the
"precompose on Linux" hack to build at all with my usual "-Wall -Werror"
settings. I guess clang doesn't have a similar warning, or maybe people
on Macs are not as pedantic as I am.

 compat/precompose_utf8.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c
index 95fe849..044c686 100644
--- a/compat/precompose_utf8.c
+++ b/compat/precompose_utf8.c
@@ -139,9 +139,8 @@ struct dirent_prec_psx *precompose_utf8_readdir(PREC_DIR *prec_dir)
 				size_t inleft = namelenz;
 				char *outpos = &prec_dir->dirent_nfc->d_name[0];
 				size_t outsz = prec_dir->dirent_nfc->max_name_len;
-				size_t cnt;
 				errno = 0;
-				cnt = iconv(prec_dir->ic_precompose, &cp, &inleft, &outpos, &outsz);
+				iconv(prec_dir->ic_precompose, &cp, &inleft, &outpos, &outsz);
 				if (errno || inleft) {
 					/*
 					 * iconv() failed and errno could be E2BIG, EILSEQ, EINVAL, EBADF
-- 
2.6.0.455.ga3f9923

[PATCH 2/3] probe_utf8_pathname_composition: use internal strbuf

From: Jeff King <hidden>
Date: 2016-06-15 23:06:45

When we are initializing a .git directory, we may call
probe_utf8_pathname_composition to detect utf8 mangling. We
pass in a path buffer for it to use, and it blindly
strcpy()s into it, not knowing whether the buffer is large
enough to hold the result or not.

In practice this isn't a big deal, because the buffer we
pass in already contains "$GIT_DIR/config", and we append
only a few extra bytes to it. But we can easily do the right
thing just by calling git_path_buf ourselves. Technically
this results in a different pathname (before we appended our
utf8 characters to the "config" path, and now they get their
own files in $GIT_DIR), but that should not matter for our
purposes.

Signed-off-by: Jeff King <redacted>
---
I assume that "$GIT_DIR/$auml_nfc" is fine to perform this test based on
Torsten's patch showing the same simplification. If it matters, or if we
simply want to be ultra-conservative, changing the "%s" to "CoNfIg%s"
would yield identical behavior (but if it doesn't matter, I think I
prefer this as a simplification).

 builtin/init-db.c        |  2 +-
 compat/precompose_utf8.c | 18 ++++++++++--------
 compat/precompose_utf8.h |  2 +-
 git-compat-util.h        |  2 +-
 4 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/builtin/init-db.c b/builtin/init-db.c
index e7d0e31..89addda 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -312,7 +312,7 @@ static int create_default_files(const char *template_path)
 		strcpy(path + len, "CoNfIg");
 		if (!access(path, F_OK))
 			git_config_set("core.ignorecase", "true");
-		probe_utf8_pathname_composition(path, len);
+		probe_utf8_pathname_composition();
 	}
 
 	return reinit;
diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c
index 044c686..079070f 100644
--- a/compat/precompose_utf8.c
+++ b/compat/precompose_utf8.c
@@ -36,24 +36,26 @@ static size_t has_non_ascii(const char *s, size_t maxlen, size_t *strlen_c)
 }
 
 
-void probe_utf8_pathname_composition(char *path, int len)
+void probe_utf8_pathname_composition(void)
 {
+	struct strbuf path = STRBUF_INIT;
 	static const char *auml_nfc = "\xc3\xa4";
 	static const char *auml_nfd = "\x61\xcc\x88";
 	int output_fd;
 	if (precomposed_unicode != -1)
 		return; /* We found it defined in the global config, respect it */
-	strcpy(path + len, auml_nfc);
-	output_fd = open(path, O_CREAT|O_EXCL|O_RDWR, 0600);
+	git_path_buf(&path, "%s", auml_nfc);
+	output_fd = open(path.buf, O_CREAT|O_EXCL|O_RDWR, 0600);
 	if (output_fd >= 0) {
 		close(output_fd);
-		strcpy(path + len, auml_nfd);
-		precomposed_unicode = access(path, R_OK) ? 0 : 1;
+		git_path_buf(&path, "%s", auml_nfd);
+		precomposed_unicode = access(path.buf, R_OK) ? 0 : 1;
 		git_config_set("core.precomposeunicode", precomposed_unicode ? "true" : "false");
-		strcpy(path + len, auml_nfc);
-		if (unlink(path))
-			die_errno(_("failed to unlink '%s'"), path);
+		git_path_buf(&path, "%s", auml_nfc);
+		if (unlink(path.buf))
+			die_errno(_("failed to unlink '%s'"), path.buf);
 	}
+	strbuf_release(&path);
 }
 
 
diff --git a/compat/precompose_utf8.h b/compat/precompose_utf8.h
index 3b73585..a94e7c4 100644
--- a/compat/precompose_utf8.h
+++ b/compat/precompose_utf8.h
@@ -27,7 +27,7 @@ typedef struct {
 } PREC_DIR;
 
 void precompose_argv(int argc, const char **argv);
-void probe_utf8_pathname_composition(char *, int);
+void probe_utf8_pathname_composition(void);
 
 PREC_DIR *precompose_utf8_opendir(const char *dirname);
 struct dirent_prec_psx *precompose_utf8_readdir(PREC_DIR *dirp);
diff --git a/git-compat-util.h b/git-compat-util.h
index 348b9dc..9a3e559 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -229,7 +229,7 @@ typedef unsigned long uintptr_t;
 #else
 #define precompose_str(in,i_nfd2nfc)
 #define precompose_argv(c,v)
-#define probe_utf8_pathname_composition(a,b)
+#define probe_utf8_pathname_composition()
 #endif
 
 #ifdef MKDIR_WO_TRAILING_SLASH
-- 
2.6.0.455.ga3f9923

[PATCH 3/3] init: use strbufs to store paths

From: Jeff King <hidden>
Date: 2016-06-15 23:06:45

The init code predates strbufs, and uses PATH_MAX-sized
buffers along with many manual checks on intermediate sizes
(some of which make magic assumptions, such as that init
will not create a path inside .git longer than 50
characters).

We can simplify this greatly by using strbufs, which drops
some hard-to-verify strcpy calls in favor of git_path_buf.
While we're in the area, let's also convert existing calls
to git_path to the safer git_path_buf (our existing calls
were passed to pretty tame functions, and so were not a
problem, but it's easy to be consistent and safe here).

Note that we had an explicit test that "git init" rejects
long template directories. This comes from 32d1776 (init: Do
not segfault on big GIT_TEMPLATE_DIR environment variable,
2009-04-18). We can drop the test_must_fail here, as we now
accept this and need only confirm that we don't segfault,
which was the original point of the test.

Signed-off-by: Jeff King <redacted>
---
Same as the original, but minus the bits that touch the precompose
probing.

 builtin/init-db.c | 172 +++++++++++++++++++++++-------------------------------
 t/t0001-init.sh   |   4 +-
 2 files changed, 76 insertions(+), 100 deletions(-)
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 89addda..f59f407 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -36,10 +36,11 @@ static void safe_create_dir(const char *dir, int share)
 		die(_("Could not make %s writable by group"), dir);
 }
 
-static void copy_templates_1(char *path, int baselen,
-			     char *template, int template_baselen,
+static void copy_templates_1(struct strbuf *path, struct strbuf *template,
 			     DIR *dir)
 {
+	size_t path_baselen = path->len;
+	size_t template_baselen = template->len;
 	struct dirent *de;
 
 	/* Note: if ".git/hooks" file exists in the repository being
@@ -49,77 +50,64 @@ static void copy_templates_1(char *path, int baselen,
 	 * with the way the namespace under .git/ is organized, should
 	 * be really carefully chosen.
 	 */
-	safe_create_dir(path, 1);
+	safe_create_dir(path->buf, 1);
 	while ((de = readdir(dir)) != NULL) {
 		struct stat st_git, st_template;
-		int namelen;
 		int exists = 0;
 
+		strbuf_setlen(path, path_baselen);
+		strbuf_setlen(template, template_baselen);
+
 		if (de->d_name[0] == '.')
 			continue;
-		namelen = strlen(de->d_name);
-		if ((PATH_MAX <= baselen + namelen) ||
-		    (PATH_MAX <= template_baselen + namelen))
-			die(_("insanely long template name %s"), de->d_name);
-		memcpy(path + baselen, de->d_name, namelen+1);
-		memcpy(template + template_baselen, de->d_name, namelen+1);
-		if (lstat(path, &st_git)) {
+		strbuf_addstr(path, de->d_name);
+		strbuf_addstr(template, de->d_name);
+		if (lstat(path->buf, &st_git)) {
 			if (errno != ENOENT)
-				die_errno(_("cannot stat '%s'"), path);
+				die_errno(_("cannot stat '%s'"), path->buf);
 		}
 		else
 			exists = 1;
 
-		if (lstat(template, &st_template))
-			die_errno(_("cannot stat template '%s'"), template);
+		if (lstat(template->buf, &st_template))
+			die_errno(_("cannot stat template '%s'"), template->buf);
 
 		if (S_ISDIR(st_template.st_mode)) {
-			DIR *subdir = opendir(template);
-			int baselen_sub = baselen + namelen;
-			int template_baselen_sub = template_baselen + namelen;
+			DIR *subdir = opendir(template->buf);
 			if (!subdir)
-				die_errno(_("cannot opendir '%s'"), template);
-			path[baselen_sub++] =
-				template[template_baselen_sub++] = '/';
-			path[baselen_sub] =
-				template[template_baselen_sub] = 0;
-			copy_templates_1(path, baselen_sub,
-					 template, template_baselen_sub,
-					 subdir);
+				die_errno(_("cannot opendir '%s'"), template->buf);
+			strbuf_addch(path, '/');
+			strbuf_addch(template, '/');
+			copy_templates_1(path, template, subdir);
 			closedir(subdir);
 		}
 		else if (exists)
 			continue;
 		else if (S_ISLNK(st_template.st_mode)) {
-			char lnk[256];
-			int len;
-			len = readlink(template, lnk, sizeof(lnk));
-			if (len < 0)
-				die_errno(_("cannot readlink '%s'"), template);
-			if (sizeof(lnk) <= len)
-				die(_("insanely long symlink %s"), template);
-			lnk[len] = 0;
-			if (symlink(lnk, path))
-				die_errno(_("cannot symlink '%s' '%s'"), lnk, path);
+			struct strbuf lnk = STRBUF_INIT;
+			if (strbuf_readlink(&lnk, template->buf, 0) < 0)
+				die_errno(_("cannot readlink '%s'"), template->buf);
+			if (symlink(lnk.buf, path->buf))
+				die_errno(_("cannot symlink '%s' '%s'"),
+					  lnk.buf, path->buf);
+			strbuf_release(&lnk);
 		}
 		else if (S_ISREG(st_template.st_mode)) {
-			if (copy_file(path, template, st_template.st_mode))
-				die_errno(_("cannot copy '%s' to '%s'"), template,
-					  path);
+			if (copy_file(path->buf, template->buf, st_template.st_mode))
+				die_errno(_("cannot copy '%s' to '%s'"),
+					  template->buf, path->buf);
 		}
 		else
-			error(_("ignoring template %s"), template);
+			error(_("ignoring template %s"), template->buf);
 	}
 }
 
 static void copy_templates(const char *template_dir)
 {
-	char path[PATH_MAX];
-	char template_path[PATH_MAX];
-	int template_len;
+	struct strbuf path = STRBUF_INIT;
+	struct strbuf template_path = STRBUF_INIT;
+	size_t template_len;
 	DIR *dir;
-	const char *git_dir = get_git_dir();
-	int len = strlen(git_dir);
 	char *to_free = NULL;
 
 	if (!template_dir)
@@ -132,26 +120,23 @@ static void copy_templates(const char *template_dir)
 		free(to_free);
 		return;
 	}
-	template_len = strlen(template_dir);
-	if (PATH_MAX <= (template_len+strlen("/config")))
-		die(_("insanely long template path %s"), template_dir);
-	strcpy(template_path, template_dir);
-	if (template_path[template_len-1] != '/') {
-		template_path[template_len++] = '/';
-		template_path[template_len] = 0;
-	}
-	dir = opendir(template_path);
+
+	strbuf_addstr(&template_path, template_dir);
+	strbuf_complete(&template_path, '/');
+	template_len = template_path.len;
+
+	dir = opendir(template_path.buf);
 	if (!dir) {
 		warning(_("templates not found %s"), template_dir);
 		goto free_return;
 	}
 
 	/* Make sure that template is from the correct vintage */
-	strcpy(template_path + template_len, "config");
+	strbuf_addstr(&template_path, "config");
 	repository_format_version = 0;
 	git_config_from_file(check_repository_format_version,
-			     template_path, NULL);
-	template_path[template_len] = 0;
+			     template_path.buf, NULL);
+	strbuf_setlen(&template_path, template_len);
 
 	if (repository_format_version &&
 	    repository_format_version != GIT_REPO_VERSION) {
@@ -162,17 +147,15 @@ static void copy_templates(const char *template_dir)
 		goto close_free_return;
 	}
 
-	memcpy(path, git_dir, len);
-	if (len && path[len - 1] != '/')
-		path[len++] = '/';
-	path[len] = 0;
-	copy_templates_1(path, len,
-			 template_path, template_len,
-			 dir);
+	strbuf_addstr(&path, get_git_dir());
+	strbuf_complete(&path, '/');
+	copy_templates_1(&path, &template_path, dir);
 close_free_return:
 	closedir(dir);
 free_return:
 	free(to_free);
+	strbuf_release(&path);
+	strbuf_release(&template_path);
 }
 
 static int git_init_db_config(const char *k, const char *v, void *cb)
@@ -199,28 +182,20 @@ static int needs_work_tree_config(const char *git_dir, const char *work_tree)
 
 static int create_default_files(const char *template_path)
 {
-	const char *git_dir = get_git_dir();
-	unsigned len = strlen(git_dir);
-	static char path[PATH_MAX];
 	struct stat st1;
+	struct strbuf buf = STRBUF_INIT;
+	char *path;
 	char repo_version_string[10];
 	char junk[2];
 	int reinit;
 	int filemode;
 
-	if (len > sizeof(path)-50)
-		die(_("insane git directory %s"), git_dir);
-	memcpy(path, git_dir, len);
-
-	if (len && path[len-1] != '/')
-		path[len++] = '/';
-
 	/*
 	 * Create .git/refs/{heads,tags}
 	 */
-	safe_create_dir(git_path("refs"), 1);
-	safe_create_dir(git_path("refs/heads"), 1);
-	safe_create_dir(git_path("refs/tags"), 1);
+	safe_create_dir(git_path_buf(&buf, "refs"), 1);
+	safe_create_dir(git_path_buf(&buf, "refs/heads"), 1);
+	safe_create_dir(git_path_buf(&buf, "refs/tags"), 1);
 
 	/* Just look for `init.templatedir` */
 	git_config(git_init_db_config, NULL);
@@ -244,16 +219,16 @@ static int create_default_files(const char *template_path)
 	 */
 	if (shared_repository) {
 		adjust_shared_perm(get_git_dir());
-		adjust_shared_perm(git_path("refs"));
-		adjust_shared_perm(git_path("refs/heads"));
-		adjust_shared_perm(git_path("refs/tags"));
+		adjust_shared_perm(git_path_buf(&buf, "refs"));
+		adjust_shared_perm(git_path_buf(&buf, "refs/heads"));
+		adjust_shared_perm(git_path_buf(&buf, "refs/tags"));
 	}
 
 	/*
 	 * Create the default symlink from ".git/HEAD" to the "master"
 	 * branch, if it does not exist yet.
 	 */
-	strcpy(path + len, "HEAD");
+	path = git_path_buf(&buf, "HEAD");
 	reinit = (!access(path, R_OK)
 		  || readlink(path, junk, sizeof(junk)-1) != -1);
 	if (!reinit) {
@@ -266,10 +241,8 @@ static int create_default_files(const char *template_path)
 		  "%d", GIT_REPO_VERSION);
 	git_config_set("core.repositoryformatversion", repo_version_string);
 
-	path[len] = 0;
-	strcpy(path + len, "config");
-
 	/* Check filemode trustability */
+	path = git_path_buf(&buf, "config");
 	filemode = TEST_FILEMODE;
 	if (TEST_FILEMODE && !lstat(path, &st1)) {
 		struct stat st2;
@@ -290,14 +263,13 @@ static int create_default_files(const char *template_path)
 		/* allow template config file to override the default */
 		if (log_all_ref_updates == -1)
 		    git_config_set("core.logallrefupdates", "true");
-		if (needs_work_tree_config(git_dir, work_tree))
+		if (needs_work_tree_config(get_git_dir(), work_tree))
 			git_config_set("core.worktree", work_tree);
 	}
 
 	if (!reinit) {
 		/* Check if symlink is supported in the work tree */
-		path[len] = 0;
-		strcpy(path + len, "tXXXXXX");
+		path = git_path_buf(&buf, "tXXXXXX");
 		if (!close(xmkstemp(path)) &&
 		    !unlink(path) &&
 		    !symlink("testing", path) &&
@@ -308,31 +280,35 @@ static int create_default_files(const char *template_path)
 			git_config_set("core.symlinks", "false");
 
 		/* Check if the filesystem is case-insensitive */
-		path[len] = 0;
-		strcpy(path + len, "CoNfIg");
+		path = git_path_buf(&buf, "CoNfIg");
 		if (!access(path, F_OK))
 			git_config_set("core.ignorecase", "true");
 		probe_utf8_pathname_composition();
 	}
 
+	strbuf_release(&buf);
 	return reinit;
 }
 
 static void create_object_directory(void)
 {
-	const char *object_directory = get_object_directory();
-	int len = strlen(object_directory);
-	char *path = xmalloc(len + 40);
+	struct strbuf path = STRBUF_INIT;
+	size_t baselen;
+
+	strbuf_addstr(&path, get_object_directory());
+	baselen = path.len;
+
+	safe_create_dir(path.buf, 1);
 
-	memcpy(path, object_directory, len);
+	strbuf_setlen(&path, baselen);
+	strbuf_addstr(&path, "/pack");
+	safe_create_dir(path.buf, 1);
 
-	safe_create_dir(object_directory, 1);
-	strcpy(path+len, "/pack");
-	safe_create_dir(path, 1);
-	strcpy(path+len, "/info");
-	safe_create_dir(path, 1);
+	strbuf_setlen(&path, baselen);
+	strbuf_addstr(&path, "/info");
+	safe_create_dir(path.buf, 1);
 
-	free(path);
+	strbuf_release(&path);
 }
 
 int set_git_dir_init(const char *git_dir, const char *real_git_dir,
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 7de8d85..f91bbcf 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -202,8 +202,8 @@ test_expect_success 'init honors global core.sharedRepository' '
 	x$(git config -f shared-honor-global/.git/config core.sharedRepository)
 '
 
-test_expect_success 'init rejects insanely long --template' '
-	test_must_fail git init --template=$(printf "x%09999dx" 1) test
+test_expect_success 'init allows insanely long --template' '
+	git init --template=$(printf "x%09999dx" 1) test
 '
 
 test_expect_success 'init creates a new directory' '
-- 
2.6.0.455.ga3f9923

Re: [PATCH 1/3] precompose_utf8: drop unused variable

From: Torsten Bögershausen <hidden>
Date: 2016-06-15 23:06:46

On 2015-10-05 05.43, Jeff King wrote:
[]
The whole series looks good, cleans up my mess and passes t3910.
Thanks for that.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help