Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:47:43
Matthieu Moy [off-list ref] writes:
My version is made a bit simpler by using strbuf for string manipulation in expand_user_path.
Nice to see people keeping track of issues that we tried to address but didn't complete.
I'm not sure I fully adressed Junio's point here:
We'll see ;-)
I'm just copying back into the static buffer to let enter_repo() do the same string manipulation as it used to do (concatenate with .git suffixes). I think the whole enter_repo could use strbuf instead of static buffers, but that's a different point (probably made easier by my patch).
quoted hunk
diff --git a/config.c b/config.c index c644061..0fcc4ce 100644 --- a/config.c +++ b/config.c@@ -351,6 +351,15 @@ int git_config_string(const char **dest, const char *var, const char *value) return 0; } +int git_config_pathname(const char **dest, const char *var, const char *value) { + if (!value) + return config_error_nonbool(var); + *dest = expand_user_path(value); + if (!*dest) + die("Failed to expand user dir in: '%s'", value); + return 0; +} +
quoted hunk
@@ -207,43 +208,49 @@ int validate_headref(const char *path) return -1; } -static char *user_path(char *buf, char *path, int sz) +static inline struct passwd *getpw_str(const char *username, size_t len) { + if (len == 0) + return getpwuid(getuid()); + struct passwd *pw;
Decl-after-statement. Do you need this special case of passing a zero-length string (not NULL pointer as a string) to getpw_str() to grab the current user? Which codepath is this needed?
+ char *username_z = xmalloc(len + 1); + memcpy(username_z, username, len); + username_z[len] = '\0'; + pw = getpwnam(username_z); + free(username_z); + return pw; +}
+/*
+ * Return a string with ~ and ~user expanded via getpw*. If buf != NULL, then
+ * it is a newly allocated string. Returns NULL on getpw failure or if
+ * path is NULL.
+ */
+char *expand_user_path(const char *path)
+{
+ struct strbuf user_path = STRBUF_INIT;
+ char * first_slash = strchrnul(path, '/');
+ char * to_copy;Style: "char *first_slash" Should't "to_copy" be initialized to "path" here? What do you copy when path[0] is '/'?
+ if (path == NULL)
+ goto return_null;
+
+ if (path[0] == '~') {
+ const char *username = path + 1;
+ size_t username_len = first_slash - username;
+ struct passwd *pw = getpw_str(username, username_len);
+ if (!pw)
+ goto return_null;
+ strbuf_add(&user_path, pw->pw_dir, strlen(pw->pw_dir));
+ to_copy = first_slash;
+ } else if (path[0] != '/') {
+ to_copy = path;
}
- return buf;
+ strbuf_add(&user_path, to_copy, strlen(to_copy));
+ return strbuf_detach(&user_path, NULL);
+return_null:
+ strbuf_release(&user_path);
+ return NULL;
}quoted hunk
/*@@ -291,8 +298,17 @@ 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 - 10 < strlen(newpath))) { + if (path != newpath) + free(newpath);
Your expand_user_path() never returns the original, no?
return NULL; + } + /* Copy back into the static buffer. A pity + since newpath was not bounded, but other + branches of the if are limited by PATH_MAX + anyway. */ + strcpy(used_path, newpath); free(newpath); strcpy(validated_path, path); path = used_path; }
Heh, in a sense you _did_ address my point by punting, but that is Ok. As
you said earlier that would be a good topic of a separate patch.
/*
* By the way, we write our
* multi-line comments like
* this.
*/