git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Matthieu Moy <Matthieu.Moy@imag.fr>
Cc: git@vger.kernel.org, Karl Chen <quarl@quarl.org>
Subject: Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template
Date: Mon, 16 Nov 2009 22:49:27 -0800	[thread overview]
Message-ID: <7vy6m5hci0.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <7v3a4enkzt.fsf@alter.siamese.dyndns.org> (Junio C. Hamano's message of "Mon\, 16 Nov 2009 14\:49\:26 -0800")

I'd like to squash this to your patch, based on the earlier review
comments.

I didn't mention about the first hunk; it is just a style.  An opening
brace that begins a function body comes at column 1.

Also first_slash and to_copy are made const pointers, as they do not have
to touch the region of memory they point to (otherwise you cannot assign
path to to_copy without getting warned).

 config.c |    3 ++-
 path.c   |   32 ++++++++++++++------------------
 2 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/config.c b/config.c
index 0fcc4ce..b3d1ff4 100644
--- a/config.c
+++ b/config.c
@@ -351,7 +351,8 @@ 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) {
+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);
diff --git a/path.c b/path.c
index 009c8e0..2470f78 100644
--- a/path.c
+++ b/path.c
@@ -208,11 +208,8 @@ int validate_headref(const char *path)
 	return -1;
 }
 
-static inline struct passwd *getpw_str(const char *username, size_t len)
+static struct passwd *getpw_str(const char *username, size_t len)
 {
-	if (len == 0)
-		return getpwuid(getuid());
-
 	struct passwd *pw;
 	char *username_z = xmalloc(len + 1);
 	memcpy(username_z, username, len);
@@ -223,18 +220,18 @@ static inline struct passwd *getpw_str(const char *username, size_t len)
 }
 
 /*
- * 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.
+ * 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;
+	const char *first_slash = strchrnul(path, '/');
+	const char *to_copy = path;
+
 	if (path == NULL)
 		goto return_null;
-
 	if (path[0] == '~') {
 		const char *username = path + 1;
 		size_t username_len = first_slash - username;
@@ -243,8 +240,6 @@ char *expand_user_path(const char *path)
 			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;
 	}
 	strbuf_add(&user_path, to_copy, strlen(to_copy));
 	return strbuf_detach(&user_path, NULL);
@@ -300,14 +295,15 @@ char *enter_repo(char *path, int strict)
 		if (path[0] == '~') {
 			char *newpath = expand_user_path(path);
 			if (!newpath || (PATH_MAX - 10 < strlen(newpath))) {
-				if (path != newpath)
-					free(newpath);
+				free(newpath);
 				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. */
+			/*
+			 * 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;
-- 
1.6.5.3.283.g4b054

  reply	other threads:[~2009-11-17  6:49 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-16 10:07 [PATCH] Expand ~ and ~user in core.excludesfile, commit.template Matthieu Moy
2009-11-16 22:49 ` Junio C Hamano
2009-11-17  6:49   ` Junio C Hamano [this message]
2009-11-17  8:59     ` Matthieu Moy
2009-11-16 23:47 ` Jakub Narebski
2009-11-17  6:22   ` Junio C Hamano
2009-11-17  8:57     ` Matthieu Moy
2009-11-17 13:30       ` Jakub Narebski
2009-11-17  9:30     ` Jakub Narebski
2009-11-17  7:34 ` Jeff King
2009-11-17  7:49   ` Mike Hommey
2009-11-17 21:20     ` Andreas Schwab
2009-11-17 22:16       ` Junio C Hamano
2009-11-18  0:42         ` Andreas Schwab
2009-11-18  7:24         ` Matthieu Moy
2009-11-17  8:53   ` Matthieu Moy
2009-11-17  8:56     ` Jeff King
2009-11-17 17:24 ` [PATCH v2] " Matthieu Moy
2009-11-18  7:29   ` [PATCH v3] " Matthieu Moy
2009-11-18  8:58     ` [PATCH v4] " Matthieu Moy
2009-11-19 15:21       ` [PATCH] expand_user_path: expand ~ to $HOME, not to the actual homedir Matthieu Moy
2009-11-19 15:23         ` Jeff King
2009-11-19 16:32           ` Matthieu Moy
2009-11-19 18:12       ` [PATCH v4] Expand ~ and ~user in core.excludesfile, commit.template Junio C Hamano

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=7vy6m5hci0.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=Matthieu.Moy@imag.fr \
    --cc=git@vger.kernel.org \
    --cc=quarl@quarl.org \
    /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).