Re: [PATCH v4] Expand ~ and ~user in core.excludesfile, commit.template
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:45:15
Karl Chen [off-list ref] 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 <redacted>
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]);