{"thread":{"id":"24389","subject":"[PATCH/RFC 2/4] git-shell-commands: Add a command to list bare repos","startedAt":"2010-07-14T03:01:11Z","lastAt":"2010-07-24T15:27:37Z","messageCount":19,"participants":["Greg Brockman","Ævar Arnfjörð Bjarmason","Johannes Sixt","Kevin P. Fleming","Bernhard R. Link","Junio C Hamano","Thomas Rast","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"145506","messageId":"1279076475-27730-1-git-send-email-gdb@mit.edu","threadId":"24389","inReplyTo":null,"subject":"[PATCH/RFC 0/4] Providing mechanism to list available repositories","fromName":"Greg Brockman","fromEmail":"gdb@mit.edu","sentAt":"2010-07-14T03:01:11Z","receivedAt":"2010-07-14T03:01:11Z","isPatch":true,"sender":{"key":"gdb@mit.edu","avatar":"https://gravatar.com/avatar/3b907cf60d0cadf6d0659d5429d971b16965fb6b1e3f5acd26a6ba2974b45da4?d=mp&s=160"},"body":"I'm working on a project that has separate Git repositories for\ndifferent components.  Repositories are cloneable via\n  git clone git@xvm.mit.edu:path/to/repo.git,\nwhere the 'git' user's shell is git-shell.\n\nWe have been seeking a simple and maintainable way for users to\ndiscover the set of available repositories.  E.g. posting the list on\nour website would add extra steps for users to find and retrieve the\nlist as well as require extra effort from our end.  Since we already give\nusers ssh access to git@xvm.mit.edu, we would like to multiplex the\nfunctionality to allow discovery of available repositories.\n\nOur solution is to expose a 'list' command to the end user, invocable\nas\n  ssh git@xvm.mit.edu list,\nwhich displays the available repositories.\n\nWe find this mechanism useful in that it requires no extra\ninfrastructure on either our end or the user's end.  Our\nimplementation is extensible, allowing the system administrator to\nplace arbitrary commands in ~/git-shell-commands (if the directory is\nomitted, no extra functionality is exposed), and also supports an\ninteractive mode.\n\nWhat do people think of this approach?  I'd love to get this\nfunctionality merged in some form.\n\nThank you!\n\nGreg Brockman\n"},{"id":"145507","messageId":"1279076475-27730-2-git-send-email-gdb@mit.edu","threadId":"24389","inReplyTo":"1279076475-27730-1-git-send-email-gdb@mit.edu","subject":"[PATCH/RFC 1/4] Allow creation of arbitrary git-shell commands","fromName":"Greg Brockman","fromEmail":"gdb@mit.edu","sentAt":"2010-07-14T03:01:12Z","receivedAt":"2010-07-14T03:01:12Z","isPatch":true,"sender":{"key":"gdb@mit.edu","avatar":"https://gravatar.com/avatar/3b907cf60d0cadf6d0659d5429d971b16965fb6b1e3f5acd26a6ba2974b45da4?d=mp&s=160"},"body":"This provides a mechanism for the server to expose custom\nfunctionality to clients.  My particular use case is that I would like\na way of discovering all repositories available for cloning.  A\nclient that clones via\n  git clone user@example.com\ncan invoke a command by\n  ssh user@example.com $command\n\nSigned-off-by: Greg Brockman <gdb@mit.edu>\n---\n shell.c |   16 ++++++++++++++++\n 1 files changed, 16 insertions(+), 0 deletions(-)\n\ndiff --git a/shell.c b/shell.c\nindex e4864e0..3fee0ed 100644\n--- a/shell.c\n+++ b/shell.c\n@@ -3,6 +3,8 @@\n #include \"exec_cmd.h\"\n #include \"strbuf.h\"\n \n+#define COMMAND_DIR \"git-shell-commands\"\n+\n static int do_generic_cmd(const char *me, char *arg)\n {\n \tconst char *my_argv[4];\n@@ -33,6 +35,12 @@ static int do_cvs_cmd(const char *me, char *arg)\n \treturn execv_git_cmd(cvsserver_argv);\n }\n \n+static int is_valid_cmd_name(const char *cmd)\n+{\n+\t/* Test command contains no . or / characters */\n+\treturn cmd[strcspn(cmd, \"./\")] == '\\0';\n+}\n+\n \n static struct commands {\n \tconst char *name;\n@@ -99,5 +107,13 @@ int main(int argc, char **argv)\n \t\t}\n \t\texit(cmd->exec(cmd->name, arg));\n \t}\n+\n+\t/* Shell should be spawned with cwd in the git user's home directory */\n+\tif (chdir(COMMAND_DIR))\n+\t\tdie(\"unrecognized command '%s'\", prog);\n+\n+\tif (is_valid_cmd_name(prog))\n+\t\texecl(prog, prog, (char *) NULL);\n+\n \tdie(\"unrecognized command '%s'\", prog);\n }\n-- \n1.7.0.4\n"},{"id":"145505","messageId":"1279076475-27730-3-git-send-email-gdb@mit.edu","threadId":"24389","inReplyTo":"1279076475-27730-1-git-send-email-gdb@mit.edu","subject":"[PATCH/RFC 2/4] git-shell-commands: Add a command to list bare repos","fromName":"Greg Brockman","fromEmail":"gdb@mit.edu","sentAt":"2010-07-14T03:01:13Z","receivedAt":"2010-07-14T03:01:13Z","isPatch":true,"sender":{"key":"gdb@mit.edu","avatar":"https://gravatar.com/avatar/3b907cf60d0cadf6d0659d5429d971b16965fb6b1e3f5acd26a6ba2974b45da4?d=mp&s=160"},"body":"Signed-off-by: Greg Brockman <gdb@mit.edu>\n---\n git-shell-commands/list |    9 +++++++++\n 1 files changed, 9 insertions(+), 0 deletions(-)\n create mode 100755 git-shell-commands/list\n\ndiff --git a/git-shell-commands/list b/git-shell-commands/list\nnew file mode 100755\nindex 0000000..dca2472\n--- /dev/null\n+++ b/git-shell-commands/list\n@@ -0,0 +1,9 @@\n+#!/bin/bash\n+\n+cd \"..\";\n+# TODO: make safe for spaces\n+for dir in $(find -type d -name '*.git'); do\n+    if [ \"$(git --git-dir=\"$dir\" rev-parse --is-bare-repository)\" = \"true\" ]; then\n+\techo \"${dir#./}\"\n+    fi\n+done\n-- \n1.7.0.4\n"},{"id":"145508","messageId":"1279076475-27730-4-git-send-email-gdb@mit.edu","threadId":"24389","inReplyTo":"1279076475-27730-1-git-send-email-gdb@mit.edu","subject":"[PATCH/RFC 3/4] git-shell-commands: Add a help command","fromName":"Greg Brockman","fromEmail":"gdb@mit.edu","sentAt":"2010-07-14T03:01:14Z","receivedAt":"2010-07-14T03:01:14Z","isPatch":true,"sender":{"key":"gdb@mit.edu","avatar":"https://gravatar.com/avatar/3b907cf60d0cadf6d0659d5429d971b16965fb6b1e3f5acd26a6ba2974b45da4?d=mp&s=160"},"body":"Signed-off-by: Greg Brockman <gdb@mit.edu>\n---\n git-shell-commands/help |    7 +++++++\n 1 files changed, 7 insertions(+), 0 deletions(-)\n create mode 100755 git-shell-commands/help\n\ndiff --git a/git-shell-commands/help b/git-shell-commands/help\nnew file mode 100755\nindex 0000000..a6b1a68\n--- /dev/null\n+++ b/git-shell-commands/help\n@@ -0,0 +1,7 @@\n+#!/bin/sh\n+\n+echo \"Commands you may want to run by hand:\"\n+ls\n+if tty -s; then\n+    echo \"You can leave by running 'exit'\"\n+fi\n-- \n1.7.0.4\n"},{"id":"145509","messageId":"1279076475-27730-5-git-send-email-gdb@mit.edu","threadId":"24389","inReplyTo":"1279076475-27730-1-git-send-email-gdb@mit.edu","subject":"[PATCH/RFC 4/4] Add interactive mode to git-shell for user-friendliness","fromName":"Greg Brockman","fromEmail":"gdb@mit.edu","sentAt":"2010-07-14T03:01:15Z","receivedAt":"2010-07-14T03:01:15Z","isPatch":true,"sender":{"key":"gdb@mit.edu","avatar":"https://gravatar.com/avatar/3b907cf60d0cadf6d0659d5429d971b16965fb6b1e3f5acd26a6ba2974b45da4?d=mp&s=160"},"body":"Signed-off-by: Greg Brockman <gdb@mit.edu>\n---\n shell.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++++--\n 1 files changed, 48 insertions(+), 2 deletions(-)\n\ndiff --git a/shell.c b/shell.c\nindex 3fee0ed..9f80226 100644\n--- a/shell.c\n+++ b/shell.c\n@@ -1,8 +1,11 @@\n+#include <stdio.h>\n+\n #include \"cache.h\"\n #include \"quote.h\"\n #include \"exec_cmd.h\"\n #include \"strbuf.h\"\n \n+#define MAX_LINE_LEN 128\n #define COMMAND_DIR \"git-shell-commands\"\n \n static int do_generic_cmd(const char *me, char *arg)\n@@ -41,6 +44,26 @@ static int is_valid_cmd_name(const char *cmd)\n \treturn cmd[strcspn(cmd, \"./\")] == '\\0';\n }\n \n+static int run(const char *prog)\n+{\n+\tpid_t pid, res;\n+\tint w;\n+\tpid = fork();\n+\tif (pid == -1) {\n+\t\tperror(\"fork\");\n+\t\texit(-1);\n+\t} else if ( pid == 0 ) {\n+\t\texecl(prog, prog, (char *) NULL);\n+\t\tif (prog[0] != '\\0')\n+\t\t\tfprintf(stderr, \"unrecognized command '%s'\\n\", prog);\n+\t\texit(127);\n+\t} else {\n+\t\tdo {\n+\t\t\tres = waitpid (pid, &w, 0);\n+\t\t} while (res == -1 && errno == EINTR);\n+\t}\n+}\n+\n \n static struct commands {\n \tconst char *name;\n@@ -56,6 +79,7 @@ static struct commands {\n int main(int argc, char **argv)\n {\n \tchar *prog;\n+\tchar line[MAX_LINE_LEN];\n \tstruct commands *cmd;\n \tint devnull_fd;\n \n@@ -81,8 +105,30 @@ int main(int argc, char **argv)\n \t * We do not accept anything but \"-c\" followed by \"cmd arg\",\n \t * where \"cmd\" is a very limited subset of git commands.\n \t */\n-\telse if (argc != 3 || strcmp(argv[1], \"-c\"))\n-\t\tdie(\"What do you think I am? A shell?\");\n+\telse if (argc != 3 || strcmp(argv[1], \"-c\")) {\n+\t\tif (chdir(COMMAND_DIR))\n+\t\t\tdie(\"Sorry, the interactive git-shell is not enabled\");\n+\t\tfor (;;) {\n+\t\t\tprintf(\"git> \");\n+\t\t\tif (fgets(line, MAX_LINE_LEN, stdin) == NULL) {\n+\t\t\t\tprintf(\"\\n\");\n+\t\t\t\texit(0);\n+\t\t\t}\n+\n+\t\t\tif (line[strlen(line) - 1] == '\\n')\n+\t\t\t\tline[strlen(line) - 1] = '\\0';\n+\n+\t\t\tif (!strcmp(line, \"quit\") || !strcmp(line, \"logout\") ||\n+\t\t\t\t   !strcmp(line, \"exit\")) {\n+\t\t\t\texit(0);\n+\t\t\t} else if (!strcmp(line, \"\")) {\n+\t\t\t} else if (is_valid_cmd_name(line)) {\n+\t\t\t\trun(line);\n+\t\t\t} else {\n+\t\t\t\tfprintf(stderr, \"invalid command format '%s'\\n\", line);\n+\t\t\t}\n+\t\t};\n+\t}\n \n \tprog = argv[2];\n \tif (!strncmp(prog, \"git\", 3) && isspace(prog[3]))\n-- \n1.7.0.4\n"},{"id":"145522","messageId":"AANLkTil4XkVXM-96Jb7UOpH2CZBmtXEf7eEIIgrsqhg5@mail.gmail.com","threadId":"24389","inReplyTo":"1279076475-27730-5-git-send-email-gdb@mit.edu","subject":"Re: [PATCH/RFC 4/4] Add interactive mode to git-shell for user-friendliness","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-14T09:04:49Z","receivedAt":"2010-07-14T09:04:49Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Wed, Jul 14, 2010 at 03:01, Greg Brockman <gdb@mit.edu> wrote:\n> +               execl(prog, prog, (char *) NULL);\n\nWhy the casting of NULL? It's not done in the builtin/help.c code.\n\nAnyway, if it was cast it should be to (const char *), shouldn't it?\n"},{"id":"145525","messageId":"4C3D910B.7080401@viscovery.net","threadId":"24389","inReplyTo":"1279076475-27730-5-git-send-email-gdb@mit.edu","subject":"Re: [PATCH/RFC 4/4] Add interactive mode to git-shell for user-friendliness","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2010-07-14T10:27:23Z","receivedAt":"2010-07-14T10:27:23Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"I don't have an immediate need for features implemented by this series,\nbut I think they can be useful occasionally.\n\nAm 7/14/2010 5:01, schrieb Greg Brockman:\n> --- a/shell.c\n> +++ b/shell.c\n> @@ -1,8 +1,11 @@\n> +#include <stdio.h>\n\nIs it really needed? Doesn't cache.h pull it in already?\n\n> +\n>  #include \"cache.h\"\n...\n> +static int run(const char *prog)\n> +{\n> +\tpid_t pid, res;\n> +\tint w;\n> +\tpid = fork();\n> +\tif (pid == -1) {\n> +\t\tperror(\"fork\");\n> +\t\texit(-1);\n> +\t} else if ( pid == 0 ) {\n> +\t\texecl(prog, prog, (char *) NULL);\n> +\t\tif (prog[0] != '\\0')\n> +\t\t\tfprintf(stderr, \"unrecognized command '%s'\\n\", prog);\n> +\t\texit(127);\n> +\t} else {\n> +\t\tdo {\n> +\t\t\tres = waitpid (pid, &w, 0);\n> +\t\t} while (res == -1 && errno == EINTR);\n> +\t}\n> +}\n\nIs there a reason that you duplicate functionality offered by run_command()?\n\n> @@ -81,8 +105,30 @@ int main(int argc, char **argv)\n>  \t * We do not accept anything but \"-c\" followed by \"cmd arg\",\n>  \t * where \"cmd\" is a very limited subset of git commands.\n>  \t */\n> -\telse if (argc != 3 || strcmp(argv[1], \"-c\"))\n> -\t\tdie(\"What do you think I am? A shell?\");\n> +\telse if (argc != 3 || strcmp(argv[1], \"-c\")) {\n> +\t\tif (chdir(COMMAND_DIR))\n> +\t\t\tdie(\"Sorry, the interactive git-shell is not enabled\");\n> +\t\tfor (;;) {\n> +\t\t\tprintf(\"git> \");\n> +\t\t\tif (fgets(line, MAX_LINE_LEN, stdin) == NULL) {\n> +\t\t\t\tprintf(\"\\n\");\n> +\t\t\t\texit(0);\n> +\t\t\t}\n> +\n> +\t\t\tif (line[strlen(line) - 1] == '\\n')\n> +\t\t\t\tline[strlen(line) - 1] = '\\0';\n> +\n> +\t\t\tif (!strcmp(line, \"quit\") || !strcmp(line, \"logout\") ||\n> +\t\t\t\t   !strcmp(line, \"exit\")) {\n> +\t\t\t\texit(0);\n> +\t\t\t} else if (!strcmp(line, \"\")) {\n> +\t\t\t} else if (is_valid_cmd_name(line)) {\n> +\t\t\t\trun(line);\n> +\t\t\t} else {\n> +\t\t\t\tfprintf(stderr, \"invalid command format '%s'\\n\", line);\n> +\t\t\t}\n> +\t\t};\n> +\t}\n\nI can imagine that this loop grows in the future, so I suggest to move it\nto a separate function right from the beginning.\n\nI think it would make sense to print a help message before the first prompt.\n\n-- Hannes\n"},{"id":"145537","messageId":"4C3DC2BD.6020907@digium.com","threadId":"24389","inReplyTo":"AANLkTil4XkVXM-96Jb7UOpH2CZBmtXEf7eEIIgrsqhg5@mail.gmail.com","subject":"Re: [PATCH/RFC 4/4] Add interactive mode to git-shell for user-friendliness","fromName":"Kevin P. Fleming","fromEmail":"kpfleming@digium.com","sentAt":"2010-07-14T13:59:25Z","receivedAt":"2010-07-14T13:59:25Z","isPatch":true,"sender":{"key":"kpfleming@digium.com","avatar":null},"body":"On 07/14/2010 04:04 AM, Ævar Arnfjörð Bjarmason wrote:\n> On Wed, Jul 14, 2010 at 03:01, Greg Brockman <gdb@mit.edu> wrote:\n>> +               execl(prog, prog, (char *) NULL);\n> \n> Why the casting of NULL? It's not done in the builtin/help.c code.\n> \n> Anyway, if it was cast it should be to (const char *), shouldn't it?\n\nWhen a NULL sentinel is passed to a varargs function that only\nunderstands 'char *' arguments, the NULL must be cast specifically,\notherwise it will appear in the varargs array as an int or a long.\nexecl() is an example of a varargs function that only uses varargs\nfunctionality to accept a variable *number* of arguments, it does not\nallow for arguments of differing types, so it does not check the types\nof its arguments at all. On any platform where an int and a pointer are\nnot the same size, this can cause a serious problem. When we came across\nthis problem in Asterisk, we added a macro called SENTINEL (that just\nexpands to the proper type for the target platform) that is used in\nthese cases, so that it is clear to the reader of the code what is going on.\n\n-- \nKevin P. Fleming\nDigium, Inc. | Director of Software Technologies\n445 Jan Davis Drive NW - Huntsville, AL 35806 - USA\nskype: kpfleming | jabber: kfleming@digium.com\nCheck us out at www.digium.com & www.asterisk.org\n"},{"id":"145542","messageId":"20100714152444.GA26674@pcpool00.mathematik.uni-freiburg.de","threadId":"24389","inReplyTo":"4C3DC2BD.6020907@digium.com","subject":"Re: [PATCH/RFC 4/4] Add interactive mode to git-shell for user-friendliness","fromName":"Bernhard R. Link","fromEmail":"brlink@debian.org","sentAt":"2010-07-14T15:24:44Z","receivedAt":"2010-07-14T15:24:44Z","isPatch":true,"sender":{"key":"brlink@debian.org","avatar":null},"body":"* Kevin P. Fleming <kpfleming@digium.com> [100714 15:59]:\n> On 07/14/2010 04:04 AM, Ævar Arnfjörð Bjarmason wrote:\n> > On Wed, Jul 14, 2010 at 03:01, Greg Brockman <gdb@mit.edu> wrote:\n> >> +               execl(prog, prog, (char *) NULL);\n> >\n> > Why the casting of NULL? It's not done in the builtin/help.c code.\n> >\n> > Anyway, if it was cast it should be to (const char *), shouldn't it?\n>\n> When a NULL sentinel is passed to a varargs function that only\n> understands 'char *' arguments, the NULL must be cast specifically,\n> otherwise it will appear in the varargs array as an int or a long.\n\nTo be more specific: If NULL is (void *)0 then it does not need to be\ncast. Sadly the standard allows to define it as 0, and so it is on\nsome systems. So to be portable it needs to be cast to be a pointer,\notherwise the varargs argument is assumed to be an int.\n\n\tBernhard R. Link\n"},{"id":"145543","messageId":"7vbpaaytfl.fsf@alter.siamese.dyndns.org","threadId":"24389","inReplyTo":"1279076475-27730-2-git-send-email-gdb@mit.edu","subject":"Re: [PATCH/RFC 1/4] Allow creation of arbitrary git-shell commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-07-14T15:27:58Z","receivedAt":"2010-07-14T15:27:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Greg Brockman <gdb@MIT.EDU> writes:\n\n> This provides a mechanism for the server to expose custom\n> functionality to clients.  My particular use case is that I would like\n> a way of discovering all repositories available for cloning.  A\n> client that clones via\n>   git clone user@example.com\n> can invoke a command by\n>   ssh user@example.com $command\n\nPlease have a blank line above and below sample command display like these\nfor readability.\n\n> Signed-off-by: Greg Brockman <gdb@mit.edu>\n> ---\n>  shell.c |   16 ++++++++++++++++\n>  1 files changed, 16 insertions(+), 0 deletions(-)\n>\n> diff --git a/shell.c b/shell.c\n> index e4864e0..3fee0ed 100644\n> --- a/shell.c\n> +++ b/shell.c\n> @@ -3,6 +3,8 @@\n>  #include \"exec_cmd.h\"\n>  #include \"strbuf.h\"\n>  \n> +#define COMMAND_DIR \"git-shell-commands\"\n> +\n>  static int do_generic_cmd(const char *me, char *arg)\n>  {\n>  \tconst char *my_argv[4];\n> @@ -33,6 +35,12 @@ static int do_cvs_cmd(const char *me, char *arg)\n>  \treturn execv_git_cmd(cvsserver_argv);\n>  }\n>  \n> +static int is_valid_cmd_name(const char *cmd)\n> +{\n> +\t/* Test command contains no . or / characters */\n> +\treturn cmd[strcspn(cmd, \"./\")] == '\\0';\n> +}\n> +\n>  \n>  static struct commands {\n>  \tconst char *name;\n> @@ -99,5 +107,13 @@ int main(int argc, char **argv)\n>  \t\t}\n>  \t\texit(cmd->exec(cmd->name, arg));\n>  \t}\n> +\n> +\t/* Shell should be spawned with cwd in the git user's home directory */\n> +\tif (chdir(COMMAND_DIR))\n> +\t\tdie(\"unrecognized command '%s'\", prog);\n\nHmm, could you justify \"should be\" above please?\n\nAn example would be \"All of the custom commands I wrote to give added\nfeatures to users at my installation wanted to be in that directory, not\nat the user's home directory, as they mostly operated on files in that\ndirectory\", but please do not make me (or other reviewers) guess why.\n\nWhat I am getting at is that it may be more natural and useful to run\nthese custom commands in the user's $HOME directory---you would need to\nmake sure that execl() finds the command you get from the request, perhaps\nby prefixing COMMAND_DIR / to the command name, though.\n\n> +\tif (is_valid_cmd_name(prog))\n> +\t\texecl(prog, prog, (char *) NULL);\n> +\n>  \tdie(\"unrecognized command '%s'\", prog);\n>  }\n"},{"id":"145547","messageId":"201007141740.37867.trast@student.ethz.ch","threadId":"24389","inReplyTo":"20100714152444.GA26674@pcpool00.mathematik.uni-freiburg.de","subject":"Re: [PATCH/RFC 4/4] Add interactive mode to git-shell for user-friendliness","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2010-07-14T15:40:37Z","receivedAt":"2010-07-14T15:40:37Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"[Please don't trim the Cc list without good reason.]\n\nBernhard R. Link wrote:\n> * Kevin P. Fleming <kpfleming@digium.com> [100714 15:59]:\n> > On 07/14/2010 04:04 AM, Ævar Arnfjörð Bjarmason wrote:\n> > > On Wed, Jul 14, 2010 at 03:01, Greg Brockman <gdb@mit.edu> wrote:\n> > >> +               execl(prog, prog, (char *) NULL);\n> > >\n> > > Why the casting of NULL? It's not done in the builtin/help.c code.\n> > >\n> > > Anyway, if it was cast it should be to (const char *), shouldn't it?\n> >\n> > When a NULL sentinel is passed to a varargs function that only\n> > understands 'char *' arguments, the NULL must be cast specifically,\n> > otherwise it will appear in the varargs array as an int or a long.\n> \n> To be more specific: If NULL is (void *)0 then it does not need to be\n> cast. Sadly the standard allows to define it as 0, and so it is on\n> some systems. So to be portable it needs to be cast to be a pointer,\n> otherwise the varargs argument is assumed to be an int.\n\nWorse, the pointer representations need not be the same between types,\neven though that is a fairly exotic idea:\n\n  http://c-faq.com/null/machexamp.html\n\nSo it seems execl() must always have an explicitly-cast (char*)NULL\nsentinel.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"145569","messageId":"AANLkTimAkSB1bysyE6R3CWp-U3vk_S5L0CbMhIWXJfHE@mail.gmail.com","threadId":"24389","inReplyTo":"7vbpaaytfl.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC 1/4] Allow creation of arbitrary git-shell commands","fromName":"Greg Brockman","fromEmail":"gdb@mit.edu","sentAt":"2010-07-14T17:42:49Z","receivedAt":"2010-07-14T17:42:49Z","isPatch":true,"sender":{"key":"gdb@mit.edu","avatar":"https://gravatar.com/avatar/3b907cf60d0cadf6d0659d5429d971b16965fb6b1e3f5acd26a6ba2974b45da4?d=mp&s=160"},"body":">> This provides a mechanism for the server to expose custom\n>> functionality to clients.  My particular use case is that I would like\n>> a way of discovering all repositories available for cloning.  A\n>> client that clones via\n>>   git clone user@example.com\n>> can invoke a command by\n>>   ssh user@example.com $command\n>\n> Please have a blank line above and below sample command display like these\n> for readability.\nSounds good.\n\n>> +     /* Shell should be spawned with cwd in the git user's home directory */\n>> +     if (chdir(COMMAND_DIR))\n>> +             die(\"unrecognized command '%s'\", prog);\n>\n> Hmm, could you justify \"should be\" above please?\n>\n> An example would be \"All of the custom commands I wrote to give added\n> features to users at my installation wanted to be in that directory, not\n> at the user's home directory, as they mostly operated on files in that\n> directory\", but please do not make me (or other reviewers) guess why.\n>\n> What I am getting at is that it may be more natural and useful to run\n> these custom commands in the user's $HOME directory---you would need to\n> make sure that execl() finds the command you get from the request, perhaps\n> by prefixing COMMAND_DIR / to the command name, though.\nErr, good point.  The commands I wrote end up running 'cd ..' anyway\n:).  Instead just running these commands in the user's $HOME does make\na lot more sense.\n\nThanks everyone for the comments thus far.\n"},{"id":"145576","messageId":"7viq4hyj3g.fsf@alter.siamese.dyndns.org","threadId":"24389","inReplyTo":"1279076475-27730-1-git-send-email-gdb@mit.edu","subject":"Re: [PATCH/RFC 0/4] Providing mechanism to list available repositories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-07-14T19:11:15Z","receivedAt":"2010-07-14T19:11:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Greg Brockman <gdb@MIT.EDU> writes:\n\n> We find this mechanism useful in that it requires no extra\n> infrastructure on either our end or the user's end.  Our\n> implementation is extensible, allowing the system administrator to\n> place arbitrary commands in ~/git-shell-commands (if the directory is\n> omitted, no extra functionality is exposed), and also supports an\n> interactive mode.\n>\n> What do people think of this approach?  I'd love to get this\n> functionality merged in some form.\n\nIt seems to me that any time you need to add a new helper command, the\nadministrator needs to make sure that appears in ~$user/git-shell-commands\nof all the users who need it.  When adding a new user, a similar\nmanagement action needs to happen.  Perhaps that is done by making a\nsymlink from all the users' home directories to one shared place.  Is that\nthe general idea?\n\nIn any case, I'd prefer that the sample command implementations like list\nand help to live in contrib/ somewhere.  They are not part of what the\nmain Makefile needs to know about, right?\n"},{"id":"145578","messageId":"AANLkTilCoyOcm8cvW06UTWJk7P4m6WNLeZICHrTp5-aI@mail.gmail.com","threadId":"24389","inReplyTo":"7viq4hyj3g.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC 0/4] Providing mechanism to list available repositories","fromName":"Greg Brockman","fromEmail":"gdb@mit.edu","sentAt":"2010-07-14T19:29:31Z","receivedAt":"2010-07-14T19:29:31Z","isPatch":true,"sender":{"key":"gdb@mit.edu","avatar":"https://gravatar.com/avatar/3b907cf60d0cadf6d0659d5429d971b16965fb6b1e3f5acd26a6ba2974b45da4?d=mp&s=160"},"body":">> We find this mechanism useful in that it requires no extra\n>> infrastructure on either our end or the user's end.  Our\n>> implementation is extensible, allowing the system administrator to\n>> place arbitrary commands in ~/git-shell-commands (if the directory is\n>> omitted, no extra functionality is exposed), and also supports an\n>> interactive mode.\n>>\n>> What do people think of this approach?  I'd love to get this\n>> functionality merged in some form.\n>\n> It seems to me that any time you need to add a new helper command, the\n> administrator needs to make sure that appears in ~$user/git-shell-commands\n> of all the users who need it.  When adding a new user, a similar\n> management action needs to happen.  Perhaps that is done by making a\n> symlink from all the users' home directories to one shared place.  Is that\n> the general idea?\nThat's correct.  Our particular environment only has a single git\nuser, but if we were to add more we would probably make\ngit-shell-commands a symlink as you suggest.\n\n> In any case, I'd prefer that the sample command implementations like list\n> and help to live in contrib/ somewhere.  They are not part of what the\n> main Makefile needs to know about, right?\nAlso correct.  I'll look for a reasonable place within contrib/ to put them.\n"},{"id":"145718","messageId":"AANLkTikiOgV1iE7dwPUkLpWTb_zXSFdEuOYvyqJ1eDCo@mail.gmail.com","threadId":"24389","inReplyTo":"AANLkTikEjMeKPkyY4RdRq-ESkmmq4PvqCFPgp8yvLVBz@mail.gmail.com","subject":"Re: [PATCH/RFC 4/4] Add interactive mode to git-shell for user-friendliness","fromName":"Greg Brockman","fromEmail":"gdb@mit.edu","sentAt":"2010-07-17T04:12:33Z","receivedAt":"2010-07-17T04:12:33Z","isPatch":true,"sender":{"key":"gdb@mit.edu","avatar":"https://gravatar.com/avatar/3b907cf60d0cadf6d0659d5429d971b16965fb6b1e3f5acd26a6ba2974b45da4?d=mp&s=160"},"body":"It looks like git@ got dropped from the CC at some point, but I had\nwritten a few days ago:\n\n>> Is there a reason that you duplicate functionality offered by run_command()?\n> No--I hadn't realized that existed.  I'll switch over to that in V2 of\n> this patch series.\n\nToday, I inspected run_command() in more detail.  Unfortunately, I'm\nnot sure of the best way to use it in this situation.  In particular,\nrun_command() uses execvp, meaning that PATH is invoked.  However, the\nuser should only be able to run commands in the git-shell-commands\ndirectory.   I do have a few ideas for approaches here; maybe others\nsee more?  Anyway:\n- Set PATH to just $HOME/git-shell-commands.  But then the helper\nscripts have to restore PATH to a sane value themselves, and it's not\nreally clear to me what that value should be.\n- Under the hood, exec a different script, which processes the user's\ncommand on its own.  (So if the user types 'help' at the git shell\nprompt, actually exec 'git-shell-wrapper help'.)  The\ngit-shell-wrapper could be a dumb wrapper that just execs\n$HOME/git-shell-commands/help or similar.\n- Extend run_command to optionally use execv.  Would any other code\nactually want this functionality though?  If not, it's probably an\nexcessively large code change for little benefit.\n- Continue using the one-off run() method that I wrote here.\n\nDo people have opinions on the most elegant way to handle this?\n\nThanks!\n\nGreg\n\n> On Wed, Jul 14, 2010 at 12:07 PM, Bernhard R. Link <brlink@debian.org> wrote:\n>> * Thomas Rast <trast@student.ethz.ch> [100714 17:41]:\n>>> [Please don't trim the Cc list without good reason.]\n>>\n>> The mail I answered to had only git@vger.kernel.org in CC and some\n>> syntax errors in To.\n>>\n>>> Bernhard R. Link wrote:\n>>> > To be more specific: If NULL is (void *)0 then it does not need to be\n>>> > cast. Sadly the standard allows to define it as 0, and so it is on\n>>> > some systems. So to be portable it needs to be cast to be a pointer,\n>>> > otherwise the varargs argument is assumed to be an int.\n>>>\n>>> Worse, the pointer representations need not be the same between types,\n>>> even though that is a fairly exotic idea:\n>>>\n>>>   http://c-faq.com/null/machexamp.html\n>>>\n>>> So it seems execl() must always have an explicitly-cast (char*)NULL\n>>> sentinel.\n>>\n>> There is a difference between ugly operating systems where everything\n>> else works and you need to cast it and things too exotic to have any\n>> chance to get the rest of the code to work without big changes.\n>>\n>> Machines where you do not get a NULL pointer by a memset(,0,), calloc\n>> or the like will have bigger problems anyway. (have not looked at git,\n>> but I'd be suprised if at every place there is an explicit assignment\n>> for the pointers).\n>> Note that in the other examples, char * and void * are the same anyway.\n>>\n>>        Bernhard R. Link\n>>\n>\n"},{"id":"145719","messageId":"20100717055257.GB29290@burratino","threadId":"24389","inReplyTo":"AANLkTikiOgV1iE7dwPUkLpWTb_zXSFdEuOYvyqJ1eDCo@mail.gmail.com","subject":"Re: [PATCH/RFC 4/4] Add interactive mode to git-shell for user-friendliness","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-17T05:52:57Z","receivedAt":"2010-07-17T05:52:57Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Greg,\n\nGreg Brockman wrote:\n\n> - Extend run_command to optionally use execv.  Would any other code\n> actually want this functionality though?  If not, it's probably an\n> excessively large code change for little benefit.\n\nOf the options you presented, this is the best one.  It doesn’t matter\nwhether any other code would use it; even if you are the only caller,\nit is still good because\n\n - if someone else needs the facility, it will be obvious where\n   to find it\n\n - you can share the existing logic to portably run a command\n   (i.e., near free portability to msys).\n\nrun_command() already takes an argument for options like\nRUN_USING_SHELL; your new facility would fit right in.\n\nBut first a more basic question: why not just add “./” to the start of\nthe command name?\n"},{"id":"145722","messageId":"AANLkTik7VJlCIZHGVLX-eVRTrf45RDUggQqq-FjUtqq9@mail.gmail.com","threadId":"24389","inReplyTo":"20100717055257.GB29290@burratino","subject":"Re: [PATCH/RFC 4/4] Add interactive mode to git-shell for user-friendliness","fromName":"Greg Brockman","fromEmail":"gdb@mit.edu","sentAt":"2010-07-17T14:53:08Z","receivedAt":"2010-07-17T14:53:08Z","isPatch":true,"sender":{"key":"gdb@mit.edu","avatar":"https://gravatar.com/avatar/3b907cf60d0cadf6d0659d5429d971b16965fb6b1e3f5acd26a6ba2974b45da4?d=mp&s=160"},"body":"> But first a more basic question: why not just add “./” to the start of\n> the command name?\nWow, of course... that's the obvious solution.\n\nThanks for the comments!\n"},{"id":"146193","messageId":"00564b8ba93617801bb78b4a0ec67784e597d02a.1279983892.git.trast@student.ethz.ch","threadId":"24389","inReplyTo":"201007141740.37867.trast@student.ethz.ch","subject":"[PATCH] Cast execl*() NULL sentinels to (char *)","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2010-07-24T15:20:23Z","receivedAt":"2010-07-24T15:20:23Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"The NULL sentinel argument to the execl*() family of calls must be\ncast to (char *), as otherwise:\n\n- platforms where NULL is just 0 (not (void *)) would pass an int\n\n- (admittedly esoteric) platforms where NULL is (void *)0 and (void *)\n  and (char *) have different memory layouts would pass the wrong kind\n  of pointer\n\nSigned-off-by: Thomas Rast <trast@student.ethz.ch>\n---\n\nLet's not forget about this.\n\n builtin/help.c |   12 ++++++------\n 1 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/help.c b/builtin/help.c\nindex a9836b0..61ff798 100644\n--- a/builtin/help.c\n+++ b/builtin/help.c\n@@ -120,7 +120,7 @@ static void exec_woman_emacs(const char *path, const char *page)\n \t\tif (!path)\n \t\t\tpath = \"emacsclient\";\n \t\tstrbuf_addf(&man_page, \"(woman \\\"%s\\\")\", page);\n-\t\texeclp(path, \"emacsclient\", \"-e\", man_page.buf, NULL);\n+\t\texeclp(path, \"emacsclient\", \"-e\", man_page.buf, (char *)NULL);\n \t\twarning(\"failed to exec '%s': %s\", path, strerror(errno));\n \t}\n }\n@@ -148,7 +148,7 @@ static void exec_man_konqueror(const char *path, const char *page)\n \t\t} else\n \t\t\tpath = \"kfmclient\";\n \t\tstrbuf_addf(&man_page, \"man:%s(1)\", page);\n-\t\texeclp(path, filename, \"newTab\", man_page.buf, NULL);\n+\t\texeclp(path, filename, \"newTab\", man_page.buf, (char *)NULL);\n \t\twarning(\"failed to exec '%s': %s\", path, strerror(errno));\n \t}\n }\n@@ -157,7 +157,7 @@ static void exec_man_man(const char *path, const char *page)\n {\n \tif (!path)\n \t\tpath = \"man\";\n-\texeclp(path, \"man\", page, NULL);\n+\texeclp(path, \"man\", page, (char *)NULL);\n \twarning(\"failed to exec '%s': %s\", path, strerror(errno));\n }\n \n@@ -165,7 +165,7 @@ static void exec_man_cmd(const char *cmd, const char *page)\n {\n \tstruct strbuf shell_cmd = STRBUF_INIT;\n \tstrbuf_addf(&shell_cmd, \"%s %s\", cmd, page);\n-\texecl(\"/bin/sh\", \"sh\", \"-c\", shell_cmd.buf, NULL);\n+\texecl(\"/bin/sh\", \"sh\", \"-c\", shell_cmd.buf, (char *)NULL);\n \twarning(\"failed to exec '%s': %s\", cmd, strerror(errno));\n }\n \n@@ -372,7 +372,7 @@ static void show_info_page(const char *git_cmd)\n {\n \tconst char *page = cmd_to_page(git_cmd);\n \tsetenv(\"INFOPATH\", system_path(GIT_INFO_PATH), 1);\n-\texeclp(\"info\", \"info\", \"gitman\", page, NULL);\n+\texeclp(\"info\", \"info\", \"gitman\", page, (char *)NULL);\n \tdie(\"no info viewer handled the request\");\n }\n \n@@ -398,7 +398,7 @@ static void get_html_page_path(struct strbuf *page_path, const char *page)\n #ifndef open_html\n static void open_html(const char *path)\n {\n-\texecl_git_cmd(\"web--browse\", \"-c\", \"help.browser\", path, NULL);\n+\texecl_git_cmd(\"web--browse\", \"-c\", \"help.browser\", path, (char *)NULL);\n }\n #endif\n \n-- \n1.7.2.278.g76edd.dirty\n"},{"id":"146194","messageId":"AANLkTimxB-n4oq-5XUSK5XwEXLnwLsakyuryNlUnghJr@mail.gmail.com","threadId":"24389","inReplyTo":"00564b8ba93617801bb78b4a0ec67784e597d02a.1279983892.git.trast@student.ethz.ch","subject":"Re: [PATCH] Cast execl*() NULL sentinels to (char *)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-24T15:27:37Z","receivedAt":"2010-07-24T15:27:37Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sat, Jul 24, 2010 at 15:20, Thomas Rast <trast@student.ethz.ch> wrote:\n> The NULL sentinel argument to the execl*() family of calls must be\n> cast to (char *), as otherwise:\n>\n> - platforms where NULL is just 0 (not (void *)) would pass an int\n>\n> - (admittedly esoteric) platforms where NULL is (void *)0 and (void *)\n>  and (char *) have different memory layouts would pass the wrong kind\n>  of pointer\n>\n> Signed-off-by: Thomas Rast <trast@student.ethz.ch>\n\nNice that you got around to this after I inadvertently pointed it out\nin another thread.\n\nAcked.\n"}]}