Re: [msysGit] Re: Pull request for msysGit patches

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Junio,

On 9/28/2010 3:10 PM, Junio C Hamano wrote:
Pat Thoyts<patthoyts@xxxxxxxxxxxxxxxxxxxxx>  writes:
Junio,
The msysGit tree currently tracks some 50+ patches on top of 'next'. I
have gathered 42 of these that look good to move upstream.

A quick and superficial review follows.
----------------------------------------------------------------
abspath.c

@@ -108,10 +108,15 @@ const char *make_nonrelative_path(const char *path)
  		if (strlcpy(buf, path, PATH_MAX)>= PATH_MAX)
  			die("Too long path: %.*s", 60, path);
  	} else {
+		size_t len;
+		const char *fmt;
  		const char *cwd = get_pwd_cwd();
  		if (!cwd)
  			die_errno("Cannot determine the current working directory");
-		if (snprintf(buf, PATH_MAX, "%s/%s", cwd, path)>= PATH_MAX)
+		len = strlen(cwd);
+		/* For cwd c:/, return c:/foo rather than URL-like c://foo */

For the patch to be regression free, the logic described by this comment
requires get_pwd_cmd() to return a string with trailing dir-sep only at
slash.  IOW, if you see any non-root path returned with a trailing dir-sep
for whatever reason, you are changing the behaviour in that case as well,
and that clearly is not "fix at the root level".
But if you label this as "avoid duplicated dir-sep", everything flows
smoothly ;-).

I am the author of this patch. Do I understand correctly that your primary concern is that you find the comment misleading? I consider the code

  fmt = (len > 0 && is_dir_sep(cwd[len-1])) ? "%s%s" : "%s/%s";

sufficiently self-documenting that the comment is superfluous, and would be happy to remove the comment. Would you prefer the patch submitted with the comment removed?

-- ES
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]