{"thread":{"id":"22016","subject":"Patch which adds syslog support to git-shell","startedAt":"2009-12-23T17:32:00Z","lastAt":"2009-12-24T13:03:32Z","messageCount":3,"participants":["Gerhard Gappmeier","Erik Faye-Lund"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"130283","messageId":"200912231832.03541.gerhard.gappmeier@ascolab.com","threadId":"22016","inReplyTo":null,"subject":"Patch which adds syslog support to git-shell","fromName":"Gerhard Gappmeier","fromEmail":"gerhard.gappmeier@ascolab.com","sentAt":"2009-12-23T17:32:00Z","receivedAt":"2009-12-23T17:32:00Z","isPatch":false,"sender":{"key":"gerhard.gappmeier@ascolab.com","avatar":"https://gravatar.com/avatar/5b63eaa6f5ab8e059a28e3b70981e3088c9272b16489e49bce896b1f773ca43d?d=mp&s=160"},"body":"Hi\n\nI'm not sure if this is the right list, but here is my first GIT patch.\n\nI had a problem with git-shell and wanted to analyze it.\nUnfortunately it does not contain any trace capabilities.\nSo I cloned git and added some basic syslog support.\nAfter that I recognized that the current git version just works ;-)\nbut the syslog functionality is always a nice thing I think.\nSo here is the patch.\n\nMerry X-Mas.\n\n-- \n\nmit freundlichen Grüßen / best regards\n\nGerhard Gappmeier\nascolab GmbH - automation system communication laboratory\nTel.: +49 9131 691 123\nFax: +49 9131 691 128\nWeb: http://www.ascolab.com\nGPG Key-Id: 5AAC50C4\nGPG Fingerprint: 967A 15F1 2788 164D CCA3 6C46 07CD 6F82 5AAC 50C4\n\n\nFrom 0683e3a249419620abcf4363086cd53ec34972e8 Mon Sep 17 00:00:00 2001\nFrom: Gerhard Gappmeier <gerhard.gappmeier@ascolab.com>\nDate: Wed, 23 Dec 2009 17:19:34 +0100\nSubject: [PATCH] Added syslog functionality to git-shell.\n\n---\n shell.c |  192 ++++++++++++++++++++++++++++++++++++++-------------------------\n 1 files changed, 115 insertions(+), 77 deletions(-)\n\ndiff --git a/shell.c b/shell.c\nindex e4864e0..4cdf385 100644\n--- a/shell.c\n+++ b/shell.c\n@@ -1,103 +1,141 @@\n+#include <syslog.h>\n+#include <errno.h>\n #include \"cache.h\"\n #include \"quote.h\"\n #include \"exec_cmd.h\"\n #include \"strbuf.h\"\n \n+/* Syslog defines */\n+#define GIT_SYSLOG_IDENT    \"git-shell\"\n+#define GIT_SYSLOG_OPTION   0\n+#define GIT_SYSLOG_FACILITY LOG_LOCAL0\n+\n static int do_generic_cmd(const char *me, char *arg)\n {\n-\tconst char *my_argv[4];\n+    const char *my_argv[4];\n+    int ret = 0;\n+    \n+    setup_path();\n+    if (!arg || !(arg = sq_dequote(arg))) {\n+        syslog(LOG_INFO, \"bad argument\");\n+        die(\"bad argument\");\n+    }\n+    if (prefixcmp(me, \"git-\")) {\n+        syslog(LOG_INFO, \"bad command\");\n+        die(\"bad command\");\n+    }\n+\n+    my_argv[0] = me + 4;\n+    my_argv[1] = arg;\n+    my_argv[2] = NULL;\n \n-\tsetup_path();\n-\tif (!arg || !(arg = sq_dequote(arg)))\n-\t\tdie(\"bad argument\");\n-\tif (prefixcmp(me, \"git-\"))\n-\t\tdie(\"bad command\");\n+    syslog(LOG_INFO, \"Executing '%s' '%s'.\", my_argv[0], my_argv[1]);\n \n-\tmy_argv[0] = me + 4;\n-\tmy_argv[1] = arg;\n-\tmy_argv[2] = NULL;\n+    ret =  execv_git_cmd(my_argv);\n+    if (ret == -1) {\n+        syslog(LOG_ERR, \" execv_git_cmd failed: %s\\n\", strerror(errno));\n+    }\n \n-\treturn execv_git_cmd(my_argv);\n+    return ret;\n }\n \n static int do_cvs_cmd(const char *me, char *arg)\n {\n-\tconst char *cvsserver_argv[3] = {\n-\t\t\"cvsserver\", \"server\", NULL\n-\t};\n+    const char *cvsserver_argv[3] = {\n+        \"cvsserver\", \"server\", NULL\n+    };\n+    int ret = 0;\n+\n+    if (!arg || strcmp(arg, \"server\")) {\n+        syslog(LOG_INFO, \"git-cvsserver only handles server: %s\", arg);\n+        die(\"git-cvsserver only handles server: %s\", arg);\n+    }\n+\n+    setup_path();\n \n-\tif (!arg || strcmp(arg, \"server\"))\n-\t\tdie(\"git-cvsserver only handles server: %s\", arg);\n+    syslog(LOG_INFO, \"Executing '%s' '%s'.\", cvsserver_argv[0], cvsserver_argv[1]);\n \n-\tsetup_path();\n-\treturn execv_git_cmd(cvsserver_argv);\n+    ret = execv_git_cmd(cvsserver_argv);\n+    if (ret == -1) {\n+        syslog(LOG_ERR, \" execv_git_cmd failed: %s\\n\", strerror(errno));\n+    }\n+\n+    return ret;\n }\n \n \n static struct commands {\n-\tconst char *name;\n-\tint (*exec)(const char *me, char *arg);\n+    const char *name;\n+    int (*exec)(const char *me, char *arg);\n } cmd_list[] = {\n-\t{ \"git-receive-pack\", do_generic_cmd },\n-\t{ \"git-upload-pack\", do_generic_cmd },\n-\t{ \"git-upload-archive\", do_generic_cmd },\n-\t{ \"cvs\", do_cvs_cmd },\n-\t{ NULL },\n+    { \"git-receive-pack\", do_generic_cmd },\n+    { \"git-upload-pack\", do_generic_cmd },\n+    { \"git-upload-archive\", do_generic_cmd },\n+    { \"cvs\", do_cvs_cmd },\n+    { NULL },\n };\n \n int main(int argc, char **argv)\n {\n-\tchar *prog;\n-\tstruct commands *cmd;\n-\tint devnull_fd;\n-\n-\t/*\n-\t * Always open file descriptors 0/1/2 to avoid clobbering files\n-\t * in die().  It also avoids not messing up when the pipes are\n-\t * dup'ed onto stdin/stdout/stderr in the child processes we spawn.\n-\t */\n-\tdevnull_fd = open(\"/dev/null\", O_RDWR);\n-\twhile (devnull_fd >= 0 && devnull_fd <= 2)\n-\t\tdevnull_fd = dup(devnull_fd);\n-\tif (devnull_fd == -1)\n-\t\tdie_errno(\"opening /dev/null failed\");\n-\tclose (devnull_fd);\n-\n-\t/*\n-\t * Special hack to pretend to be a CVS server\n-\t */\n-\tif (argc == 2 && !strcmp(argv[1], \"cvs server\"))\n-\t\targv--;\n-\n-\t/*\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-\n-\tprog = argv[2];\n-\tif (!strncmp(prog, \"git\", 3) && isspace(prog[3]))\n-\t\t/* Accept \"git foo\" as if the caller said \"git-foo\". */\n-\t\tprog[3] = '-';\n-\n-\tfor (cmd = cmd_list ; cmd->name ; cmd++) {\n-\t\tint len = strlen(cmd->name);\n-\t\tchar *arg;\n-\t\tif (strncmp(cmd->name, prog, len))\n-\t\t\tcontinue;\n-\t\targ = NULL;\n-\t\tswitch (prog[len]) {\n-\t\tcase '\\0':\n-\t\t\targ = NULL;\n-\t\t\tbreak;\n-\t\tcase ' ':\n-\t\t\targ = prog + len + 1;\n-\t\t\tbreak;\n-\t\tdefault:\n-\t\t\tcontinue;\n-\t\t}\n-\t\texit(cmd->exec(cmd->name, arg));\n-\t}\n-\tdie(\"unrecognized command '%s'\", prog);\n+    char *prog;\n+    struct commands *cmd;\n+    int devnull_fd;\n+\n+    /* Open syslog. */\n+    openlog(GIT_SYSLOG_IDENT, GIT_SYSLOG_OPTION, GIT_SYSLOG_FACILITY);\n+\n+    /*\n+     * Always open file descriptors 0/1/2 to avoid clobbering files\n+     * in die().  It also avoids not messing up when the pipes are\n+     * dup'ed onto stdin/stdout/stderr in the child processes we spawn.\n+     */\n+    devnull_fd = open(\"/dev/null\", O_RDWR);\n+    while (devnull_fd >= 0 && devnull_fd <= 2)\n+        devnull_fd = dup(devnull_fd);\n+    if (devnull_fd == -1)\n+        die_errno(\"opening /dev/null failed\");\n+    close (devnull_fd);\n+\n+    /*\n+     * Special hack to pretend to be a CVS server\n+     */\n+    if (argc == 2 && !strcmp(argv[1], \"cvs server\"))\n+        argv--;\n+\n+    /*\n+     * We do not accept anything but \"-c\" followed by \"cmd arg\",\n+     * where \"cmd\" is a very limited subset of git commands.\n+     */\n+    else if (argc != 3 || strcmp(argv[1], \"-c\")) {\n+        syslog(LOG_WARNING, \"Invalid parameter '%s'\", argv[1]);\n+        die(\"What do you think I am? A shell?\");\n+    }\n+\n+    prog = argv[2];\n+    if (!strncmp(prog, \"git\", 3) && isspace(prog[3]))\n+        /* Accept \"git foo\" as if the caller said \"git-foo\". */\n+        prog[3] = '-';\n+\n+    for (cmd = cmd_list ; cmd->name ; cmd++) {\n+        int len = strlen(cmd->name);\n+        char *arg;\n+        if (strncmp(cmd->name, prog, len))\n+            continue;\n+        arg = NULL;\n+        switch (prog[len]) {\n+        case '\\0':\n+            arg = NULL;\n+            break;\n+        case ' ':\n+            arg = prog + len + 1;\n+            break;\n+        default:\n+            continue;\n+        }\n+        exit(cmd->exec(cmd->name, arg));\n+    }\n+\n+    syslog(LOG_WARNING, \"Somebody tried to execute an unallowed command '%s'\", prog);\n+    die(\"unrecognized command '%s'\", prog);\n }\n+\n-- \n1.6.4.4\n\n"},{"id":"130310","messageId":"40aa078e0912240338g7765d17cyf948650deab2aa01@mail.gmail.com","threadId":"22016","inReplyTo":"200912231832.03541.gerhard.gappmeier@ascolab.com","subject":"Re: Patch which adds syslog support to git-shell","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2009-12-24T11:38:57Z","receivedAt":"2009-12-24T11:38:57Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"Please read Documentation/SubmittingPatches. We prefer inline patches,\nas they are easier to review.\n\nOn Wed, Dec 23, 2009 at 6:32 PM, Gerhard Gappmeier\n<gerhard.gappmeier@ascolab.com> wrote:\n> Hi\n>\n> I'm not sure if this is the right list, but here is my first GIT patch.\n>\n\nIt's the right list :)\n\n> I had a problem with git-shell and wanted to analyze it.\n> Unfortunately it does not contain any trace capabilities.\n> So I cloned git and added some basic syslog support.\n> After that I recognized that the current git version just works ;-)\n> but the syslog functionality is always a nice thing I think.\n> So here is the patch.\n\nLooking at your patch, I see there's a lot of white-space changes. Stuff like:\n\n> -\tconst char *cvsserver_argv[3] = {\n> -\t\t\"cvsserver\", \"server\", NULL\n> -\t};\n> +    const char *cvsserver_argv[3] = {\n> +        \"cvsserver\", \"server\", NULL\n> +    };\n\njust makes this harder to review. Besides, we use tabs for indentation in git.\n\nAlso, I think it would be better to use set_die_routine() from usage.h\nthan to change all the die call-sites. This is what git-daemon does:\n\n--->8---\n\tif (log_syslog) {\n\t\topenlog(\"git-daemon\", LOG_PID, LOG_DAEMON);\n\t\tset_die_routine(daemon_die);\n\t}\n--->8---\nLook at daemon.c for the implementation of daemon_die().\n\n+/* Syslog defines */\n+#define GIT_SYSLOG_IDENT    \"git-shell\"\n+#define GIT_SYSLOG_OPTION   0\n+#define GIT_SYSLOG_FACILITY LOG_LOCAL0\n+\n\nIs this really needed? These are only used at one place. Just doing\n+    openlog(\"git-shell\", 0, LOG_LOCAL0);\nwould IMO be cleaner.\n\nAnyway, this is all I bother to point out before I see an inlined,\nwhite-space fixed patch.\n\n> Merry X-Mas.\n\nHappy holidays to you too :)\n\n-- \nErik \"kusma\" Faye-Lund\n"},{"id":"130311","messageId":"40aa078e0912240503u3d6ffd26n90aa0e3e30220c93@mail.gmail.com","threadId":"22016","inReplyTo":"40aa078e0912240338g7765d17cyf948650deab2aa01@mail.gmail.com","subject":"Re: Patch which adds syslog support to git-shell","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2009-12-24T13:03:32Z","receivedAt":"2009-12-24T13:03:32Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Thu, Dec 24, 2009 at 12:38 PM, Erik Faye-Lund\n<kusmabite@googlemail.com> wrote:\n> Anyway, this is all I bother to point out before I see an inlined,\n> white-space fixed patch.\n>\n\nActually, I'll add one thing: using syslog in git-shell breaks the\nWindows builds. I don't know about other platforms.\n\nCurrently, git-daemon is the only git-program that depends on syslog,\nand it's excluded from Windows builds (but this is something that\nmight change soon).\n\n-- \nErik \"kusma\" Faye-Lund\n"}]}