git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] C implementation of the 'git' program, take two.
@ 2005-11-15 23:31 Andreas Ericsson
  2005-11-15 23:45 ` Junio C Hamano
  2005-11-16  0:18 ` Linus Torvalds
  0 siblings, 2 replies; 9+ messages in thread
From: Andreas Ericsson @ 2005-11-15 23:31 UTC (permalink / raw)
  To: git


This patch provides a C implementation of the 'git' program and
introduces support for putting the git-* commands in a directory
of their own. It also saves some time on executing those commands
in a tight loop and it prints the currently available git commands
in a nicely formatted list.

The location of the GIT_EXEC_PATH (name discussion's closed, thank gods)
can be obtained by running

	git --exec-path

which will hopefully give porcelainistas ample time to adapt their
heavy-duty loops to call the core programs directly and thus save
the extra fork() / execve() overhead, although that's not really
necessary any more.

The --exec-path value is prepended to $PATH, so the git-* programs
should Just Work without ever requiring any changes to how they call
other programs in the suite.

Some timing values for 10000 invocations of git-var >&/dev/null:
	git.sh: 24.194s
	git.c:   9.044s
	git-var: 7.377s

The git-<tab><tab> behaviour can, along with the someday-to-be-deprecated
git-<command> form of invocation, be indefinitely retained by adding
the following line to one's .bash_profile or equivalent:

	PATH=$PATH:$(git --exec-path)

Experimental libraries can be used by either setting the environment variable
GIT_EXEC_PATH, or by using

	git --exec-path=/some/experimental/exec-path

Relative paths are properly grok'ed as exec-path values.

Signed-off-by: Andreas Ericsson <ae@op5.se>

---

 Makefile |   20 ++---
 git.c    |  229 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 git.sh   |   76 ---------------------
 3 files changed, 237 insertions(+), 88 deletions(-)
 create mode 100644 git.c
 delete mode 100755 git.sh

applies-to: 4b6dbe856a3e63699b299c76f4f1fc5cb34cbe26
d9a0c94f64a140b5e32d8541875e77ee96ed5ff8
diff --git a/Makefile b/Makefile
index 63cb998..0515968 100644
--- a/Makefile
+++ b/Makefile
@@ -88,7 +88,7 @@ SCRIPT_SH = \
 	git-prune.sh git-pull.sh git-push.sh git-rebase.sh \
 	git-repack.sh git-request-pull.sh git-reset.sh \
 	git-resolve.sh git-revert.sh git-sh-setup.sh git-status.sh \
-	git-tag.sh git-verify-tag.sh git-whatchanged.sh git.sh \
+	git-tag.sh git-verify-tag.sh git-whatchanged.sh \
 	git-applymbox.sh git-applypatch.sh git-am.sh \
 	git-merge.sh git-merge-stupid.sh git-merge-octopus.sh \
 	git-merge-resolve.sh git-merge-ours.sh git-grep.sh \
@@ -334,19 +334,15 @@ SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)
 export prefix TAR INSTALL DESTDIR SHELL_PATH template_dir
 ### Build rules
 
-all: $(PROGRAMS) $(SCRIPTS)
+all: $(PROGRAMS) $(SCRIPTS) git
 
 all:
 	$(MAKE) -C templates
 
-git: git.sh Makefile
-	rm -f $@+ $@
-	sed -e '1s|#!.*/sh|#!$(call shq,$(SHELL_PATH))|' \
-	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
-	    -e 's/@@X@@/$(X)/g' \
-	    $(GIT_LIST_TWEAK) <$@.sh >$@+
-	chmod +x $@+
-	mv $@+ $@
+# Only use $(CFLAGS). We don't need anything else.
+git: git.c Makefile
+	$(CC) -DGIT_EXEC_PATH='"$(bindir)"' -DGIT_VERSION='"$(GIT_VERSION)"' \
+		$(CFLAGS) $@.c -o $@
 
 $(filter-out git,$(patsubst %.sh,%,$(SCRIPT_SH))) : % : %.sh
 	rm -f $@
@@ -431,9 +427,9 @@ check:
 
 ### Installation rules
 
-install: $(PROGRAMS) $(SCRIPTS)
+install: $(PROGRAMS) $(SCRIPTS) git
 	$(INSTALL) -d -m755 $(call shellquote,$(DESTDIR)$(bindir))
-	$(INSTALL) $(PROGRAMS) $(SCRIPTS) $(call shellquote,$(DESTDIR)$(bindir))
+	$(INSTALL) git $(PROGRAMS) $(SCRIPTS) $(call shellquote,$(DESTDIR)$(bindir))
 	$(MAKE) -C templates install
 	$(INSTALL) -d -m755 $(call shellquote,$(DESTDIR)$(GIT_PYTHON_DIR))
 	$(INSTALL) $(PYMODULES) $(call shellquote,$(DESTDIR)$(GIT_PYTHON_DIR))
diff --git a/git.c b/git.c
new file mode 100644
index 0000000..d189801
--- /dev/null
+++ b/git.c
@@ -0,0 +1,229 @@
+#include <stdio.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <limits.h>
+#include <stdarg.h>
+#include <glob.h>
+
+#ifndef PATH_MAX
+# define PATH_MAX 4096
+#endif
+
+static const char git_usage[] =
+	"Usage: git [--version] [--exec-path[=GIT_EXEC_PATH]] [--help] COMMAND [ ARGS ]";
+
+struct string_list {
+	size_t len;
+	char *str;
+	struct string_list *next;
+};
+
+/* most gui terms set COLUMNS (although some don't export it) */
+static int term_columns(void)
+{
+	char *col_string = getenv("COLUMNS");
+	int n_cols = 0;
+
+	if (col_string && (n_cols = atoi(col_string)) > 0)
+		return n_cols;
+
+	return 80;
+}
+
+static inline void mput_char(char c, unsigned int num)
+{
+	while(num--)
+		putchar(c);
+}
+
+static void pretty_print_string_list(struct string_list *list, int longest)
+{
+	int cols = 1;
+	int space = longest + 1; /* min 1 SP between words */
+	int max_cols = term_columns() - 1; /* don't print *on* the edge */
+
+	if (space < max_cols)
+		cols = max_cols / space;
+
+	while (list) {
+		int c;
+		printf("  ");
+
+		for (c = cols; c && list; list = list->next) {
+			printf("%s", list->str);
+
+			if (--c)
+				mput_char(' ', space - list->len);
+		}
+		putchar('\n');
+	}
+}
+
+static void list_commands(const char *exec_path, const char *pattern)
+{
+	struct string_list *list = NULL, *tail = NULL;
+	unsigned int longest = 0, i;
+	glob_t gl;
+
+	if (chdir(exec_path) < 0) {
+		printf("git: '%s': %s\n", exec_path, strerror(errno));
+		exit(1);
+	}
+
+	i = glob(pattern, 0, NULL, &gl);
+	switch(i) {
+	case GLOB_NOSPACE:
+		puts("Out of memory when running glob()");
+		exit(2);
+	case GLOB_ABORTED:
+		printf("'%s': Read error: %s\n", exec_path, strerror(errno));
+		exit(2);
+	case GLOB_NOMATCH:
+		printf("No git commands available in '%s'.\n", exec_path);
+		printf("Do you need to specify --exec-path or set GIT_EXEC_PATH?\n");
+		exit(1);
+	}
+
+	for (i = 0; i < gl.gl_pathc; i++) {
+		int len = strlen(gl.gl_pathv[i] + 4);
+
+		if (access(gl.gl_pathv[i], X_OK))
+			continue;
+
+		if (longest < len)
+			longest = len;
+
+		if (!tail)
+			tail = list = malloc(sizeof(struct string_list));
+		else {
+			tail->next = malloc(sizeof(struct string_list));
+			tail = tail->next;
+		}
+		tail->len = len;
+		tail->str = gl.gl_pathv[i] + 4;
+		tail->next = NULL;
+	}
+
+	printf("git commands available in '%s'\n", exec_path);
+	printf("----------------------------");
+	mput_char('-', strlen(exec_path));
+	putchar('\n');
+	pretty_print_string_list(list, longest);
+	putchar('\n');
+}
+
+#ifdef __GNUC__
+static void usage(const char *exec_path, const char *fmt, ...)
+	__attribute__((__format__(__printf__, 2, 3), __noreturn__));
+#endif
+static void usage(const char *exec_path, const char *fmt, ...)
+{
+	if (fmt) {
+		va_list ap;
+
+		va_start(ap, fmt);
+		printf("git: ");
+		vprintf(fmt, ap);
+		va_end(ap);
+		putchar('\n');
+	}
+	else
+		puts(git_usage);
+
+	putchar('\n');
+
+	if(exec_path)
+		list_commands(exec_path, "git-*");
+
+	exit(1);
+}
+
+static void prepend_to_path(const char *dir, int len)
+{
+	char *path, *old_path = getenv("PATH");
+	int path_len = len;
+
+	if (!old_path)
+		old_path = "/bin:/usr/bin:.";
+
+	path_len = len + strlen(old_path) + 1;
+
+	path = malloc(path_len + 1);
+	path[path_len + 1] = '\0';
+
+	memcpy(path, dir, len);
+	path[len] = ':';
+	memcpy(path + len + 1, old_path, path_len - len);
+
+	setenv("PATH", path, 1);
+}
+
+int main(int argc, char **argv, char **envp)
+{
+	char git_command[PATH_MAX + 1];
+	char wd[PATH_MAX + 1];
+	int i, len, show_help = 0;
+	char *exec_path = getenv("GIT_EXEC_PATH");
+
+	getcwd(wd, PATH_MAX);
+
+	if (!exec_path)
+		exec_path = GIT_EXEC_PATH;
+
+	for (i = 1; i < argc; i++) {
+		char *arg = argv[i];
+
+		if (strncmp(arg, "--", 2))
+			break;
+
+		arg += 2;
+
+		if (!strncmp(arg, "exec-path", 9)) {
+			arg += 9;
+			if (*arg == '=')
+				exec_path = arg + 1;
+			else {
+				puts(exec_path);
+				exit(0);
+			}
+		}
+		else if (!strcmp(arg, "version")) {
+			printf("git version %s\n", GIT_VERSION);
+			exit(0);
+		}
+		else if (!strcmp(arg, "help"))
+			show_help = 1;
+		else if (!show_help)
+			usage(NULL, NULL);
+	}
+
+	if (i >= argc || show_help)
+		usage(exec_path, NULL);
+
+	/* allow relative paths, but run with exact */
+	if (chdir(exec_path)) {
+		printf("git: '%s': %s\n", exec_path, strerror(errno));
+		exit (1);
+	}
+
+	getcwd(git_command, sizeof(git_command));
+	chdir(wd);
+
+	len = strlen(git_command);
+	prepend_to_path(git_command, len);
+
+	strncat(&git_command[len], "/git-", sizeof(git_command) - len);
+	len += 5;
+	strncat(&git_command[len], argv[i], sizeof(git_command) - len);
+
+	if (access(git_command, X_OK))
+		usage(exec_path, "'%s' is not a git-command", argv[i]);
+
+	/* execve() can only ever return if it fails */
+	execve(git_command, &argv[i], envp);
+	printf("Failed to run command '%s': %s\n", git_command, strerror(errno));
+
+	return 1;
+}
diff --git a/git.sh b/git.sh
deleted file mode 100755
index 94940ae..0000000
--- a/git.sh
+++ /dev/null
@@ -1,76 +0,0 @@
-#!/bin/sh
-
-cmd=
-path=$(dirname "$0")
-case "$#" in
-0)	;;
-*)	cmd="$1"
-	shift
-	case "$cmd" in
-	-v|--v|--ve|--ver|--vers|--versi|--versio|--version)
-		echo "git version @@GIT_VERSION@@"
-		exit 0 ;;
-	esac
-	
-	test -x "$path/git-$cmd" && exec "$path/git-$cmd" "$@"
-	
-	case '@@X@@' in
-	    '')
-		;;
-	    *)
-		test -x "$path/git-$cmd@@X@@" &&
-		exec "$path/git-$cmd@@X@@" "$@"
-		;;
-	esac
-	;;
-esac
-
-echo "Usage: git COMMAND [OPTIONS] [TARGET]"
-if [ -n "$cmd" ]; then
-    echo "git command '$cmd' not found."
-fi
-echo "git commands are:"
-
-fmt <<\EOF | sed -e 's/^/    /'
-add
-apply
-archimport
-bisect
-branch
-checkout
-cherry
-clone
-commit
-count-objects
-cvsimport
-diff
-fetch
-format-patch
-fsck-objects
-get-tar-commit-id
-init-db
-log
-ls-remote
-octopus
-pack-objects
-parse-remote
-patch-id
-prune
-pull
-push
-rebase
-relink
-rename
-repack
-request-pull
-reset
-resolve
-revert
-send-email
-shortlog
-show-branch
-status
-tag
-verify-tag
-whatchanged
-EOF
---
0.99.9.GIT

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

* Re: [PATCH 1/3] C implementation of the 'git' program, take two.
  2005-11-15 23:31 [PATCH 1/3] C implementation of the 'git' program, take two Andreas Ericsson
@ 2005-11-15 23:45 ` Junio C Hamano
  2005-11-16  0:10   ` Andreas Ericsson
  2005-11-16  0:18 ` Linus Torvalds
  1 sibling, 1 reply; 9+ messages in thread
From: Junio C Hamano @ 2005-11-15 23:45 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: git

exon@op5.se (Andreas Ericsson) writes:

> This patch provides a C implementation of the 'git' program and
> introduces support for putting the git-* commands in a directory
> of their own.

Very nice, thanks.  Two questions and a half.

> +static void prepend_to_path(const char *dir, int len)
> +{
> +	char *path, *old_path = getenv("PATH");
> +	int path_len = len;
> +
> +	if (!old_path)
> +		old_path = "/bin:/usr/bin:.";

This is to cover strange case and probably would not matter in
practice, but perhaps without current directory?

> +int main(int argc, char **argv, char **envp)
> +{
> +	char git_command[PATH_MAX + 1];
> +	char wd[PATH_MAX + 1];
> +	int i, len, show_help = 0;
> +	char *exec_path = getenv("GIT_EXEC_PATH");
> +
> +	getcwd(wd, PATH_MAX);
> +...
> +	/* allow relative paths, but run with exact */
> +	if (chdir(exec_path)) {
> +		printf("git: '%s': %s\n", exec_path, strerror(errno));
> +		exit (1);
> +	}
> +
> +	getcwd(git_command, sizeof(git_command));
> +	chdir(wd);

Can we always come back from where we started?

> +
> +	len = strlen(git_command);
> +	prepend_to_path(git_command, len);
> +
> +	strncat(&git_command[len], "/git-", sizeof(git_command) - len);
> +	len += 5;
> +	strncat(&git_command[len], argv[i], sizeof(git_command) - len);
> +
> +	if (access(git_command, X_OK))
> +		usage(exec_path, "'%s' is not a git-command", argv[i]);
> +
> +	/* execve() can only ever return if it fails */
> +	execve(git_command, &argv[i], envp);

Shell version for Cygwin seems to do ".exe" at the end --- does
it matter?

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

* Re: [PATCH 1/3] C implementation of the 'git' program, take two.
  2005-11-15 23:45 ` Junio C Hamano
@ 2005-11-16  0:10   ` Andreas Ericsson
  2005-11-16  0:17     ` Junio C Hamano
  0 siblings, 1 reply; 9+ messages in thread
From: Andreas Ericsson @ 2005-11-16  0:10 UTC (permalink / raw)
  To: git

Junio C Hamano wrote:
> exon@op5.se (Andreas Ericsson) writes:
> 
> 
>>This patch provides a C implementation of the 'git' program and
>>introduces support for putting the git-* commands in a directory
>>of their own.
> 
> 
> Very nice, thanks.  Two questions and a half.
> 
> 
>>+static void prepend_to_path(const char *dir, int len)
>>+{
>>+	char *path, *old_path = getenv("PATH");
>>+	int path_len = len;
>>+
>>+	if (!old_path)
>>+		old_path = "/bin:/usr/bin:.";
> 
> 
> This is to cover strange case and probably would not matter in
> practice, but perhaps without current directory?
> 

I have no preference really and since it already covers a strange case 
it probably shouldn't matter either way.

> 
>>+int main(int argc, char **argv, char **envp)
>>+{
>>+	char git_command[PATH_MAX + 1];
>>+	char wd[PATH_MAX + 1];
>>+	int i, len, show_help = 0;
>>+	char *exec_path = getenv("GIT_EXEC_PATH");
>>+
>>+	getcwd(wd, PATH_MAX);
>>+...
>>+	/* allow relative paths, but run with exact */
>>+	if (chdir(exec_path)) {
>>+		printf("git: '%s': %s\n", exec_path, strerror(errno));
>>+		exit (1);
>>+	}
>>+
>>+	getcwd(git_command, sizeof(git_command));
>>+	chdir(wd);
> 
> 
> Can we always come back from where we started?
> 

Not sure what you mean. Perhaps "Come back *to* where we started"?

If getcwd(wd, sizeof(wd)) fails then chdir(wd) will also fail (or do 
something strange, at least). wd is otherwise absolute.

> 
>>+
>>+	len = strlen(git_command);
>>+	prepend_to_path(git_command, len);
>>+
>>+	strncat(&git_command[len], "/git-", sizeof(git_command) - len);
>>+	len += 5;
>>+	strncat(&git_command[len], argv[i], sizeof(git_command) - len);
>>+
>>+	if (access(git_command, X_OK))
>>+		usage(exec_path, "'%s' is not a git-command", argv[i]);
>>+
>>+	/* execve() can only ever return if it fails */
>>+	execve(git_command, &argv[i], envp);
> 
> 
> Shell version for Cygwin seems to do ".exe" at the end --- does
> it matter?
> 

Dunno, really. I suppose it does as it bypasses the shell with the 
execve() call, unless windows or the cygwin stuff does some trickery to 
find an .exe regardless.

Is it ok if I send a separate patch for it, or would you rather have me 
redo this one?

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

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

* Re: [PATCH 1/3] C implementation of the 'git' program, take two.
  2005-11-16  0:10   ` Andreas Ericsson
@ 2005-11-16  0:17     ` Junio C Hamano
  0 siblings, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2005-11-16  0:17 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: git

Andreas Ericsson <ae@op5.se> writes:

> Dunno, really. I suppose it does as it bypasses the shell with the 
> execve() call, unless windows or the cygwin stuff does some trickery to 
> find an .exe regardless.
>
> Is it ok if I send a separate patch for it, or would you rather have me 
> redo this one?

I'll take this as is and have Cygwin folks holler if it breaks
things for them ;-).  Thanks.

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

* Re: [PATCH 1/3] C implementation of the 'git' program, take two.
  2005-11-15 23:31 [PATCH 1/3] C implementation of the 'git' program, take two Andreas Ericsson
  2005-11-15 23:45 ` Junio C Hamano
@ 2005-11-16  0:18 ` Linus Torvalds
  2005-11-16  0:42   ` Andreas Ericsson
  1 sibling, 1 reply; 9+ messages in thread
From: Linus Torvalds @ 2005-11-16  0:18 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: git



On Wed, 16 Nov 2005, Andreas Ericsson wrote:
> +
> +	/* allow relative paths, but run with exact */
> +	if (chdir(exec_path)) {
> +		printf("git: '%s': %s\n", exec_path, strerror(errno));
> +		exit (1);
> +	}
> +
> +	getcwd(git_command, sizeof(git_command));
> +	chdir(wd);

Argh. This is pretty horrible way to turn a path into an absolute one. 
Especially since you didn't even test whether the original "wd" was 
successful.

Why don't you just do

	if (exec_path[0] != '/') {
		.. prepend "cwd/" to exec_path ..

since as far as I can tell you don't actually care whether it's a 
simplified path or not (you can remove "./" at the beginning just to make 
it cleaner, if you wish. In fact, you can remove "../" at the beginning 
too (but only the beginning) since getcwd() shouldn't have any symlink 
components).

The reason to avoid "chdir(relative) + chdir(back)" is that it totally 
unnecessarily breaks under some extreme cases. For example, if the 
exec_path is already absolute, and we just happen to be in a really deep 
subdirectory, then the getcwd() could have failed due to the PATH_MAX 
limitations.

Also, depending on getcwd() will not work if any parent directory is 
unreadable or non-executable (well, under Linux it will, as long as it's 
executable, since getcwd() is actually a system call. Not in UNIX in 
general, though). Again, that means that unless you _have_ to know what 
the cwd is, you should try to avoid relying on it.

Now, there are Linux-specific tricks that can avoid some of the problems 
if you want to, but they are very much hacks:

	if (filename[0] != '/') {
		fd = open(filename, O_DIRECTORY);
		if (fd >= 0) {
			snprintf(link_name, sizeof(link_name), "/proc/self/fd/%d", fd);
			if (!readlink(link_name ...)) {
				.. there it is ..
			}
			close(fd);
		}
	..

and the nicer thign to do is to just not try to be clever.

		Linus

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

* Re: [PATCH 1/3] C implementation of the 'git' program, take two.
  2005-11-16  0:18 ` Linus Torvalds
@ 2005-11-16  0:42   ` Andreas Ericsson
  2005-11-16  2:02     ` Linus Torvalds
  0 siblings, 1 reply; 9+ messages in thread
From: Andreas Ericsson @ 2005-11-16  0:42 UTC (permalink / raw)
  To: git

Linus Torvalds wrote:
> 
> On Wed, 16 Nov 2005, Andreas Ericsson wrote:
> 
>>+
>>+	/* allow relative paths, but run with exact */
>>+	if (chdir(exec_path)) {
>>+		printf("git: '%s': %s\n", exec_path, strerror(errno));
>>+		exit (1);
>>+	}
>>+
>>+	getcwd(git_command, sizeof(git_command));
>>+	chdir(wd);
> 
> 
> Argh. This is pretty horrible way to turn a path into an absolute one. 


It was your idea to begin with, actually, in the thread on how to do 
proper path validation in the git-daemon. :)

> 
> Why don't you just do
> 
> 	if (exec_path[0] != '/') {
> 		.. prepend "cwd/" to exec_path ..
> 

You mean
	setenv("PATH", concat3(cwd, exec_path, old_path), 1);

?

Because that can fail too. Every solution is bad if you twist and turn 
it enough.

> 
> The reason to avoid "chdir(relative) + chdir(back)" is that it totally 
> unnecessarily breaks under some extreme cases. For example, if the 
> exec_path is already absolute, and we just happen to be in a really deep 
> subdirectory, then the getcwd() could have failed due to the PATH_MAX 
> limitations.
> 

True. So, would something like
char *try_goddamn_hard_to_get_working_dir(char *rel_path)
{
	size_t plen = PATH_MAX;
	char *p = malloc(PATH_MAX);
	while(p && plen < 1 << 20 && !(p = getcwd(p, plen))) {
		plen += PATH_MAX;
		p = realloc(p, plen);
	}

	return p;
}

be considered safe from this particular point of view?


> Also, depending on getcwd() will not work if any parent directory is 
> unreadable or non-executable (well, under Linux it will, as long as it's 
> executable, since getcwd() is actually a system call. Not in UNIX in 
> general, though). Again, that means that unless you _have_ to know what 
> the cwd is, you should try to avoid relying on it.
> 

True. I'll redo that part (tomorrow).

> 
> and the nicer thign to do is to just not try to be clever.
> 

I never try. It just comes to me naturally. ;)

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

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

* Re: [PATCH 1/3] C implementation of the 'git' program, take two.
  2005-11-16  0:42   ` Andreas Ericsson
@ 2005-11-16  2:02     ` Linus Torvalds
  2005-11-16  2:18       ` Johannes Schindelin
  0 siblings, 1 reply; 9+ messages in thread
From: Linus Torvalds @ 2005-11-16  2:02 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: git



On Wed, 16 Nov 2005, Andreas Ericsson wrote:
> 
> It was your idea to begin with, actually, in the thread on how to do proper
> path validation in the git-daemon. :)

But that was because the important part wasn't to get an absolute path, 
but because the important part was to get the _canonical_ path. 

That was for security. 

Besides, I don't think we ever switched back. We just did a chdir() + a 
getcwd().

> > Why don't you just do
> > 
> > 	if (exec_path[0] != '/') {
> > 		.. prepend "cwd/" to exec_path ..
> > 
> 
> You mean
> 	setenv("PATH", concat3(cwd, exec_path, old_path), 1);

No.

I mean exactly what I said.

I mean testing whether the exec_path[] is already absolute, and not 
touching it at all if it is.

It's really as simple as something like

	const char *absolute_path(const char *input)
	{
		int a, b;
		char *buf;
		char cwd[PATH_MAX];

		if (*input == '/')
			return input;

		if (!getcwd(cwd, sizeof(cwd))
			return NULL;

		/* Do some trivial cleanup */
		while (!strncmp(input, "./", 2)) {
			input += 2;
			while (*input == '/')
				input++;
		}

		a = strlen(cwd);
		b = strlen(input);
		buf = malloc(a + b + 2);
		if (!buf)
			return NULL;

		memcpy(buf, cwd, a);
		buf[a] = '/';
		memcpy(buf + a + 1, input, b);
		buf[a + 1 + b] = 0;
		return buf;
	}

and there it is.

The magic rule being:
 - if the path is already absolute, it's _good_. Don't play games with it.
 - just append the dang thing with cwd. Don't play games (the above does 
   trivial simplification, which is unnecessary, but it's so simple that 
   hey, who cares? And it makes one common case a bit prettier)

Then, you just prepend it to the PATH, with a : in between (and if the 
pathname has a ":" in it, tough, there's nothing we can do about it).

		Linus

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

* Re: [PATCH 1/3] C implementation of the 'git' program, take two.
  2005-11-16  2:02     ` Linus Torvalds
@ 2005-11-16  2:18       ` Johannes Schindelin
  2005-11-16 21:04         ` Alex Riesen
  0 siblings, 1 reply; 9+ messages in thread
From: Johannes Schindelin @ 2005-11-16  2:18 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Andreas Ericsson, git

Hi,

On Tue, 15 Nov 2005, Linus Torvalds wrote:

> Then, you just prepend it to the PATH, with a : in between (and if the 
> pathname has a ":" in it, tough, there's nothing we can do about it).

You mean like "c:/cygwin/git"? Yes, I know, the default is 
"/cygdrive/c/cygwin/git", but there are people out there with ":" in their 
pathname.

Ciao,
Dscho

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

* Re: [PATCH 1/3] C implementation of the 'git' program, take two.
  2005-11-16  2:18       ` Johannes Schindelin
@ 2005-11-16 21:04         ` Alex Riesen
  0 siblings, 0 replies; 9+ messages in thread
From: Alex Riesen @ 2005-11-16 21:04 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Linus Torvalds, Andreas Ericsson, git

Johannes Schindelin, Wed, Nov 16, 2005 03:18:05 +0100:
> > Then, you just prepend it to the PATH, with a : in between (and if the 
> > pathname has a ":" in it, tough, there's nothing we can do about it).
> 
> You mean like "c:/cygwin/git"? Yes, I know, the default is 
> "/cygdrive/c/cygwin/git", but there are people out there with ":" in their 
> pathname.
> 

and you can't possibly be hinting at "\" as path element separator...

Cygwin rewrites values in PATH (well, poorly: there are things like
"z:.", which admins of another stupid thing, novel netware, are so much
fond of).

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

end of thread, other threads:[~2005-11-16 21:05 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-11-15 23:31 [PATCH 1/3] C implementation of the 'git' program, take two Andreas Ericsson
2005-11-15 23:45 ` Junio C Hamano
2005-11-16  0:10   ` Andreas Ericsson
2005-11-16  0:17     ` Junio C Hamano
2005-11-16  0:18 ` Linus Torvalds
2005-11-16  0:42   ` Andreas Ericsson
2005-11-16  2:02     ` Linus Torvalds
2005-11-16  2:18       ` Johannes Schindelin
2005-11-16 21:04         ` Alex Riesen

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