git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/4] enter_repo: do not modify input
@ 2011-10-04 20:02 Phil Hord
  2011-10-04 20:24 ` Junio C Hamano
  0 siblings, 1 reply; 5+ messages in thread
From: Phil Hord @ 2011-10-04 20:02 UTC (permalink / raw)
  To: git, Erik Faye-Lund

From: Erik Faye-Lund <kusmabite@gmail.com>

entr_repo(..., 0) currently modifies the input to strip away
trailing slashes. This means that we some times need to copy the
input to keep the original.

Change it to unconditionally copy it into the used_path buffer so
we can safely use the input without having to copy it. Also store
a working copy in validated_path up-front before we start
resolving anything.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
Signed-off-by: Phil Hord <hordp@cisco.com>
---
This is Erik Faye-Lund's patch with some minor cleanup by me.  I sent
this to the list earlier today for Erik, but I'm including it here
again as part of this series.  Not sure what the etiquette is on this,
but this way seemed safest.

diff --git a/cache.h b/cache.h
index 9994a3c..7eeb8cf 100644
--- a/cache.h
+++ b/cache.h
@@ -734,7 +734,7 @@ int safe_create_leading_directories(char *path);
 int safe_create_leading_directories_const(const char *path);
 int mkdir_in_gitdir(const char *path);
 extern char *expand_user_path(const char *path);
-char *enter_repo(char *path, int strict);
+const char *enter_repo(const char *path, int strict);
 static inline int is_absolute_path(const char *path)
 {
 	return is_dir_sep(path[0]) || has_dos_drive_prefix(path);
diff --git a/daemon.c b/daemon.c
index 4c8346d..9253192 100644
--- a/daemon.c
+++ b/daemon.c
@@ -108,11 +108,11 @@ static void NORETURN daemon_die(const char *err,
va_list params)
 	exit(1);
 }

-static char *path_ok(char *directory)
+static const char *path_ok(char *directory)
 {
 	static char rpath[PATH_MAX];
 	static char interp_path[PATH_MAX];
-	char *path;
+	const char *path;
 	char *dir;

 	dir = directory;
diff --git a/path.c b/path.c
index 6f3f5d5..01028f2 100644
--- a/path.c
+++ b/path.c
@@ -283,7 +283,7 @@ return_null:
  * links.  User relative paths are also returned as they are given,
  * except DWIM suffixing.
  */
-char *enter_repo(char *path, int strict)
+const char *enter_repo(const char *path, int strict)
 {
 	static char used_path[PATH_MAX];
 	static char validated_path[PATH_MAX];
@@ -297,14 +297,16 @@ char *enter_repo(char *path, int strict)
 		};
 		int len = strlen(path);
 		int i;
-		while ((1 < len) && (path[len-1] == '/')) {
-			path[len-1] = 0;
+		while ((1 < len) && (path[len-1] == '/'))
 			len--;
-		}
+
 		if (PATH_MAX <= len)
 			return NULL;
-		if (path[0] == '~') {
-			char *newpath = expand_user_path(path);
+		strncpy(used_path, path, len); used_path[len] = 0 ;
+		strcpy(validated_path, used_path);
+
+		if (used_path[0] == '~') {
+			char *newpath = expand_user_path(used_path);
 			if (!newpath || (PATH_MAX - 10 < strlen(newpath))) {
 				free(newpath);
 				return NULL;
@@ -316,24 +318,18 @@ char *enter_repo(char *path, int strict)
 			 * anyway.
 			 */
 			strcpy(used_path, newpath); free(newpath);
-			strcpy(validated_path, path);
-			path = used_path;
 		}
 		else if (PATH_MAX - 10 < len)
 			return NULL;
-		else {
-			path = strcpy(used_path, path);
-			strcpy(validated_path, path);
-		}
-		len = strlen(path);
+		len = strlen(used_path);
 		for (i = 0; suffix[i]; i++) {
-			strcpy(path + len, suffix[i]);
-			if (!access(path, F_OK)) {
+			strcpy(used_path + len, suffix[i]);
+			if (!access(used_path, F_OK)) {
 				strcat(validated_path, suffix[i]);
 				break;
 			}
 		}
-		if (!suffix[i] || chdir(path))
+		if (!suffix[i] || chdir(used_path))
 			return NULL;
 		path = validated_path;
 	}
-- 
1.7.7.503.g26392.dirty

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/4] enter_repo: do not modify input
  2011-10-04 20:02 Phil Hord
@ 2011-10-04 20:24 ` Junio C Hamano
  0 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2011-10-04 20:24 UTC (permalink / raw)
  To: Phil Hord; +Cc: git, Erik Faye-Lund

Phil Hord <phil.hord@gmail.com> writes:

> diff --git a/daemon.c b/daemon.c
> index 4c8346d..9253192 100644
> --- a/daemon.c
> +++ b/daemon.c
> @@ -108,11 +108,11 @@ static void NORETURN daemon_die(const char *err,
> va_list params)

Corrupt and unappliable patch.

> -		if (path[0] == '~') {
> -			char *newpath = expand_user_path(path);
> +		strncpy(used_path, path, len); used_path[len] = 0 ;

Do not write two statements on a single line; extra SP before ';'.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/4] enter_repo: do not modify input
@ 2011-10-04 22:51 Phil Hord
  2011-10-04 23:01 ` Phil Hord
  0 siblings, 1 reply; 5+ messages in thread
From: Phil Hord @ 2011-10-04 22:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Erik Faye-Lund, phil.hord

From: Erik Faye-Lund <kusmabite@gmail.com>

entr_repo(..., 0) currently modifies the input to strip away
trailing slashes. This means that we some times need to copy the
input to keep the original.

Change it to unconditionally copy it into the used_path buffer so
we can safely use the input without having to copy it. Also store
a working copy in validated_path up-front before we start
resolving anything.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
Signed-off-by: Phil Hord <hordp@cisco.com>

diff --git a/cache.h b/cache.h
index 9994a3c..7eeb8cf 100644
--- a/cache.h
+++ b/cache.h
@@ -734,7 +734,7 @@ int safe_create_leading_directories(char *path);
 int safe_create_leading_directories_const(const char *path);
 int mkdir_in_gitdir(const char *path);
 extern char *expand_user_path(const char *path);
-char *enter_repo(char *path, int strict);
+const char *enter_repo(const char *path, int strict);
 static inline int is_absolute_path(const char *path)
 {
     return is_dir_sep(path[0]) || has_dos_drive_prefix(path);
diff --git a/daemon.c b/daemon.c
index 4c8346d..9253192 100644
--- a/daemon.c
+++ b/daemon.c
@@ -108,11 +108,11 @@ static void NORETURN daemon_die(const char *err,
va_list params)
     exit(1);
 }
 
-static char *path_ok(char *directory)
+static const char *path_ok(char *directory)
 {
     static char rpath[PATH_MAX];
     static char interp_path[PATH_MAX];
-    char *path;
+    const char *path;
     char *dir;
 
     dir = directory;
diff --git a/path.c b/path.c
index 6f3f5d5..f3d96aa 100644
--- a/path.c
+++ b/path.c
@@ -283,7 +283,7 @@ return_null:
  * links.  User relative paths are also returned as they are given,
  * except DWIM suffixing.
  */
-char *enter_repo(char *path, int strict)
+const char *enter_repo(const char *path, int strict)
 {
     static char used_path[PATH_MAX];
     static char validated_path[PATH_MAX];
@@ -297,14 +297,17 @@ char *enter_repo(char *path, int strict)
         };
         int len = strlen(path);
         int i;
-        while ((1 < len) && (path[len-1] == '/')) {
-            path[len-1] = 0;
+        while ((1 < len) && (path[len-1] == '/'))
             len--;
-        }
+
         if (PATH_MAX <= len)
             return NULL;
-        if (path[0] == '~') {
-            char *newpath = expand_user_path(path);
+        strncpy(used_path, path, len);
+        used_path[len] = 0;
+        strcpy(validated_path, used_path);
+
+        if (used_path[0] == '~') {
+            char *newpath = expand_user_path(used_path);
             if (!newpath || (PATH_MAX - 10 < strlen(newpath))) {
                 free(newpath);
                 return NULL;
@@ -316,24 +319,18 @@ char *enter_repo(char *path, int strict)
              * anyway.
              */
             strcpy(used_path, newpath); free(newpath);
-            strcpy(validated_path, path);
-            path = used_path;
         }
         else if (PATH_MAX - 10 < len)
             return NULL;
-        else {
-            path = strcpy(used_path, path);
-            strcpy(validated_path, path);
-        }
-        len = strlen(path);
+        len = strlen(used_path);
         for (i = 0; suffix[i]; i++) {
-            strcpy(path + len, suffix[i]);
-            if (!access(path, F_OK)) {
+            strcpy(used_path + len, suffix[i]);
+            if (!access(used_path, F_OK)) {
                 strcat(validated_path, suffix[i]);
                 break;
             }
         }
-        if (!suffix[i] || chdir(path))
+        if (!suffix[i] || chdir(used_path))
             return NULL;
         path = validated_path;
     }
-- 
1.7.7.503.g26392.dirty

 

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/4] enter_repo: do not modify input
  2011-10-04 22:51 [PATCH 1/4] enter_repo: do not modify input Phil Hord
@ 2011-10-04 23:01 ` Phil Hord
  2011-10-04 23:06   ` Phil Hord
  0 siblings, 1 reply; 5+ messages in thread
From: Phil Hord @ 2011-10-04 23:01 UTC (permalink / raw)
  To: Phil Hord; +Cc: Junio C Hamano, git, Erik Faye-Lund

On Tue, Oct 4, 2011 at 6:51 PM, Phil Hord <hordp@cisco.com> wrote:
> From: Erik Faye-Lund <kusmabite@gmail.com>
>
> +++ b/daemon.c
> @@ -108,11 +108,11 @@ static void NORETURN daemon_die(const char *err,
> va_list params)
>     exit(1);

Blast!  Borked again.

I'll re-work on this and try again tomorrow.

P

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/4] enter_repo: do not modify input
  2011-10-04 23:01 ` Phil Hord
@ 2011-10-04 23:06   ` Phil Hord
  0 siblings, 0 replies; 5+ messages in thread
From: Phil Hord @ 2011-10-04 23:06 UTC (permalink / raw)
  To: Phil Hord; +Cc: Junio C Hamano, git, Erik Faye-Lund

From: Erik Faye-Lund <kusmabite@gmail.com>

entr_repo(..., 0) currently modifies the input to strip away
trailing slashes. This means that we some times need to copy the
input to keep the original.

Change it to unconditionally copy it into the used_path buffer so
we can safely use the input without having to copy it. Also store
a working copy in validated_path up-front before we start
resolving anything.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
Signed-off-by: Phil Hord <hordp@cisco.com>

diff --git a/cache.h b/cache.h
index 9994a3c..7eeb8cf 100644
--- a/cache.h
+++ b/cache.h
@@ -734,7 +734,7 @@ int safe_create_leading_directories(char *path);
 int safe_create_leading_directories_const(const char *path);
 int mkdir_in_gitdir(const char *path);
 extern char *expand_user_path(const char *path);
-char *enter_repo(char *path, int strict);
+const char *enter_repo(const char *path, int strict);
 static inline int is_absolute_path(const char *path)
 {
 	return is_dir_sep(path[0]) || has_dos_drive_prefix(path);
diff --git a/daemon.c b/daemon.c
index 4c8346d..9253192 100644
--- a/daemon.c
+++ b/daemon.c
@@ -108,11 +108,11 @@ static void NORETURN daemon_die(const char *err, va_list params)
 	exit(1);
 }
 
-static char *path_ok(char *directory)
+static const char *path_ok(char *directory)
 {
 	static char rpath[PATH_MAX];
 	static char interp_path[PATH_MAX];
-	char *path;
+	const char *path;
 	char *dir;
 
 	dir = directory;
diff --git a/path.c b/path.c
index 6f3f5d5..f3d96aa 100644
--- a/path.c
+++ b/path.c
@@ -283,7 +283,7 @@ return_null:
  * links.  User relative paths are also returned as they are given,
  * except DWIM suffixing.
  */
-char *enter_repo(char *path, int strict)
+const char *enter_repo(const char *path, int strict)
 {
 	static char used_path[PATH_MAX];
 	static char validated_path[PATH_MAX];
@@ -297,14 +297,17 @@ char *enter_repo(char *path, int strict)
 		};
 		int len = strlen(path);
 		int i;
-		while ((1 < len) && (path[len-1] == '/')) {
-			path[len-1] = 0;
+		while ((1 < len) && (path[len-1] == '/'))
 			len--;
-		}
+
 		if (PATH_MAX <= len)
 			return NULL;
-		if (path[0] == '~') {
-			char *newpath = expand_user_path(path);
+		strncpy(used_path, path, len);
+		used_path[len] = 0;
+		strcpy(validated_path, used_path);
+
+		if (used_path[0] == '~') {
+			char *newpath = expand_user_path(used_path);
 			if (!newpath || (PATH_MAX - 10 < strlen(newpath))) {
 				free(newpath);
 				return NULL;
@@ -316,24 +319,18 @@ char *enter_repo(char *path, int strict)
 			 * anyway.
 			 */
 			strcpy(used_path, newpath); free(newpath);
-			strcpy(validated_path, path);
-			path = used_path;
 		}
 		else if (PATH_MAX - 10 < len)
 			return NULL;
-		else {
-			path = strcpy(used_path, path);
-			strcpy(validated_path, path);
-		}
-		len = strlen(path);
+		len = strlen(used_path);
 		for (i = 0; suffix[i]; i++) {
-			strcpy(path + len, suffix[i]);
-			if (!access(path, F_OK)) {
+			strcpy(used_path + len, suffix[i]);
+			if (!access(used_path, F_OK)) {
 				strcat(validated_path, suffix[i]);
 				break;
 			}
 		}
-		if (!suffix[i] || chdir(path))
+		if (!suffix[i] || chdir(used_path))
 			return NULL;
 		path = validated_path;
 	}
-- 
1.7.7.503.g26392.dirty

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2011-10-04 23:06 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-04 22:51 [PATCH 1/4] enter_repo: do not modify input Phil Hord
2011-10-04 23:01 ` Phil Hord
2011-10-04 23:06   ` Phil Hord
  -- strict thread matches above, loose matches on Subject: below --
2011-10-04 20:02 Phil Hord
2011-10-04 20:24 ` Junio C Hamano

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).