{"thread":{"id":"2540","subject":"[PATCH 1/3] C implementation of the 'git' program, take two.","startedAt":"2005-11-15T23:31:25Z","lastAt":"2005-11-16T21:04:49Z","messageCount":9,"participants":["Andreas Ericsson","Junio C Hamano","Linus Torvalds","Johannes Schindelin","Alex Riesen"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"11954","messageId":"20051115233125.3153B5BF76@nox.op5.se","threadId":"2540","inReplyTo":null,"subject":"[PATCH 1/3] C implementation of the 'git' program, take two.","fromName":"Andreas Ericsson","fromEmail":"exon@op5.se","sentAt":"2005-11-15T23:31:25Z","receivedAt":"2005-11-15T23:31:25Z","isPatch":true,"sender":{"key":"exon@op5.se","avatar":"https://gravatar.com/avatar/b948c4f759e868f8e721e545e37afe2cf89cfa8e2ee8b70a432f0d76aee39891?d=mp&s=160"},"body":"\nThis patch provides a C implementation of the 'git' program and\nintroduces support for putting the git-* commands in a directory\nof their own. It also saves some time on executing those commands\nin a tight loop and it prints the currently available git commands\nin a nicely formatted list.\n\nThe location of the GIT_EXEC_PATH (name discussion's closed, thank gods)\ncan be obtained by running\n\n\tgit --exec-path\n\nwhich will hopefully give porcelainistas ample time to adapt their\nheavy-duty loops to call the core programs directly and thus save\nthe extra fork() / execve() overhead, although that's not really\nnecessary any more.\n\nThe --exec-path value is prepended to $PATH, so the git-* programs\nshould Just Work without ever requiring any changes to how they call\nother programs in the suite.\n\nSome timing values for 10000 invocations of git-var >&/dev/null:\n\tgit.sh: 24.194s\n\tgit.c:   9.044s\n\tgit-var: 7.377s\n\nThe git-<tab><tab> behaviour can, along with the someday-to-be-deprecated\ngit-<command> form of invocation, be indefinitely retained by adding\nthe following line to one's .bash_profile or equivalent:\n\n\tPATH=$PATH:$(git --exec-path)\n\nExperimental libraries can be used by either setting the environment variable\nGIT_EXEC_PATH, or by using\n\n\tgit --exec-path=/some/experimental/exec-path\n\nRelative paths are properly grok'ed as exec-path values.\n\nSigned-off-by: Andreas Ericsson <ae@op5.se>\n\n---\n\n Makefile |   20 ++---\n git.c    |  229 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n git.sh   |   76 ---------------------\n 3 files changed, 237 insertions(+), 88 deletions(-)\n create mode 100644 git.c\n delete mode 100755 git.sh\n\napplies-to: 4b6dbe856a3e63699b299c76f4f1fc5cb34cbe26\nd9a0c94f64a140b5e32d8541875e77ee96ed5ff8\ndiff --git a/Makefile b/Makefile\nindex 63cb998..0515968 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -88,7 +88,7 @@ SCRIPT_SH = \\\n \tgit-prune.sh git-pull.sh git-push.sh git-rebase.sh \\\n \tgit-repack.sh git-request-pull.sh git-reset.sh \\\n \tgit-resolve.sh git-revert.sh git-sh-setup.sh git-status.sh \\\n-\tgit-tag.sh git-verify-tag.sh git-whatchanged.sh git.sh \\\n+\tgit-tag.sh git-verify-tag.sh git-whatchanged.sh \\\n \tgit-applymbox.sh git-applypatch.sh git-am.sh \\\n \tgit-merge.sh git-merge-stupid.sh git-merge-octopus.sh \\\n \tgit-merge-resolve.sh git-merge-ours.sh git-grep.sh \\\n@@ -334,19 +334,15 @@ SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)\n export prefix TAR INSTALL DESTDIR SHELL_PATH template_dir\n ### Build rules\n \n-all: $(PROGRAMS) $(SCRIPTS)\n+all: $(PROGRAMS) $(SCRIPTS) git\n \n all:\n \t$(MAKE) -C templates\n \n-git: git.sh Makefile\n-\trm -f $@+ $@\n-\tsed -e '1s|#!.*/sh|#!$(call shq,$(SHELL_PATH))|' \\\n-\t    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \\\n-\t    -e 's/@@X@@/$(X)/g' \\\n-\t    $(GIT_LIST_TWEAK) <$@.sh >$@+\n-\tchmod +x $@+\n-\tmv $@+ $@\n+# Only use $(CFLAGS). We don't need anything else.\n+git: git.c Makefile\n+\t$(CC) -DGIT_EXEC_PATH='\"$(bindir)\"' -DGIT_VERSION='\"$(GIT_VERSION)\"' \\\n+\t\t$(CFLAGS) $@.c -o $@\n \n $(filter-out git,$(patsubst %.sh,%,$(SCRIPT_SH))) : % : %.sh\n \trm -f $@\n@@ -431,9 +427,9 @@ check:\n \n ### Installation rules\n \n-install: $(PROGRAMS) $(SCRIPTS)\n+install: $(PROGRAMS) $(SCRIPTS) git\n \t$(INSTALL) -d -m755 $(call shellquote,$(DESTDIR)$(bindir))\n-\t$(INSTALL) $(PROGRAMS) $(SCRIPTS) $(call shellquote,$(DESTDIR)$(bindir))\n+\t$(INSTALL) git $(PROGRAMS) $(SCRIPTS) $(call shellquote,$(DESTDIR)$(bindir))\n \t$(MAKE) -C templates install\n \t$(INSTALL) -d -m755 $(call shellquote,$(DESTDIR)$(GIT_PYTHON_DIR))\n \t$(INSTALL) $(PYMODULES) $(call shellquote,$(DESTDIR)$(GIT_PYTHON_DIR))\ndiff --git a/git.c b/git.c\nnew file mode 100644\nindex 0000000..d189801\n--- /dev/null\n+++ b/git.c\n@@ -0,0 +1,229 @@\n+#include <stdio.h>\n+#include <unistd.h>\n+#include <stdlib.h>\n+#include <string.h>\n+#include <errno.h>\n+#include <limits.h>\n+#include <stdarg.h>\n+#include <glob.h>\n+\n+#ifndef PATH_MAX\n+# define PATH_MAX 4096\n+#endif\n+\n+static const char git_usage[] =\n+\t\"Usage: git [--version] [--exec-path[=GIT_EXEC_PATH]] [--help] COMMAND [ ARGS ]\";\n+\n+struct string_list {\n+\tsize_t len;\n+\tchar *str;\n+\tstruct string_list *next;\n+};\n+\n+/* most gui terms set COLUMNS (although some don't export it) */\n+static int term_columns(void)\n+{\n+\tchar *col_string = getenv(\"COLUMNS\");\n+\tint n_cols = 0;\n+\n+\tif (col_string && (n_cols = atoi(col_string)) > 0)\n+\t\treturn n_cols;\n+\n+\treturn 80;\n+}\n+\n+static inline void mput_char(char c, unsigned int num)\n+{\n+\twhile(num--)\n+\t\tputchar(c);\n+}\n+\n+static void pretty_print_string_list(struct string_list *list, int longest)\n+{\n+\tint cols = 1;\n+\tint space = longest + 1; /* min 1 SP between words */\n+\tint max_cols = term_columns() - 1; /* don't print *on* the edge */\n+\n+\tif (space < max_cols)\n+\t\tcols = max_cols / space;\n+\n+\twhile (list) {\n+\t\tint c;\n+\t\tprintf(\"  \");\n+\n+\t\tfor (c = cols; c && list; list = list->next) {\n+\t\t\tprintf(\"%s\", list->str);\n+\n+\t\t\tif (--c)\n+\t\t\t\tmput_char(' ', space - list->len);\n+\t\t}\n+\t\tputchar('\\n');\n+\t}\n+}\n+\n+static void list_commands(const char *exec_path, const char *pattern)\n+{\n+\tstruct string_list *list = NULL, *tail = NULL;\n+\tunsigned int longest = 0, i;\n+\tglob_t gl;\n+\n+\tif (chdir(exec_path) < 0) {\n+\t\tprintf(\"git: '%s': %s\\n\", exec_path, strerror(errno));\n+\t\texit(1);\n+\t}\n+\n+\ti = glob(pattern, 0, NULL, &gl);\n+\tswitch(i) {\n+\tcase GLOB_NOSPACE:\n+\t\tputs(\"Out of memory when running glob()\");\n+\t\texit(2);\n+\tcase GLOB_ABORTED:\n+\t\tprintf(\"'%s': Read error: %s\\n\", exec_path, strerror(errno));\n+\t\texit(2);\n+\tcase GLOB_NOMATCH:\n+\t\tprintf(\"No git commands available in '%s'.\\n\", exec_path);\n+\t\tprintf(\"Do you need to specify --exec-path or set GIT_EXEC_PATH?\\n\");\n+\t\texit(1);\n+\t}\n+\n+\tfor (i = 0; i < gl.gl_pathc; i++) {\n+\t\tint len = strlen(gl.gl_pathv[i] + 4);\n+\n+\t\tif (access(gl.gl_pathv[i], X_OK))\n+\t\t\tcontinue;\n+\n+\t\tif (longest < len)\n+\t\t\tlongest = len;\n+\n+\t\tif (!tail)\n+\t\t\ttail = list = malloc(sizeof(struct string_list));\n+\t\telse {\n+\t\t\ttail->next = malloc(sizeof(struct string_list));\n+\t\t\ttail = tail->next;\n+\t\t}\n+\t\ttail->len = len;\n+\t\ttail->str = gl.gl_pathv[i] + 4;\n+\t\ttail->next = NULL;\n+\t}\n+\n+\tprintf(\"git commands available in '%s'\\n\", exec_path);\n+\tprintf(\"----------------------------\");\n+\tmput_char('-', strlen(exec_path));\n+\tputchar('\\n');\n+\tpretty_print_string_list(list, longest);\n+\tputchar('\\n');\n+}\n+\n+#ifdef __GNUC__\n+static void usage(const char *exec_path, const char *fmt, ...)\n+\t__attribute__((__format__(__printf__, 2, 3), __noreturn__));\n+#endif\n+static void usage(const char *exec_path, const char *fmt, ...)\n+{\n+\tif (fmt) {\n+\t\tva_list ap;\n+\n+\t\tva_start(ap, fmt);\n+\t\tprintf(\"git: \");\n+\t\tvprintf(fmt, ap);\n+\t\tva_end(ap);\n+\t\tputchar('\\n');\n+\t}\n+\telse\n+\t\tputs(git_usage);\n+\n+\tputchar('\\n');\n+\n+\tif(exec_path)\n+\t\tlist_commands(exec_path, \"git-*\");\n+\n+\texit(1);\n+}\n+\n+static void prepend_to_path(const char *dir, int len)\n+{\n+\tchar *path, *old_path = getenv(\"PATH\");\n+\tint path_len = len;\n+\n+\tif (!old_path)\n+\t\told_path = \"/bin:/usr/bin:.\";\n+\n+\tpath_len = len + strlen(old_path) + 1;\n+\n+\tpath = malloc(path_len + 1);\n+\tpath[path_len + 1] = '\\0';\n+\n+\tmemcpy(path, dir, len);\n+\tpath[len] = ':';\n+\tmemcpy(path + len + 1, old_path, path_len - len);\n+\n+\tsetenv(\"PATH\", path, 1);\n+}\n+\n+int main(int argc, char **argv, char **envp)\n+{\n+\tchar git_command[PATH_MAX + 1];\n+\tchar wd[PATH_MAX + 1];\n+\tint i, len, show_help = 0;\n+\tchar *exec_path = getenv(\"GIT_EXEC_PATH\");\n+\n+\tgetcwd(wd, PATH_MAX);\n+\n+\tif (!exec_path)\n+\t\texec_path = GIT_EXEC_PATH;\n+\n+\tfor (i = 1; i < argc; i++) {\n+\t\tchar *arg = argv[i];\n+\n+\t\tif (strncmp(arg, \"--\", 2))\n+\t\t\tbreak;\n+\n+\t\targ += 2;\n+\n+\t\tif (!strncmp(arg, \"exec-path\", 9)) {\n+\t\t\targ += 9;\n+\t\t\tif (*arg == '=')\n+\t\t\t\texec_path = arg + 1;\n+\t\t\telse {\n+\t\t\t\tputs(exec_path);\n+\t\t\t\texit(0);\n+\t\t\t}\n+\t\t}\n+\t\telse if (!strcmp(arg, \"version\")) {\n+\t\t\tprintf(\"git version %s\\n\", GIT_VERSION);\n+\t\t\texit(0);\n+\t\t}\n+\t\telse if (!strcmp(arg, \"help\"))\n+\t\t\tshow_help = 1;\n+\t\telse if (!show_help)\n+\t\t\tusage(NULL, NULL);\n+\t}\n+\n+\tif (i >= argc || show_help)\n+\t\tusage(exec_path, NULL);\n+\n+\t/* allow relative paths, but run with exact */\n+\tif (chdir(exec_path)) {\n+\t\tprintf(\"git: '%s': %s\\n\", exec_path, strerror(errno));\n+\t\texit (1);\n+\t}\n+\n+\tgetcwd(git_command, sizeof(git_command));\n+\tchdir(wd);\n+\n+\tlen = strlen(git_command);\n+\tprepend_to_path(git_command, len);\n+\n+\tstrncat(&git_command[len], \"/git-\", sizeof(git_command) - len);\n+\tlen += 5;\n+\tstrncat(&git_command[len], argv[i], sizeof(git_command) - len);\n+\n+\tif (access(git_command, X_OK))\n+\t\tusage(exec_path, \"'%s' is not a git-command\", argv[i]);\n+\n+\t/* execve() can only ever return if it fails */\n+\texecve(git_command, &argv[i], envp);\n+\tprintf(\"Failed to run command '%s': %s\\n\", git_command, strerror(errno));\n+\n+\treturn 1;\n+}\ndiff --git a/git.sh b/git.sh\ndeleted file mode 100755\nindex 94940ae..0000000\n--- a/git.sh\n+++ /dev/null\n@@ -1,76 +0,0 @@\n-#!/bin/sh\n-\n-cmd=\n-path=$(dirname \"$0\")\n-case \"$#\" in\n-0)\t;;\n-*)\tcmd=\"$1\"\n-\tshift\n-\tcase \"$cmd\" in\n-\t-v|--v|--ve|--ver|--vers|--versi|--versio|--version)\n-\t\techo \"git version @@GIT_VERSION@@\"\n-\t\texit 0 ;;\n-\tesac\n-\t\n-\ttest -x \"$path/git-$cmd\" && exec \"$path/git-$cmd\" \"$@\"\n-\t\n-\tcase '@@X@@' in\n-\t    '')\n-\t\t;;\n-\t    *)\n-\t\ttest -x \"$path/git-$cmd@@X@@\" &&\n-\t\texec \"$path/git-$cmd@@X@@\" \"$@\"\n-\t\t;;\n-\tesac\n-\t;;\n-esac\n-\n-echo \"Usage: git COMMAND [OPTIONS] [TARGET]\"\n-if [ -n \"$cmd\" ]; then\n-    echo \"git command '$cmd' not found.\"\n-fi\n-echo \"git commands are:\"\n-\n-fmt <<\\EOF | sed -e 's/^/    /'\n-add\n-apply\n-archimport\n-bisect\n-branch\n-checkout\n-cherry\n-clone\n-commit\n-count-objects\n-cvsimport\n-diff\n-fetch\n-format-patch\n-fsck-objects\n-get-tar-commit-id\n-init-db\n-log\n-ls-remote\n-octopus\n-pack-objects\n-parse-remote\n-patch-id\n-prune\n-pull\n-push\n-rebase\n-relink\n-rename\n-repack\n-request-pull\n-reset\n-resolve\n-revert\n-send-email\n-shortlog\n-show-branch\n-status\n-tag\n-verify-tag\n-whatchanged\n-EOF\n---\n0.99.9.GIT\n"},{"id":"11962","messageId":"7vwtj9eaqm.fsf@assigned-by-dhcp.cox.net","threadId":"2540","inReplyTo":"20051115233125.3153B5BF76@nox.op5.se","subject":"Re: [PATCH 1/3] C implementation of the 'git' program, take two.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-11-15T23:45:37Z","receivedAt":"2005-11-15T23:45:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"exon@op5.se (Andreas Ericsson) writes:\n\n> This patch provides a C implementation of the 'git' program and\n> introduces support for putting the git-* commands in a directory\n> of their own.\n\nVery nice, thanks.  Two questions and a half.\n\n> +static void prepend_to_path(const char *dir, int len)\n> +{\n> +\tchar *path, *old_path = getenv(\"PATH\");\n> +\tint path_len = len;\n> +\n> +\tif (!old_path)\n> +\t\told_path = \"/bin:/usr/bin:.\";\n\nThis is to cover strange case and probably would not matter in\npractice, but perhaps without current directory?\n\n> +int main(int argc, char **argv, char **envp)\n> +{\n> +\tchar git_command[PATH_MAX + 1];\n> +\tchar wd[PATH_MAX + 1];\n> +\tint i, len, show_help = 0;\n> +\tchar *exec_path = getenv(\"GIT_EXEC_PATH\");\n> +\n> +\tgetcwd(wd, PATH_MAX);\n> +...\n> +\t/* allow relative paths, but run with exact */\n> +\tif (chdir(exec_path)) {\n> +\t\tprintf(\"git: '%s': %s\\n\", exec_path, strerror(errno));\n> +\t\texit (1);\n> +\t}\n> +\n> +\tgetcwd(git_command, sizeof(git_command));\n> +\tchdir(wd);\n\nCan we always come back from where we started?\n\n> +\n> +\tlen = strlen(git_command);\n> +\tprepend_to_path(git_command, len);\n> +\n> +\tstrncat(&git_command[len], \"/git-\", sizeof(git_command) - len);\n> +\tlen += 5;\n> +\tstrncat(&git_command[len], argv[i], sizeof(git_command) - len);\n> +\n> +\tif (access(git_command, X_OK))\n> +\t\tusage(exec_path, \"'%s' is not a git-command\", argv[i]);\n> +\n> +\t/* execve() can only ever return if it fails */\n> +\texecve(git_command, &argv[i], envp);\n\nShell version for Cygwin seems to do \".exe\" at the end --- does\nit matter?\n"},{"id":"11966","messageId":"437A78FC.10608@op5.se","threadId":"2540","inReplyTo":"7vwtj9eaqm.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/3] C implementation of the 'git' program, take two.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2005-11-16T00:10:36Z","receivedAt":"2005-11-16T00:10:36Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> exon@op5.se (Andreas Ericsson) writes:\n> \n> \n>>This patch provides a C implementation of the 'git' program and\n>>introduces support for putting the git-* commands in a directory\n>>of their own.\n> \n> \n> Very nice, thanks.  Two questions and a half.\n> \n> \n>>+static void prepend_to_path(const char *dir, int len)\n>>+{\n>>+\tchar *path, *old_path = getenv(\"PATH\");\n>>+\tint path_len = len;\n>>+\n>>+\tif (!old_path)\n>>+\t\told_path = \"/bin:/usr/bin:.\";\n> \n> \n> This is to cover strange case and probably would not matter in\n> practice, but perhaps without current directory?\n> \n\nI have no preference really and since it already covers a strange case \nit probably shouldn't matter either way.\n\n> \n>>+int main(int argc, char **argv, char **envp)\n>>+{\n>>+\tchar git_command[PATH_MAX + 1];\n>>+\tchar wd[PATH_MAX + 1];\n>>+\tint i, len, show_help = 0;\n>>+\tchar *exec_path = getenv(\"GIT_EXEC_PATH\");\n>>+\n>>+\tgetcwd(wd, PATH_MAX);\n>>+...\n>>+\t/* allow relative paths, but run with exact */\n>>+\tif (chdir(exec_path)) {\n>>+\t\tprintf(\"git: '%s': %s\\n\", exec_path, strerror(errno));\n>>+\t\texit (1);\n>>+\t}\n>>+\n>>+\tgetcwd(git_command, sizeof(git_command));\n>>+\tchdir(wd);\n> \n> \n> Can we always come back from where we started?\n> \n\nNot sure what you mean. Perhaps \"Come back *to* where we started\"?\n\nIf getcwd(wd, sizeof(wd)) fails then chdir(wd) will also fail (or do \nsomething strange, at least). wd is otherwise absolute.\n\n> \n>>+\n>>+\tlen = strlen(git_command);\n>>+\tprepend_to_path(git_command, len);\n>>+\n>>+\tstrncat(&git_command[len], \"/git-\", sizeof(git_command) - len);\n>>+\tlen += 5;\n>>+\tstrncat(&git_command[len], argv[i], sizeof(git_command) - len);\n>>+\n>>+\tif (access(git_command, X_OK))\n>>+\t\tusage(exec_path, \"'%s' is not a git-command\", argv[i]);\n>>+\n>>+\t/* execve() can only ever return if it fails */\n>>+\texecve(git_command, &argv[i], envp);\n> \n> \n> Shell version for Cygwin seems to do \".exe\" at the end --- does\n> it matter?\n> \n\nDunno, really. I suppose it does as it bypasses the shell with the \nexecve() call, unless windows or the cygwin stuff does some trickery to \nfind an .exe regardless.\n\nIs it ok if I send a separate patch for it, or would you rather have me \nredo this one?\n\n-- \nAndreas Ericsson                   andreas.ericsson@op5.se\nOP5 AB                             www.op5.se\nTel: +46 8-230225                  Fax: +46 8-230231\n"},{"id":"11967","messageId":"7vmzk5e993.fsf@assigned-by-dhcp.cox.net","threadId":"2540","inReplyTo":"437A78FC.10608@op5.se","subject":"Re: [PATCH 1/3] C implementation of the 'git' program, take two.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-11-16T00:17:44Z","receivedAt":"2005-11-16T00:17:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andreas Ericsson <ae@op5.se> writes:\n\n> Dunno, really. I suppose it does as it bypasses the shell with the \n> execve() call, unless windows or the cygwin stuff does some trickery to \n> find an .exe regardless.\n>\n> Is it ok if I send a separate patch for it, or would you rather have me \n> redo this one?\n\nI'll take this as is and have Cygwin folks holler if it breaks\nthings for them ;-).  Thanks.\n"},{"id":"11968","messageId":"Pine.LNX.4.64.0511151603510.11232@g5.osdl.org","threadId":"2540","inReplyTo":"20051115233125.3153B5BF76@nox.op5.se","subject":"Re: [PATCH 1/3] C implementation of the 'git' program, take two.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-11-16T00:18:26Z","receivedAt":"2005-11-16T00:18:26Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 16 Nov 2005, Andreas Ericsson wrote:\n> +\n> +\t/* allow relative paths, but run with exact */\n> +\tif (chdir(exec_path)) {\n> +\t\tprintf(\"git: '%s': %s\\n\", exec_path, strerror(errno));\n> +\t\texit (1);\n> +\t}\n> +\n> +\tgetcwd(git_command, sizeof(git_command));\n> +\tchdir(wd);\n\nArgh. This is pretty horrible way to turn a path into an absolute one. \nEspecially since you didn't even test whether the original \"wd\" was \nsuccessful.\n\nWhy don't you just do\n\n\tif (exec_path[0] != '/') {\n\t\t.. prepend \"cwd/\" to exec_path ..\n\nsince as far as I can tell you don't actually care whether it's a \nsimplified path or not (you can remove \"./\" at the beginning just to make \nit cleaner, if you wish. In fact, you can remove \"../\" at the beginning \ntoo (but only the beginning) since getcwd() shouldn't have any symlink \ncomponents).\n\nThe reason to avoid \"chdir(relative) + chdir(back)\" is that it totally \nunnecessarily breaks under some extreme cases. For example, if the \nexec_path is already absolute, and we just happen to be in a really deep \nsubdirectory, then the getcwd() could have failed due to the PATH_MAX \nlimitations.\n\nAlso, depending on getcwd() will not work if any parent directory is \nunreadable or non-executable (well, under Linux it will, as long as it's \nexecutable, since getcwd() is actually a system call. Not in UNIX in \ngeneral, though). Again, that means that unless you _have_ to know what \nthe cwd is, you should try to avoid relying on it.\n\nNow, there are Linux-specific tricks that can avoid some of the problems \nif you want to, but they are very much hacks:\n\n\tif (filename[0] != '/') {\n\t\tfd = open(filename, O_DIRECTORY);\n\t\tif (fd >= 0) {\n\t\t\tsnprintf(link_name, sizeof(link_name), \"/proc/self/fd/%d\", fd);\n\t\t\tif (!readlink(link_name ...)) {\n\t\t\t\t.. there it is ..\n\t\t\t}\n\t\t\tclose(fd);\n\t\t}\n\t..\n\nand the nicer thign to do is to just not try to be clever.\n\n\t\tLinus\n"},{"id":"11971","messageId":"437A8067.9050308@op5.se","threadId":"2540","inReplyTo":"Pine.LNX.4.64.0511151603510.11232@g5.osdl.org","subject":"Re: [PATCH 1/3] C implementation of the 'git' program, take two.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2005-11-16T00:42:15Z","receivedAt":"2005-11-16T00:42:15Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Linus Torvalds wrote:\n> \n> On Wed, 16 Nov 2005, Andreas Ericsson wrote:\n> \n>>+\n>>+\t/* allow relative paths, but run with exact */\n>>+\tif (chdir(exec_path)) {\n>>+\t\tprintf(\"git: '%s': %s\\n\", exec_path, strerror(errno));\n>>+\t\texit (1);\n>>+\t}\n>>+\n>>+\tgetcwd(git_command, sizeof(git_command));\n>>+\tchdir(wd);\n> \n> \n> Argh. This is pretty horrible way to turn a path into an absolute one. \n\n\nIt was your idea to begin with, actually, in the thread on how to do \nproper path validation in the git-daemon. :)\n\n> \n> Why don't you just do\n> \n> \tif (exec_path[0] != '/') {\n> \t\t.. prepend \"cwd/\" to exec_path ..\n> \n\nYou mean\n\tsetenv(\"PATH\", concat3(cwd, exec_path, old_path), 1);\n\n?\n\nBecause that can fail too. Every solution is bad if you twist and turn \nit enough.\n\n> \n> The reason to avoid \"chdir(relative) + chdir(back)\" is that it totally \n> unnecessarily breaks under some extreme cases. For example, if the \n> exec_path is already absolute, and we just happen to be in a really deep \n> subdirectory, then the getcwd() could have failed due to the PATH_MAX \n> limitations.\n> \n\nTrue. So, would something like\nchar *try_goddamn_hard_to_get_working_dir(char *rel_path)\n{\n\tsize_t plen = PATH_MAX;\n\tchar *p = malloc(PATH_MAX);\n\twhile(p && plen < 1 << 20 && !(p = getcwd(p, plen))) {\n\t\tplen += PATH_MAX;\n\t\tp = realloc(p, plen);\n\t}\n\n\treturn p;\n}\n\nbe considered safe from this particular point of view?\n\n\n> Also, depending on getcwd() will not work if any parent directory is \n> unreadable or non-executable (well, under Linux it will, as long as it's \n> executable, since getcwd() is actually a system call. Not in UNIX in \n> general, though). Again, that means that unless you _have_ to know what \n> the cwd is, you should try to avoid relying on it.\n> \n\nTrue. I'll redo that part (tomorrow).\n\n> \n> and the nicer thign to do is to just not try to be clever.\n> \n\nI never try. It just comes to me naturally. ;)\n\n-- \nAndreas Ericsson                   andreas.ericsson@op5.se\nOP5 AB                             www.op5.se\nTel: +46 8-230225                  Fax: +46 8-230231\n"},{"id":"11980","messageId":"Pine.LNX.4.64.0511151751520.13959@g5.osdl.org","threadId":"2540","inReplyTo":"437A8067.9050308@op5.se","subject":"Re: [PATCH 1/3] C implementation of the 'git' program, take two.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-11-16T02:02:32Z","receivedAt":"2005-11-16T02:02:32Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 16 Nov 2005, Andreas Ericsson wrote:\n> \n> It was your idea to begin with, actually, in the thread on how to do proper\n> path validation in the git-daemon. :)\n\nBut that was because the important part wasn't to get an absolute path, \nbut because the important part was to get the _canonical_ path. \n\nThat was for security. \n\nBesides, I don't think we ever switched back. We just did a chdir() + a \ngetcwd().\n\n> > Why don't you just do\n> > \n> > \tif (exec_path[0] != '/') {\n> > \t\t.. prepend \"cwd/\" to exec_path ..\n> > \n> \n> You mean\n> \tsetenv(\"PATH\", concat3(cwd, exec_path, old_path), 1);\n\nNo.\n\nI mean exactly what I said.\n\nI mean testing whether the exec_path[] is already absolute, and not \ntouching it at all if it is.\n\nIt's really as simple as something like\n\n\tconst char *absolute_path(const char *input)\n\t{\n\t\tint a, b;\n\t\tchar *buf;\n\t\tchar cwd[PATH_MAX];\n\n\t\tif (*input == '/')\n\t\t\treturn input;\n\n\t\tif (!getcwd(cwd, sizeof(cwd))\n\t\t\treturn NULL;\n\n\t\t/* Do some trivial cleanup */\n\t\twhile (!strncmp(input, \"./\", 2)) {\n\t\t\tinput += 2;\n\t\t\twhile (*input == '/')\n\t\t\t\tinput++;\n\t\t}\n\n\t\ta = strlen(cwd);\n\t\tb = strlen(input);\n\t\tbuf = malloc(a + b + 2);\n\t\tif (!buf)\n\t\t\treturn NULL;\n\n\t\tmemcpy(buf, cwd, a);\n\t\tbuf[a] = '/';\n\t\tmemcpy(buf + a + 1, input, b);\n\t\tbuf[a + 1 + b] = 0;\n\t\treturn buf;\n\t}\n\nand there it is.\n\nThe magic rule being:\n - if the path is already absolute, it's _good_. Don't play games with it.\n - just append the dang thing with cwd. Don't play games (the above does \n   trivial simplification, which is unnecessary, but it's so simple that \n   hey, who cares? And it makes one common case a bit prettier)\n\nThen, you just prepend it to the PATH, with a : in between (and if the \npathname has a \":\" in it, tough, there's nothing we can do about it).\n\n\t\tLinus\n"},{"id":"11981","messageId":"Pine.LNX.4.63.0511160316510.14820@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"2540","inReplyTo":"Pine.LNX.4.64.0511151751520.13959@g5.osdl.org","subject":"Re: [PATCH 1/3] C implementation of the 'git' program, take two.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2005-11-16T02:18:05Z","receivedAt":"2005-11-16T02:18:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 15 Nov 2005, Linus Torvalds wrote:\n\n> Then, you just prepend it to the PATH, with a : in between (and if the \n> pathname has a \":\" in it, tough, there's nothing we can do about it).\n\nYou mean like \"c:/cygwin/git\"? Yes, I know, the default is \n\"/cygdrive/c/cygwin/git\", but there are people out there with \":\" in their \npathname.\n\nCiao,\nDscho\n"},{"id":"12047","messageId":"20051116210449.GA4191@steel.home","threadId":"2540","inReplyTo":"Pine.LNX.4.63.0511160316510.14820@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 1/3] C implementation of the 'git' program, take two.","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2005-11-16T21:04:49Z","receivedAt":"2005-11-16T21:04:49Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Johannes Schindelin, Wed, Nov 16, 2005 03:18:05 +0100:\n> > Then, you just prepend it to the PATH, with a : in between (and if the \n> > pathname has a \":\" in it, tough, there's nothing we can do about it).\n> \n> You mean like \"c:/cygwin/git\"? Yes, I know, the default is \n> \"/cygdrive/c/cygwin/git\", but there are people out there with \":\" in their \n> pathname.\n> \n\nand you can't possibly be hinting at \"\\\" as path element separator...\n\nCygwin rewrites values in PATH (well, poorly: there are things like\n\"z:.\", which admins of another stupid thing, novel netware, are so much\nfond of).\n"}]}