{"thread":{"id":"28530","subject":"[PATCH v3 0/4] port upload-archive to Windows","startedAt":"2011-09-29T20:59:19Z","lastAt":"2011-10-06T13:06:04Z","messageCount":15,"participants":["Erik Faye-Lund","Jeff King","René Scharfe","Junio C Hamano","Phil Hord"],"isPatch":true,"patchVersion":3,"patchTotal":4},"messages":[{"id":"176522","messageId":"1317329963-6656-1-git-send-email-kusmabite@gmail.com","threadId":"28530","inReplyTo":null,"subject":"[PATCH v3 0/4] port upload-archive to Windows","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-09-29T20:59:19Z","receivedAt":"2011-09-29T20:59:19Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"It's been a while, but here's another updated version of this\nseries.\n\nThe only change since last time is that the series has been made\ncompatible with Peff's \"when remote, disable some features\"\nchanges.\n\nErik Faye-Lund (4):\n compat/win32/sys/poll.c: upgrade from upstream\n mingw: fix compilation of poll-emulation\n enter_repo: do not modify input\n upload-archive: use start_command instead of fork\n\n builtin/archive.c | 6 +++-\n builtin/upload-archive.c | 68 ++++++++++++++-------------------------------\n cache.h | 2 +-\n compat/mingw.h | 2 -\n compat/win32/sys/poll.c | 17 ++++++++---\n daemon.c | 4 +-\n path.c | 30 +++++++++-----------\n t/t5000-tar-tree.sh | 10 +++---\n 8 files changed, 60 insertions(+), 79 deletions(-)\n\n-- \n1.7.6.355.g842ba.dirty\n"},{"id":"176524","messageId":"1317329963-6656-2-git-send-email-kusmabite@gmail.com","threadId":"28530","inReplyTo":"1317329963-6656-1-git-send-email-kusmabite@gmail.com","subject":"[PATCH v3 1/4] compat/win32/sys/poll.c: upgrade from upstream","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-09-29T20:59:20Z","receivedAt":"2011-09-29T20:59:20Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"poll.c is updated from revision adc3a5b in\ngit://git.savannah.gnu.org/gnulib.git\n\nThe changes are applied with --whitespace=fix to reduce noise.\n\npoll.h is not upgraded, because the most recent version now\ncontains template-stuff that breaks compilation for us.\n\nSigned-off-by: Erik Faye-Lund \n---\n compat/win32/sys/poll.c | 19 +++++++++++++------\n 1 files changed, 13 insertions(+), 6 deletions(-)\n\ndiff --git a/compat/win32/sys/poll.c b/compat/win32/sys/poll.c\nindex 708a6c9..403eaa7 100644\n--- a/compat/win32/sys/poll.c\n+++ b/compat/win32/sys/poll.c\n@@ -1,7 +1,7 @@\n /* Emulation for poll(2)\n Contributed by Paolo Bonzini.\n \n- Copyright 2001-2003, 2006-2010 Free Software Foundation, Inc.\n+ Copyright 2001-2003, 2006-2011 Free Software Foundation, Inc.\n \n This file is part of gnulib.\n \n@@ -27,7 +27,10 @@\n #include \n \n #include \n-#include \"poll.h\"\n+\n+/* Specification. */\n+#include \n+\n #include \n #include \n #include \n@@ -314,10 +317,7 @@ compute_revents (int fd, int sought, fd_set *rfds, fd_set *wfds, fd_set *efds)\n #endif /* !MinGW */\n \n int\n-poll (pfd, nfd, timeout)\n- struct pollfd *pfd;\n- nfds_t nfd;\n- int timeout;\n+poll (struct pollfd *pfd, nfds_t nfd, int timeout)\n {\n #ifndef WIN32_NATIVE\n fd_set rfds, wfds, efds;\n@@ -454,6 +454,7 @@ poll (pfd, nfd, timeout)\n if (!hEvent)\n hEvent = CreateEvent (NULL, FALSE, FALSE, NULL);\n \n+restart:\n handle_array[0] = hEvent;\n nhandles = 1;\n FD_ZERO (&rfds);\n@@ -594,6 +595,12 @@ poll (pfd, nfd, timeout)\n \trc++;\n }\n \n+ if (!rc && timeout == INFTIM)\n+ {\n+ SwitchToThread();\n+ goto restart;\n+ }\n+\n return rc;\n #endif\n }\n-- \n1.7.6.355.g842ba.dirty\n"},{"id":"176523","messageId":"1317329963-6656-3-git-send-email-kusmabite@gmail.com","threadId":"28530","inReplyTo":"1317329963-6656-1-git-send-email-kusmabite@gmail.com","subject":"[PATCH v3 2/4] mingw: fix compilation of poll-emulation","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-09-29T20:59:21Z","receivedAt":"2011-09-29T20:59:21Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"gnulib has changed the inclusion of poll.h from double quotes\nto single-quotes. But because compat/win32/sys/ isn't in our\ninclude-path, this breaks compilation. Change it back the way\nit was.\n\nSigned-off-by: Erik Faye-Lund \n---\n compat/win32/sys/poll.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/compat/win32/sys/poll.c b/compat/win32/sys/poll.c\nindex 403eaa7..225ddce 100644\n--- a/compat/win32/sys/poll.c\n+++ b/compat/win32/sys/poll.c\n@@ -29,7 +29,7 @@\n #include \n \n /* Specification. */\n-#include \n+#include \"poll.h\"\n \n #include \n #include \n-- \n1.7.6.355.g842ba.dirty\n"},{"id":"176525","messageId":"1317329963-6656-4-git-send-email-kusmabite@gmail.com","threadId":"28530","inReplyTo":"1317329963-6656-1-git-send-email-kusmabite@gmail.com","subject":"[PATCH v3 3/4] enter_repo: do not modify input","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-09-29T20:59:22Z","receivedAt":"2011-09-29T20:59:22Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"entr_repo(..., 0) currently modifies the input to strip away\ntrailing slashes. This means that we some times need to copy the\ninput to keep the original.\n\nChange it to unconditionally copy it into the used_path buffer so\nwe can safely use the input without having to copy it.\n\nSigned-off-by: Erik Faye-Lund \n---\n cache.h | 2 +-\n daemon.c | 4 ++--\n path.c | 30 ++++++++++++++----------------\n 3 files changed, 17 insertions(+), 19 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 607c2ea..a7e4de1 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -734,7 +734,7 @@ int safe_create_leading_directories(char *path);\n int safe_create_leading_directories_const(const char *path);\n int mkdir_in_gitdir(const char *path);\n extern char *expand_user_path(const char *path);\n-char *enter_repo(char *path, int strict);\n+const char *enter_repo(const char *path, int strict);\n static inline int is_absolute_path(const char *path)\n {\n \treturn is_dir_sep(path[0]) || has_dos_drive_prefix(path);\ndiff --git a/daemon.c b/daemon.c\nindex 4c8346d..9253192 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -108,11 +108,11 @@ static void NORETURN daemon_die(const char *err, va_list params)\n \texit(1);\n }\n \n-static char *path_ok(char *directory)\n+static const char *path_ok(char *directory)\n {\n \tstatic char rpath[PATH_MAX];\n \tstatic char interp_path[PATH_MAX];\n-\tchar *path;\n+\tconst char *path;\n \tchar *dir;\n \n \tdir = directory;\ndiff --git a/path.c b/path.c\nindex 6f3f5d5..f7dfd0b 100644\n--- a/path.c\n+++ b/path.c\n@@ -283,7 +283,7 @@ return_null:\n * links. User relative paths are also returned as they are given,\n * except DWIM suffixing.\n */\n-char *enter_repo(char *path, int strict)\n+const char *enter_repo(const char *path, int strict)\n {\n \tstatic char used_path[PATH_MAX];\n \tstatic char validated_path[PATH_MAX];\n@@ -297,14 +297,15 @@ char *enter_repo(char *path, int strict)\n \t\t};\n \t\tint len = strlen(path);\n \t\tint i;\n-\t\twhile ((1 < len) && (path[len-1] == '/')) {\n-\t\t\tpath[len-1] = 0;\n+\t\twhile ((1 < len) && (path[len-1] == '/'))\n \t\t\tlen--;\n-\t\t}\n+\n \t\tif (PATH_MAX <= len)\n \t\t\treturn NULL;\n-\t\tif (path[0] == '~') {\n-\t\t\tchar *newpath = expand_user_path(path);\n+\t\tstrncpy(used_path, path, len);\n+\n+\t\tif (used_path[0] == '~') {\n+\t\t\tchar *newpath = expand_user_path(used_path);\n \t\t\tif (!newpath || (PATH_MAX - 10 < strlen(newpath))) {\n \t\t\t\tfree(newpath);\n \t\t\t\treturn NULL;\n@@ -316,24 +317,21 @@ char *enter_repo(char *path, int strict)\n \t\t\t * anyway.\n \t\t\t */\n \t\t\tstrcpy(used_path, newpath); free(newpath);\n-\t\t\tstrcpy(validated_path, path);\n-\t\t\tpath = used_path;\n+\t\t\tstrcpy(validated_path, used_path);\n \t\t}\n \t\telse if (PATH_MAX - 10 < len)\n \t\t\treturn NULL;\n-\t\telse {\n-\t\t\tpath = strcpy(used_path, path);\n-\t\t\tstrcpy(validated_path, path);\n-\t\t}\n-\t\tlen = strlen(path);\n+\t\telse\n+\t\t\tstrcpy(validated_path, used_path);\n+\t\tlen = strlen(used_path);\n \t\tfor (i = 0; suffix[i]; i++) {\n-\t\t\tstrcpy(path + len, suffix[i]);\n-\t\t\tif (!access(path, F_OK)) {\n+\t\t\tstrcpy(used_path + len, suffix[i]);\n+\t\t\tif (!access(used_path, F_OK)) {\n \t\t\t\tstrcat(validated_path, suffix[i]);\n \t\t\t\tbreak;\n \t\t\t}\n \t\t}\n-\t\tif (!suffix[i] || chdir(path))\n+\t\tif (!suffix[i] || chdir(used_path))\n \t\t\treturn NULL;\n \t\tpath = validated_path;\n \t}\n-- \n1.7.6.355.g842ba.dirty\n"},{"id":"176526","messageId":"1317329963-6656-5-git-send-email-kusmabite@gmail.com","threadId":"28530","inReplyTo":"1317329963-6656-1-git-send-email-kusmabite@gmail.com","subject":"[PATCH v3 4/4] upload-archive: use start_command instead of fork","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-09-29T20:59:23Z","receivedAt":"2011-09-29T20:59:23Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"The POSIX-function fork is not supported on Windows. Use our\nstart_command API instead.\n\nAs this is the last call-site that depends on the fork-stub in\ncompat/mingw.h, remove that as well.\n\nAdd an undocumented flag to git-archive that tells it that the\naction originated from a remote, so features can be disabled.\nThanks to Jeff King for work on this part.\n\nRemove the NOT_MINGW-prereq for t5000, as git-archive --remote\nnow works.\n\nSigned-off-by: Erik Faye-Lund \nHelped-by: Jeff King \n---\n builtin/archive.c | 6 +++-\n builtin/upload-archive.c | 68 ++++++++++++++-------------------------------\n compat/mingw.h | 2 -\n t/t5000-tar-tree.sh | 10 +++---\n 4 files changed, 31 insertions(+), 55 deletions(-)\n\ndiff --git a/builtin/archive.c b/builtin/archive.c\nindex 883c009..6668340 100644\n--- a/builtin/archive.c\n+++ b/builtin/archive.c\n@@ -85,6 +85,7 @@ int cmd_archive(int argc, const char **argv, const char *prefix)\n \tconst char *exec = \"git-upload-archive\";\n \tconst char *output = NULL;\n \tconst char *remote = NULL;\n+\tint is_remote = 0;\n \tstruct option local_opts[] = {\n \t\tOPT_STRING('o', \"output\", &output, \"file\",\n \t\t\t\"write the archive to this file\"),\n@@ -92,6 +93,9 @@ int cmd_archive(int argc, const char **argv, const char *prefix)\n \t\t\t\"retrieve the archive from remote repository \"),\n \t\tOPT_STRING(0, \"exec\", &exec, \"cmd\",\n \t\t\t\"path to the remote git-upload-archive command\"),\n+\t\t{ OPTION_BOOLEAN, 0, \"remote-request\", &is_remote, NULL,\n+\t\t\t\"indicate we are serving a remote request\",\n+\t\t\tPARSE_OPT_NOARG | PARSE_OPT_HIDDEN },\n \t\tOPT_END()\n \t};\n \n@@ -106,5 +110,5 @@ int cmd_archive(int argc, const char **argv, const char *prefix)\n \n \tsetvbuf(stderr, NULL, _IOLBF, BUFSIZ);\n \n-\treturn write_archive(argc, argv, prefix, 1, output, 0);\n+\treturn write_archive(argc, argv, prefix, 1, output, is_remote);\n }\ndiff --git a/builtin/upload-archive.c b/builtin/upload-archive.c\nindex 2d0b383..c57e8bd 100644\n--- a/builtin/upload-archive.c\n+++ b/builtin/upload-archive.c\n@@ -6,6 +6,7 @@\n #include \"archive.h\"\n #include \"pkt-line.h\"\n #include \"sideband.h\"\n+#include \"run-command.h\"\n \n static const char upload_archive_usage[] =\n \t\"git upload-archive \";\n@@ -18,28 +19,17 @@ static const char lostchild[] =\n \n #define MAX_ARGS (64)\n \n-static int run_upload_archive(int argc, const char **argv, const char *prefix)\n+static void prepare_argv(const char **sent_argv, const char **argv)\n {\n-\tconst char *sent_argv[MAX_ARGS];\n \tconst char *arg_cmd = \"argument \";\n \tchar *p, buf[4096];\n \tint sent_argc;\n \tint len;\n \n-\tif (argc != 2)\n-\t\tusage(upload_archive_usage);\n-\n-\tif (strlen(argv[1]) + 1 > sizeof(buf))\n-\t\tdie(\"insanely long repository name\");\n-\n-\tstrcpy(buf, argv[1]); /* enter-repo smudges its argument */\n-\n-\tif (!enter_repo(buf, 0))\n-\t\tdie(\"'%s' does not appear to be a git repository\", buf);\n-\n \t/* put received options in sent_argv[] */\n-\tsent_argc = 1;\n-\tsent_argv[0] = \"git-upload-archive\";\n+\tsent_argc = 2;\n+\tsent_argv[0] = \"archive\";\n+\tsent_argv[1] = \"--remote-request\";\n \tfor (p = buf;;) {\n \t\t/* This will die if not enough free space in buf */\n \t\tlen = packet_read_line(0, p, (buf + sizeof buf) - p);\n@@ -62,9 +52,6 @@ static int run_upload_archive(int argc, const char **argv, const char *prefix)\n \t\t*p++ = 0;\n \t}\n \tsent_argv[sent_argc] = NULL;\n-\n-\t/* parse all options sent by the client */\n-\treturn write_archive(sent_argc, sent_argv, prefix, 0, NULL, 1);\n }\n \n __attribute__((format (printf, 1, 2)))\n@@ -96,38 +83,25 @@ static ssize_t process_input(int child_fd, int band)\n \n int cmd_upload_archive(int argc, const char **argv, const char *prefix)\n {\n-\tpid_t writer;\n-\tint fd1[2], fd2[2];\n-\t/*\n-\t * Set up sideband subprocess.\n-\t *\n-\t * We (parent) monitor and read from child, sending its fd#1 and fd#2\n-\t * multiplexed out to our fd#1. If the child dies, we tell the other\n-\t * end over channel #3.\n-\t */\n-\tif (pipe(fd1) < 0 || pipe(fd2) < 0) {\n-\t\tint err = errno;\n-\t\tpacket_write(1, \"NACK pipe failed on the remote side\\n\");\n-\t\tdie(\"upload-archive: %s\", strerror(err));\n-\t}\n-\twriter = fork();\n-\tif (writer < 0) {\n+\tconst char *sent_argv[MAX_ARGS];\n+\tstruct child_process cld = { sent_argv };\n+\tcld.out = cld.err = -1;\n+\tcld.git_cmd = 1;\n+\n+\tif (argc != 2)\n+\t\tusage(upload_archive_usage);\n+\n+\tif (!enter_repo(argv[1], 0))\n+\t\tdie(\"'%s' does not appear to be a git repository\", argv[1]);\n+\n+\tprepare_argv(sent_argv, argv);\n+\tif (start_command(&cld)) {\n \t\tint err = errno;\n \t\tpacket_write(1, \"NACK fork failed on the remote side\\n\");\n \t\tdie(\"upload-archive: %s\", strerror(err));\n \t}\n-\tif (!writer) {\n-\t\t/* child - connect fd#1 and fd#2 to the pipe */\n-\t\tdup2(fd1[1], 1);\n-\t\tdup2(fd2[1], 2);\n-\t\tclose(fd1[1]); close(fd2[1]);\n-\t\tclose(fd1[0]); close(fd2[0]); /* we do not read from pipe */\n-\n-\t\texit(run_upload_archive(argc, argv, prefix));\n-\t}\n \n \t/* parent - read from child, multiplex and send out to fd#1 */\n-\tclose(fd1[1]); close(fd2[1]); /* we do not write to pipe */\n \tpacket_write(1, \"ACK\\n\");\n \tpacket_flush(1);\n \n@@ -135,9 +109,9 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)\n \t\tstruct pollfd pfd[2];\n \t\tint status;\n \n-\t\tpfd[0].fd = fd1[0];\n+\t\tpfd[0].fd = cld.out;\n \t\tpfd[0].events = POLLIN;\n-\t\tpfd[1].fd = fd2[0];\n+\t\tpfd[1].fd = cld.err;\n \t\tpfd[1].events = POLLIN;\n \t\tif (poll(pfd, 2, -1) < 0) {\n \t\t\tif (errno != EINTR) {\n@@ -156,7 +130,7 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)\n \t\t\tif (process_input(pfd[0].fd, 1))\n \t\t\t\tcontinue;\n \n-\t\tif (waitpid(writer, &status, 0) < 0)\n+\t\tif (waitpid(cld.pid, &status, 0) < 0)\n \t\t\terror_clnt(\"%s\", lostchild);\n \t\telse if (!WIFEXITED(status) || WEXITSTATUS(status) > 0)\n \t\t\terror_clnt(\"%s\", deadchild);\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex ce9dd98..9cb6eb9 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -85,8 +85,6 @@ static inline int symlink(const char *oldpath, const char *newpath)\n { errno = ENOSYS; return -1; }\n static inline int fchmod(int fildes, mode_t mode)\n { errno = ENOSYS; return -1; }\n-static inline pid_t fork(void)\n-{ errno = ENOSYS; return -1; }\n static inline unsigned int alarm(unsigned int seconds)\n { return 0; }\n static inline int fsync(int fd)\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex d906898..889842e 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -96,7 +96,7 @@ test_expect_success 'git archive with --output' \\\n 'git archive --output=b4.tar HEAD &&\n test_cmp b.tar b4.tar'\n \n-test_expect_success NOT_MINGW 'git archive --remote' \\\n+test_expect_success 'git archive --remote' \\\n 'git archive --remote=. HEAD >b5.tar &&\n test_cmp b.tar b5.tar'\n \n@@ -266,7 +266,7 @@ test_expect_success 'archive --list mentions user filter' '\n \tgrep \"^bar\\$\" output\n '\n \n-test_expect_success NOT_MINGW 'archive --list shows only enabled remote filters' '\n+test_expect_success 'archive --list shows only enabled remote filters' '\n \tgit archive --list --remote=. >output &&\n \t! grep \"^tar\\.foo\\$\" output &&\n \tgrep \"^bar\\$\" output\n@@ -298,7 +298,7 @@ test_expect_success 'extension matching requires dot' '\n \ttest_cmp b.tar config-implicittar.foo\n '\n \n-test_expect_success NOT_MINGW 'only enabled filters are available remotely' '\n+test_expect_success 'only enabled filters are available remotely' '\n \ttest_must_fail git archive --remote=. --format=tar.foo HEAD \\\n \t\t>remote.tar.foo &&\n \tgit archive --remote=. --format=bar >remote.bar HEAD &&\n@@ -341,12 +341,12 @@ test_expect_success GZIP,GUNZIP 'extract tgz file' '\n \ttest_cmp b.tar j.tar\n '\n \n-test_expect_success GZIP,NOT_MINGW 'remote tar.gz is allowed by default' '\n+test_expect_success GZIP 'remote tar.gz is allowed by default' '\n \tgit archive --remote=. --format=tar.gz HEAD >remote.tar.gz &&\n \ttest_cmp j.tgz remote.tar.gz\n '\n \n-test_expect_success GZIP,NOT_MINGW 'remote tar.gz can be disabled' '\n+test_expect_success GZIP 'remote tar.gz can be disabled' '\n \tgit config tar.tar.gz.remote false &&\n \ttest_must_fail git archive --remote=. --format=tar.gz HEAD \\\n \t\t>remote.tar.gz\n-- \n1.7.6.355.g842ba.dirty\n"},{"id":"176527","messageId":"CABPQNSbU01=y-Z8n5+zkhQfBZwU2M+2HhKr961gCvYBB-Mbc7A@mail.gmail.com","threadId":"28530","inReplyTo":"1317329963-6656-1-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH v3 0/4] port upload-archive to Windows","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-09-29T21:01:14Z","receivedAt":"2011-09-29T21:01:14Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Thu, Sep 29, 2011 at 10:59 PM, Erik Faye-Lund wrote:\n> It's been a while, but here's another updated version of this\n> series.\n>\n> The only change since last time is that the series has been made\n> compatible with Peff's \"when remote, disable some features\"\n> changes.\n\nOh, and I forgot to mention here in the cover-letter: I dropped the\nNO_MINGW-prereq from the archive --remote tests as well.\n"},{"id":"176564","messageId":"20110930104627.GC24507@sigill.intra.peff.net","threadId":"28530","inReplyTo":"1317329963-6656-1-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH v3 0/4] port upload-archive to Windows","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-30T10:46:27Z","receivedAt":"2011-09-30T10:46:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 29, 2011 at 10:59:19PM +0200, Erik Faye-Lund wrote:\n\n> It's been a while, but here's another updated version of this\n> series.\n> \n> The only change since last time is that the series has been made\n> compatible with Peff's \"when remote, disable some features\"\n> changes.\n> \n> Erik Faye-Lund (4):\n> compat/win32/sys/poll.c: upgrade from upstream\n> mingw: fix compilation of poll-emulation\n> enter_repo: do not modify input\n> upload-archive: use start_command instead of fork\n\nThanks. I can't comment on the earlier patches, but 4/4 looks sane to\nme.\n\n-Peff\n"},{"id":"176588","messageId":"4E8611B0.7020709@lsrfire.ath.cx","threadId":"28530","inReplyTo":"1317329963-6656-4-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH v3 3/4] enter_repo: do not modify input","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-09-30T19:00:00Z","receivedAt":"2011-09-30T19:00:00Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 29.09.2011 22:59, schrieb Erik Faye-Lund:\n> diff --git a/path.c b/path.c\n> index 6f3f5d5..f7dfd0b 100644\n> --- a/path.c\n> +++ b/path.c\n> @@ -283,7 +283,7 @@ return_null:\n> * links. User relative paths are also returned as they are given,\n> * except DWIM suffixing.\n> */\n> -char *enter_repo(char *path, int strict)\n> +const char *enter_repo(const char *path, int strict)\n> {\n> \tstatic char used_path[PATH_MAX];\n> \tstatic char validated_path[PATH_MAX];\n> @@ -297,14 +297,15 @@ char *enter_repo(char *path, int strict)\n> \t\t};\n> \t\tint len = strlen(path);\n> \t\tint i;\n> -\t\twhile ((1 < len) && (path[len-1] == '/')) {\n> -\t\t\tpath[len-1] = 0;\n> +\t\twhile ((1 < len) && (path[len-1] == '/'))\n> \t\t\tlen--;\n> -\t\t}\n> +\n> \t\tif (PATH_MAX <= len)\n> \t\t\treturn NULL;\n> -\t\tif (path[0] == '~') {\n> -\t\t\tchar *newpath = expand_user_path(path);\n> +\t\tstrncpy(used_path, path, len);\n> +\n> +\t\tif (used_path[0] == '~') {\n> +\t\t\tchar *newpath = expand_user_path(used_path);\n> \t\t\tif (!newpath || (PATH_MAX - 10 < strlen(newpath))) {\n> \t\t\t\tfree(newpath);\n> \t\t\t\treturn NULL;\n> @@ -316,24 +317,21 @@ char *enter_repo(char *path, int strict)\n> \t\t\t * anyway.\n> \t\t\t */\n> \t\t\tstrcpy(used_path, newpath); free(newpath);\n> -\t\t\tstrcpy(validated_path, path);\n> -\t\t\tpath = used_path;\n> +\t\t\tstrcpy(validated_path, used_path);\n> \t\t}\n> \t\telse if (PATH_MAX - 10 < len)\n> \t\t\treturn NULL;\n> -\t\telse {\n> -\t\t\tpath = strcpy(used_path, path);\n> -\t\t\tstrcpy(validated_path, path);\n> -\t\t}\n> -\t\tlen = strlen(path);\n> +\t\telse\n> +\t\t\tstrcpy(validated_path, used_path);\n> +\t\tlen = strlen(used_path);\n> \t\tfor (i = 0; suffix[i]; i++) {\n> -\t\t\tstrcpy(path + len, suffix[i]);\n> -\t\t\tif (!access(path, F_OK)) {\n> +\t\t\tstrcpy(used_path + len, suffix[i]);\n> +\t\t\tif (!access(used_path, F_OK)) {\n> \t\t\t\tstrcat(validated_path, suffix[i]);\n> \t\t\t\tbreak;\n> \t\t\t}\n> \t\t}\n> -\t\tif (!suffix[i] || chdir(path))\n> +\t\tif (!suffix[i] || chdir(used_path))\n> \t\t\treturn NULL;\n> \t\tpath = validated_path;\n> \t}\n\nThe use of strcpy and strncpy makes me nervous, but I can't spot a bug\ncurrently and strcpy and even strcat calls had been already in there\nbefore your patch.\n\nRené\n"},{"id":"176589","messageId":"4E8611B2.8030907@lsrfire.ath.cx","threadId":"28530","inReplyTo":"1317329963-6656-2-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH v3 1/4] compat/win32/sys/poll.c: upgrade from upstream","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-09-30T19:00:02Z","receivedAt":"2011-09-30T19:00:02Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 29.09.2011 22:59, schrieb Erik Faye-Lund:\n> poll.c is updated from revision adc3a5b in\n> git://git.savannah.gnu.org/gnulib.git\n> \n> The changes are applied with --whitespace=fix to reduce noise.\n> \n> poll.h is not upgraded, because the most recent version now\n> contains template-stuff that breaks compilation for us.\n\n> @@ -27,7 +27,10 @@\n> #include \n> \n> #include \n> -#include \"poll.h\"\n> +\n> +/* Specification. */\n> +#include \n> +\n\nIn order to make bisecting easier, I think it's a good idea to squash in\nthe next patch that undoes this part, i.e. simply skip this hunk and add\na third non-verbatim note to the commit message.\n\nRené\n"},{"id":"176764","messageId":"7vy5x1gbr6.fsf@alter.siamese.dyndns.org","threadId":"28530","inReplyTo":"1317329963-6656-5-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH v3 4/4] upload-archive: use start_command instead of fork","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-03T18:39:57Z","receivedAt":"2011-10-03T18:39:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Erik Faye-Lund writes:\n\n> Add an undocumented flag to git-archive that tells it that the\n> action originated from a remote, so features can be disabled.\n\n> diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c\n> index 2d0b383..c57e8bd 100644\n> --- a/builtin/upload-archive.c\n> +++ b/builtin/upload-archive.c\n> @@ -6,6 +6,7 @@\n> #include \"archive.h\"\n> #include \"pkt-line.h\"\n> #include \"sideband.h\"\n> +#include \"run-command.h\"\n> \n> static const char upload_archive_usage[] =\n> \t\"git upload-archive \";\n> @@ -18,28 +19,17 @@ static const char lostchild[] =\n> \n> #define MAX_ARGS (64)\n> \n> -static int run_upload_archive(int argc, const char **argv, const char *prefix)\n> +static void prepare_argv(const char **sent_argv, const char **argv)\n> {\n> -\tconst char *sent_argv[MAX_ARGS];\n> \tconst char *arg_cmd = \"argument \";\n> \tchar *p, buf[4096];\n> \tint sent_argc;\n> \tint len;\n> \n> +\tsent_argc = 2;\n> +\tsent_argv[0] = \"archive\";\n> +\tsent_argv[1] = \"--remote-request\";\n> \tfor (p = buf;;) {\n> \t\t/* This will die if not enough free space in buf */\n> \t\tlen = packet_read_line(0, p, (buf + sizeof buf) - p);\n\nHmm, forgetting the \"Windows\" for a while, does this client work against\nthe remote repositories that are running deployed versions of Git?\n"},{"id":"176765","messageId":"CABPQNSbN-sktYQUkLoywhCkc0_axgjY6Y9sN8ipeu6M+2VOVTA@mail.gmail.com","threadId":"28530","inReplyTo":"7vy5x1gbr6.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 4/4] upload-archive: use start_command instead of fork","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-10-03T18:48:20Z","receivedAt":"2011-10-03T18:48:20Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Oct 3, 2011 at 8:39 PM, Junio C Hamano wrote:\n> Erik Faye-Lund writes:\n>\n>> Add an undocumented flag to git-archive that tells it that the\n>> action originated from a remote, so features can be disabled.\n>\n>> diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c\n>> index 2d0b383..c57e8bd 100644\n>> --- a/builtin/upload-archive.c\n>> +++ b/builtin/upload-archive.c\n>> @@ -6,6 +6,7 @@\n>>  #include \"archive.h\"\n>>  #include \"pkt-line.h\"\n>>  #include \"sideband.h\"\n>> +#include \"run-command.h\"\n>>\n>>  static const char upload_archive_usage[] =\n>>       \"git upload-archive \";\n>> @@ -18,28 +19,17 @@ static const char lostchild[] =\n>>\n>>  #define MAX_ARGS (64)\n>>\n>> -static int run_upload_archive(int argc, const char **argv, const char *prefix)\n>> +static void prepare_argv(const char **sent_argv, const char **argv)\n>>  {\n>> -     const char *sent_argv[MAX_ARGS];\n>>       const char *arg_cmd = \"argument \";\n>>       char *p, buf[4096];\n>>       int sent_argc;\n>>       int len;\n>>\n>> +     sent_argc = 2;\n>> +     sent_argv[0] = \"archive\";\n>> +     sent_argv[1] = \"--remote-request\";\n>>       for (p = buf;;) {\n>>               /* This will die if not enough free space in buf */\n>>               len = packet_read_line(0, p, (buf + sizeof buf) - p);\n>\n> Hmm, forgetting the \"Windows\" for a while, does this client work against\n> the remote repositories that are running deployed versions of Git?\n>\n\nIt should, the change is completely server-side. The first two entries\nin sent_argv aren't sent over the network protocol, but the ones that\nfollow them are.\n\nOr did I misunderstand your concern?\n"},{"id":"176780","messageId":"7vd3edg9cw.fsf@alter.siamese.dyndns.org","threadId":"28530","inReplyTo":"CABPQNSbN-sktYQUkLoywhCkc0_axgjY6Y9sN8ipeu6M+2VOVTA@mail.gmail.com","subject":"Re: [PATCH v3 4/4] upload-archive: use start_command instead of fork","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-03T19:31:43Z","receivedAt":"2011-10-03T19:31:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Erik Faye-Lund writes:\n\n> It should, the change is completely server-side. The first two entries\n> in sent_argv aren't sent over the network protocol, but the ones that\n> follow them are.\n>\n> Or did I misunderstand your concern?\n\nNo you didn't. Thanks.\n"},{"id":"176858","messageId":"CABURp0qDsxHwsuyvB6-KvKPrKuUT0-Fpr730TD_TxxFY7fotpA@mail.gmail.com","threadId":"28530","inReplyTo":"1317329963-6656-4-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH v3 3/4] enter_repo: do not modify input","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2011-10-04T17:55:27Z","receivedAt":"2011-10-04T17:55:27Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Thu, Sep 29, 2011 at 4:59 PM, Erik Faye-Lund wrote:\n> entr_repo(..., 0) currently modifies the input to strip away\n> trailing slashes. This means that we some times need to copy the\n> input to keep the original.\n\nI'm also modifying enter_repo() so I'm looking a bit closer at this patch now.\n\n> Change it to unconditionally copy it into the used_path buffer so\n> we can safely use the input without having to copy it.\n>\n> Signed-off-by: Erik Faye-Lund \n> ---\n[...]\n>  */\n> -char *enter_repo(char *path, int strict)\n> +const char *enter_repo(const char *path, int strict)\n>  {\n>        static char used_path[PATH_MAX];\n>        static char validated_path[PATH_MAX];\n> @@ -297,14 +297,15 @@ char *enter_repo(char *path, int strict)\n>                };\n>                int len = strlen(path);\n>                int i;\n> -               while ((1 < len) && (path[len-1] == '/')) {\n> -                       path[len-1] = 0;\n> +               while ((1 < len) && (path[len-1] == '/'))\n>                        len--;\n> -               }\n> +\n>                if (PATH_MAX <= len)\n>                        return NULL;\n> -               if (path[0] == '~') {\n> -                       char *newpath = expand_user_path(path);\n> +               strncpy(used_path, path, len);\n\nWhen len < strlen(path), this will will leave used_path unterminated.\n\n> +\n> +               if (used_path[0] == '~') {\n> +                       char *newpath = expand_user_path(used_path);\n>                        if (!newpath || (PATH_MAX - 10 < strlen(newpath))) {\n>                                free(newpath);\n>                                return NULL;\n> @@ -316,24 +317,21 @@ char *enter_repo(char *path, int strict)\n>                         * anyway.\n>                         */\n>                        strcpy(used_path, newpath); free(newpath);\n> -                       strcpy(validated_path, path);\n> -                       path = used_path;\n> +                       strcpy(validated_path, used_path);\n\nThe point of 'validated_path' is to keep the original unmolested,\nunexpanded path string (plus DWIM suffix), but here you've just\nreplaced validated_path with a copy of the expanded_user_path. On the\nother hand, we seem always to strcpy(validated_path , path), so we\nmight as well get that done up-front.\n\nI'll re-roll these changes in in a minute. Stand by.\n\nPhil\n"},{"id":"176859","messageId":"CABURp0raR6dkBJUJOzWvpTcrKobs_kX1e+cM_xJX7qTXVnRjeA@mail.gmail.com","threadId":"28530","inReplyTo":"CABURp0qDsxHwsuyvB6-KvKPrKuUT0-Fpr730TD_TxxFY7fotpA@mail.gmail.com","subject":"Re: [PATCH v3 3/4] enter_repo: do not modify input","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2011-10-04T18:00:29Z","receivedAt":"2011-10-04T18:00:29Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"From: Erik Faye-Lund \n\nentr_repo(..., 0) currently modifies the input to strip away\ntrailing slashes. This means that we some times need to copy the\ninput to keep the original.\n\nChange it to unconditionally copy it into the used_path buffer so\nwe can safely use the input without having to copy it. Also store\na working copy in validated_path up-front before we start\nresolving anything.\n\nSigned-off-by: Erik Faye-Lund \nSigned-off-by: Phil Hord \n---\n cache.h | 2 +-\n daemon.c | 4 ++--\n path.c | 28 ++++++++++++----------------\n 3 files changed, 15 insertions(+), 19 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 607c2ea..a7e4de1 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -734,7 +734,7 @@ int safe_create_leading_directories(char *path);\n int safe_create_leading_directories_const(const char *path);\n int mkdir_in_gitdir(const char *path);\n extern char *expand_user_path(const char *path);\n-char *enter_repo(char *path, int strict);\n+const char *enter_repo(const char *path, int strict);\n static inline int is_absolute_path(const char *path)\n {\n \treturn is_dir_sep(path[0]) || has_dos_drive_prefix(path);\ndiff --git a/daemon.c b/daemon.c\nindex 4c8346d..9253192 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -108,11 +108,11 @@ static void NORETURN daemon_die(const char *err,\nva_list params)\n \texit(1);\n }\n\n-static char *path_ok(char *directory)\n+static const char *path_ok(char *directory)\n {\n \tstatic char rpath[PATH_MAX];\n \tstatic char interp_path[PATH_MAX];\n-\tchar *path;\n+\tconst char *path;\n \tchar *dir;\n\n \tdir = directory;\ndiff --git a/path.c b/path.c\nindex 6f3f5d5..01028f2 100644\n--- a/path.c\n+++ b/path.c\n@@ -283,7 +283,7 @@ return_null:\n * links. User relative paths are also returned as they are given,\n * except DWIM suffixing.\n */\n-char *enter_repo(char *path, int strict)\n+const char *enter_repo(const char *path, int strict)\n {\n \tstatic char used_path[PATH_MAX];\n \tstatic char validated_path[PATH_MAX];\n@@ -297,14 +297,16 @@ char *enter_repo(char *path, int strict)\n \t\t};\n \t\tint len = strlen(path);\n \t\tint i;\n-\t\twhile ((1 < len) && (path[len-1] == '/')) {\n-\t\t\tpath[len-1] = 0;\n+\t\twhile ((1 < len) && (path[len-1] == '/'))\n \t\t\tlen--;\n-\t\t}\n+\n \t\tif (PATH_MAX <= len)\n \t\t\treturn NULL;\n-\t\tif (path[0] == '~') {\n-\t\t\tchar *newpath = expand_user_path(path);\n+\t\tstrncpy(used_path, path, len); used_path[len] = 0 ;\n+\t\tstrcpy(validated_path, used_path);\n+\n+\t\tif (used_path[0] == '~') {\n+\t\t\tchar *newpath = expand_user_path(used_path);\n \t\t\tif (!newpath || (PATH_MAX - 10 < strlen(newpath))) {\n \t\t\t\tfree(newpath);\n \t\t\t\treturn NULL;\n@@ -316,24 +318,18 @@ char *enter_repo(char *path, int strict)\n \t\t\t * anyway.\n \t\t\t */\n \t\t\tstrcpy(used_path, newpath); free(newpath);\n-\t\t\tstrcpy(validated_path, path);\n-\t\t\tpath = used_path;\n \t\t}\n \t\telse if (PATH_MAX - 10 < len)\n \t\t\treturn NULL;\n-\t\telse {\n-\t\t\tpath = strcpy(used_path, path);\n-\t\t\tstrcpy(validated_path, path);\n-\t\t}\n-\t\tlen = strlen(path);\n+\t\tlen = strlen(used_path);\n \t\tfor (i = 0; suffix[i]; i++) {\n-\t\t\tstrcpy(path + len, suffix[i]);\n-\t\t\tif (!access(path, F_OK)) {\n+\t\t\tstrcpy(used_path + len, suffix[i]);\n+\t\t\tif (!access(used_path, F_OK)) {\n \t\t\t\tstrcat(validated_path, suffix[i]);\n \t\t\t\tbreak;\n \t\t\t}\n \t\t}\n-\t\tif (!suffix[i] || chdir(path))\n+\t\tif (!suffix[i] || chdir(used_path))\n \t\t\treturn NULL;\n \t\tpath = validated_path;\n \t}\n-- \n1.7.7.503.g26392.dirty\n"},{"id":"177018","messageId":"CABPQNSZFo7u5GBvZzhKDzR5YGYwd0j8-Mb080QymDu9ayV6uDA@mail.gmail.com","threadId":"28530","inReplyTo":"CABURp0qDsxHwsuyvB6-KvKPrKuUT0-Fpr730TD_TxxFY7fotpA@mail.gmail.com","subject":"Re: [PATCH v3 3/4] enter_repo: do not modify input","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-10-06T13:06:04Z","receivedAt":"2011-10-06T13:06:04Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, Oct 4, 2011 at 7:55 PM, Phil Hord wrote:\n> On Thu, Sep 29, 2011 at 4:59 PM, Erik Faye-Lund wrote:\n>> entr_repo(..., 0) currently modifies the input to strip away\n>> trailing slashes. This means that we some times need to copy the\n>> input to keep the original.\n>\n> I'm also modifying enter_repo() so I'm looking a bit closer at this patch now.\n>\n>> Change it to unconditionally copy it into the used_path buffer so\n>> we can safely use the input without having to copy it.\n>>\n>> Signed-off-by: Erik Faye-Lund \n>> ---\n> [...]\n>>  */\n>> -char *enter_repo(char *path, int strict)\n>> +const char *enter_repo(const char *path, int strict)\n>>  {\n>>        static char used_path[PATH_MAX];\n>>        static char validated_path[PATH_MAX];\n>> @@ -297,14 +297,15 @@ char *enter_repo(char *path, int strict)\n>>                };\n>>                int len = strlen(path);\n>>                int i;\n>> -               while ((1 < len) && (path[len-1] == '/')) {\n>> -                       path[len-1] = 0;\n>> +               while ((1 < len) && (path[len-1] == '/'))\n>>                        len--;\n>> -               }\n>> +\n>>                if (PATH_MAX <= len)\n>>                        return NULL;\n>> -               if (path[0] == '~') {\n>> -                       char *newpath = expand_user_path(path);\n>> +               strncpy(used_path, path, len);\n>\n> When len < strlen(path), this will will leave used_path unterminated.\n>\n\nGood catch, thanks!\n\n>> +\n>> +               if (used_path[0] == '~') {\n>> +                       char *newpath = expand_user_path(used_path);\n>>                        if (!newpath || (PATH_MAX - 10 < strlen(newpath))) {\n>>                                free(newpath);\n>>                                return NULL;\n>> @@ -316,24 +317,21 @@ char *enter_repo(char *path, int strict)\n>>                         * anyway.\n>>                         */\n>>                        strcpy(used_path, newpath); free(newpath);\n>> -                       strcpy(validated_path, path);\n>> -                       path = used_path;\n>> +                       strcpy(validated_path, used_path);\n>\n> The point of 'validated_path' is to keep the original unmolested,\n> unexpanded path string (plus DWIM suffix), but here you've just\n> replaced validated_path with a copy of the expanded_user_path.  On the\n> other hand, we seem always to strcpy(validated_path , path), so we\n> might as well get that done up-front.\n\nYeah, that's probably better.\n"}]}