Re: [PATCH v2 03/17] safe_create_leading_directories(): add explicit slash pointer
On Tue, Jan 7, 2014 at 6:41 PM, Junio C Hamano gits...@pobox.com wrote: Let me know if you would like me to merge or rebase the is_dir_sep() changes into this patch series. I'd want SSchuberth and windows folks to be at least aware of this series and preferrably see that they offer inputs to this series, making their is_dir_sep() change just one step in this series. That way I have one less series to worry about ;-) Thanks Junio for pointing out Michael's series to me and resolving the initial merge conflict. However, as written in my reply to Michael's mail of today, I'd prefer to take Michael's patch that applies cleanly on top of v3 of his mh/safe-create-leading-directories instead of your merge conflict resolution. -- Sebastian Schuberth -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 03/17] safe_create_leading_directories(): add explicit slash pointer
On 01/06/2014 07:32 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: Keep track of the position of the slash character independently of pos, thereby making the purpose of each variable clearer and working towards other upcoming changes. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- This step has an interaction with $gmane/239878 where Windows folks want it to pay attention to is_dir_sep()---over there, a backslash could separate directory path components. AFAIK, the function was meant to be used only on paths we internally generate, and the paths we internally generate all are slash separated, so it could be argued that feeding a path, whose path components are separated by backslashes, that we obtained from the end user without converting it to the internal form in some codepaths (e.g. $there in git clone $url $there) are bugs we acquired over time that need to be fixed, but it is easy enough to use is_dir_sep() here to work it around, and doing so will not negatively affect 1. UNIX-only projects by forbidding use of a byte with backslash in it as a path component character (yes, I am imagining using Shift-JIS that can use a backslash as the second byte of two-byte character in the pathname on UNIX); and 2. UNIX-and-Windows mixed projects, as you cannot sanely use such a pathname with backslash as part of a path component if its tree needs to be checked out on Windows. I agree that it would be reasonable to use is_dir_sep() in the implementation of this function, at least unless/until somebody does the work to figure out whether callers should really only be passing it forward-slash-normalized paths. Please be careful, though, because I don't think this function is capable of handling arbitrary Windows paths, like for example //host/path format, either before or after my change. Let me know if you would like me to merge or rebase the is_dir_sep() changes into this patch series. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 03/17] safe_create_leading_directories(): add explicit slash pointer
Michael Haggerty mhag...@alum.mit.edu writes: I agree that it would be reasonable to use is_dir_sep() in the implementation of this function, at least unless/until somebody does the work to figure out whether callers should really only be passing it forward-slash-normalized paths. Please be careful, though, because I don't think this function is capable of handling arbitrary Windows paths, like for example //host/path format, either before or after my change. Let me know if you would like me to merge or rebase the is_dir_sep() changes into this patch series. I'd want SSchuberth and windows folks to be at least aware of this series and preferrably see that they offer inputs to this series, making their is_dir_sep() change just one step in this series. That way I have one less series to worry about ;-) Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 03/17] safe_create_leading_directories(): add explicit slash pointer
Michael Haggerty mhag...@alum.mit.edu writes:
Keep track of the position of the slash character independently of
pos, thereby making the purpose of each variable clearer and
working towards other upcoming changes.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
This step has an interaction with $gmane/239878 where Windows folks
want it to pay attention to is_dir_sep()---over there, a backslash
could separate directory path components.
AFAIK, the function was meant to be used only on paths we internally
generate, and the paths we internally generate all are slash
separated, so it could be argued that feeding a path, whose path
components are separated by backslashes, that we obtained from the
end user without converting it to the internal form in some
codepaths (e.g. $there in git clone $url $there) are bugs we
acquired over time that need to be fixed, but it is easy enough to
use is_dir_sep() here to work it around, and doing so will
not negatively affect
1. UNIX-only projects by forbidding use of a byte with backslash in
it as a path component character (yes, I am imagining using
Shift-JIS that can use a backslash as the second byte of
two-byte character in the pathname on UNIX); and
2. UNIX-and-Windows mixed projects, as you cannot sanely use such a
pathname with backslash as part of a path component if its tree
needs to be checked out on Windows.
sha1_file.c | 20 +++-
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index cc9957e..197766d 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -111,19 +111,21 @@ int safe_create_leading_directories(char *path)
while (pos) {
struct stat st;
+ char *slash = strchr(pos, '/');
- pos = strchr(pos, '/');
- if (!pos)
+ if (!slash)
break;
- while (*++pos == '/')
- ;
+ while (*(slash + 1) == '/')
+ slash++;
+ pos = slash + 1;
if (!*pos)
break;
- *--pos = '\0';
+
+ *slash = '\0';
if (!stat(path, st)) {
/* path exists */
if (!S_ISDIR(st.st_mode)) {
- *pos = '/';
+ *slash = '/';
return -3;
}
} else if (mkdir(path, 0777)) {
@@ -131,14 +133,14 @@ int safe_create_leading_directories(char *path)
!stat(path, st) S_ISDIR(st.st_mode)) {
; /* somebody created it since we checked */
} else {
- *pos = '/';
+ *slash = '/';
return -1;
}
} else if (adjust_shared_perm(path)) {
- *pos = '/';
+ *slash = '/';
return -2;
}
- *pos++ = '/';
+ *slash = '/';
}
return 0;
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
