Re: [PATCH v4] Expand ~ and ~user in core.excludesfile, commit.template

[Junio C Hamano <gitster@pobox.com>]

Karl Chen <quarl@cs.berkeley.edu> writes:

> These config variables are parsed to substitute ~ and ~user with getpw
> entries.
>
> user_path() refactored into new function expand_user_path(), to allow
> dynamically allocating the return buffer.
>
> Signed-off-by: Karl Chen <quarl@quarl.org>

Thanks.

> ... Anyway, I accept the color you picked for this
> bikeshed.

I do not think Documentation/CodingStyle is bikesheding but just behaving
like Romans do while in Rome, so that the end result will blend in better.

>     Hannes> You really should use the strbuf API here. Look for
>     Hannes> strbuf_detach() in the existing code.
>
> Unfortunately expand_user_path() needs to support both a fixed
> buffer and mallocing return.  I don't think the strbuf API can do
> that easily?

I do not see any strong reason why the single caller of user_path() has to
keep using the static allocation.  Would it help to reduce the complexity
of your expand_user_path() implementation, if we fixed the caller along
the lines of this patch (untested, but just to illustrate the point)?

 path.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git i/path.c w/path.c
index 76e8872..c5b253c 100644
--- i/path.c
+++ w/path.c
@@ -221,19 +221,22 @@ char *enter_repo(char *path, int strict)
 		if (PATH_MAX <= len)
 			return NULL;
 		if (path[0] == '~') {
-			if (!user_path(used_path, path, PATH_MAX))
+			char *newpath = expand_user_path(path);
+			if (!newpath || (PATH_MAX <= strlen(newpath))) {
+				if (path != newpath)
+					free(newpath);
 				return NULL;
+			}
 			strcpy(validated_path, path);
-			path = used_path;
-		}
-		else if (PATH_MAX - 10 < len)
-			return NULL;
-		else {
+			path = newpath;
+		} else {
 			path = strcpy(used_path, path);
 			strcpy(validated_path, path);
 		}
 		len = strlen(path);
 		for (i = 0; suffix[i]; i++) {
+			if (PATH_MAX <= strlen(suffix[i] + len))
+				continue;
 			strcpy(path + len, suffix[i]);
 			if (!access(path, F_OK)) {
 				strcat(validated_path, suffix[i]);