{"thread":{"id":"16118","subject":"[PATCH] connect.c: add a way for git-daemon to pass an error back to client","startedAt":"2008-11-01T01:59:46Z","lastAt":"2008-11-01T22:48:12Z","messageCount":13,"participants":["Tom Preston-Werner","Johannes Schindelin","Nicolas Pitre","Junio C Hamano","Andreas Ericsson","Alex Riesen","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"94525","messageId":"b97024a40810311859t2e5a6102u31ad4480e7c75c03@mail.gmail.com","threadId":"16118","inReplyTo":null,"subject":"[PATCH] connect.c: add a way for git-daemon to pass an error back to client","fromName":"Tom Preston-Werner","fromEmail":"tom@github.com","sentAt":"2008-11-01T01:59:46Z","receivedAt":"2008-11-01T01:59:46Z","isPatch":true,"sender":{"key":"tom@github.com","avatar":"https://gravatar.com/avatar/e38396c4b4d7977fac079a61a1b22c7eca553a9aac50efb1862daac22c65fc58?d=mp&s=160"},"body":"The current behavior of git-daemon is to simply close the connection on\nany error condition. This leaves the client without any information as\nto the cause of the failed fetch/push/etc.\n\nThis patch allows get_remote_heads to accept a line prefixed with \"ERR\"\nthat it can display to the user in an informative fashion. Once clients\ncan understand this ERR line, git-daemon can be made to properly report\n\"repository not found\", \"permission denied\", or other errors.\n\nExample\n\nS: ERR No matching repository.\nC: fatal: remote error: No matching repository.\n\nSigned-off-by: Tom Preston-Werner \n---\n connect.c | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex 0c50d0a..3af91d6 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -70,6 +70,9 @@ struct ref **get_remote_heads(int in, struct ref **list,\n \t\tif (buffer[len-1] == '\\n')\n \t\t\tbuffer[--len] = 0;\n\n+\t\tif (len > 4 && !memcmp(\"ERR\", buffer, 3))\n+\t\t\tdie(\"remote error: %s\", buffer + 4);\n+\n \t\tif (len < 42 || get_sha1_hex(buffer, old_sha1) || buffer[40] != ' ')\n \t\t\tdie(\"protocol error: expected sha/ref, got '%s'\", buffer);\n \t\tname = buffer + 41;\n-- \n1.6.0.2\n"},{"id":"94527","messageId":"b97024a40810311918sca49d02g2dac46ca73e0fd87@mail.gmail.com","threadId":"16118","inReplyTo":"alpine.DEB.1.00.0811010316340.22125@pacific.mpi-cbg.de.mpi-cbg.de","subject":"Re: [PATCH] connect.c: add a way for git-daemon to pass an error back to client","fromName":"Tom Preston-Werner","fromEmail":"tom@github.com","sentAt":"2008-11-01T02:18:28Z","receivedAt":"2008-11-01T02:18:28Z","isPatch":true,"sender":{"key":"tom@github.com","avatar":"https://gravatar.com/avatar/e38396c4b4d7977fac079a61a1b22c7eca553a9aac50efb1862daac22c65fc58?d=mp&s=160"},"body":"On Fri, Oct 31, 2008 at 7:19 PM, Johannes Schindelin\n wrote:\n> The cute trick with this is that even older clients would be able to\n> display the error, albeit with a sort of funny message:\n>\n> protocol error: expected sha/ref, got 'No matching repository.'\n\nThis is exactly what GitHub does right now with our custom Erlang\ngit-daemon. Try doing:\n\n$ git clone git://github.com/mojombo/foo\n\nand you'll get:\n\nfatal: protocol error: expected sha/ref, got '\n*********'\n\nNo matching repositories found.\n\n*********'\n\nThis has cut down tremendously on the number of support requests we\nget from new users. It would be nice to cut out the clutter from the\nerror message though.\n\nTom\n"},{"id":"94526","messageId":"alpine.DEB.1.00.0811010316340.22125@pacific.mpi-cbg.de.mpi-cbg.de","threadId":"16118","inReplyTo":"b97024a40810311859t2e5a6102u31ad4480e7c75c03@mail.gmail.com","subject":"Re: [PATCH] connect.c: add a way for git-daemon to pass an error back to client","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-11-01T02:19:32Z","receivedAt":"2008-11-01T02:19:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 31 Oct 2008, Tom Preston-Werner wrote:\n\n> The current behavior of git-daemon is to simply close the connection on\n> any error condition. This leaves the client without any information as\n> to the cause of the failed fetch/push/etc.\n> \n> This patch allows get_remote_heads to accept a line prefixed with \"ERR\"\n> that it can display to the user in an informative fashion. Once clients\n> can understand this ERR line, git-daemon can be made to properly report\n> \"repository not found\", \"permission denied\", or other errors.\n> \n> Example\n> \n> S: ERR No matching repository.\n> C: fatal: remote error: No matching repository.\n\nMakes sense to me.\n\nThe cute trick with this is that even older clients would be able to \ndisplay the error, albeit with a sort of funny message:\n\n\tprotocol error: expected sha/ref, got 'No matching repository.'\n\nCiao,\nDscho\n"},{"id":"94529","messageId":"alpine.LFD.2.00.0810312218300.13034@xanadu.home","threadId":"16118","inReplyTo":"alpine.DEB.1.00.0811010316340.22125@pacific.mpi-cbg.de.mpi-cbg.de","subject":"Re: [PATCH] connect.c: add a way for git-daemon to pass an error back to client","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-11-01T02:20:35Z","receivedAt":"2008-11-01T02:20:35Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Sat, 1 Nov 2008, Johannes Schindelin wrote:\n\n> Hi,\n> \n> On Fri, 31 Oct 2008, Tom Preston-Werner wrote:\n> \n> > The current behavior of git-daemon is to simply close the connection on\n> > any error condition. This leaves the client without any information as\n> > to the cause of the failed fetch/push/etc.\n> > \n> > This patch allows get_remote_heads to accept a line prefixed with \"ERR\"\n> > that it can display to the user in an informative fashion. Once clients\n> > can understand this ERR line, git-daemon can be made to properly report\n> > \"repository not found\", \"permission denied\", or other errors.\n> > \n> > Example\n> > \n> > S: ERR No matching repository.\n> > C: fatal: remote error: No matching repository.\n> \n> Makes sense to me.\n\nNote that this behavior of not returning any reason for failure was \nargued to be a security feature in the past, by Linus I think.\n\n\nNicolas\n"},{"id":"94530","messageId":"alpine.DEB.1.00.0811010334010.22125@pacific.mpi-cbg.de.mpi-cbg.de","threadId":"16118","inReplyTo":"alpine.LFD.2.00.0810312218300.13034@xanadu.home","subject":"Re: [PATCH] connect.c: add a way for git-daemon to pass an error back to client","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-11-01T02:35:28Z","receivedAt":"2008-11-01T02:35:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 31 Oct 2008, Nicolas Pitre wrote:\n\n> On Sat, 1 Nov 2008, Johannes Schindelin wrote:\n> \n> > On Fri, 31 Oct 2008, Tom Preston-Werner wrote:\n> > \n> > > The current behavior of git-daemon is to simply close the connection \n> > > on any error condition. This leaves the client without any \n> > > information as to the cause of the failed fetch/push/etc.\n> > > \n> > > This patch allows get_remote_heads to accept a line prefixed with \n> > > \"ERR\" that it can display to the user in an informative fashion. \n> > > Once clients can understand this ERR line, git-daemon can be made to \n> > > properly report \"repository not found\", \"permission denied\", or \n> > > other errors.\n> > > \n> > > Example\n> > > \n> > > S: ERR No matching repository.\n> > > C: fatal: remote error: No matching repository.\n> > \n> > Makes sense to me.\n> \n> Note that this behavior of not returning any reason for failure was \n> argued to be a security feature in the past, by Linus I think.\n\nYes. And it might still be considered one. You do not need to patch \ngit-daemon to use that facility (note that Tom's patch was only for the \nclient side).\n\nBut for hosting sites such as repo.or.cz or GitHub, that security feature \njust does not make sense, but it makes for support requests that could be \nresolved better with a proper error message.\n\nCiao,\nDscho\n"},{"id":"94532","messageId":"b97024a40810312035v5416b578v51b5bed528ca8d39@mail.gmail.com","threadId":"16118","inReplyTo":"alpine.DEB.1.00.0811010334010.22125@pacific.mpi-cbg.de.mpi-cbg.de","subject":"Re: [PATCH] connect.c: add a way for git-daemon to pass an error back to client","fromName":"Tom Preston-Werner","fromEmail":"tom@github.com","sentAt":"2008-11-01T03:35:20Z","receivedAt":"2008-11-01T03:35:20Z","isPatch":true,"sender":{"key":"tom@github.com","avatar":"https://gravatar.com/avatar/e38396c4b4d7977fac079a61a1b22c7eca553a9aac50efb1862daac22c65fc58?d=mp&s=160"},"body":"On Fri, Oct 31, 2008 at 7:35 PM, Johannes Schindelin\n wrote:\n> Hi,\n>\n> On Fri, 31 Oct 2008, Nicolas Pitre wrote:\n>\n>> On Sat, 1 Nov 2008, Johannes Schindelin wrote:\n>>\n>> > On Fri, 31 Oct 2008, Tom Preston-Werner wrote:\n>> >\n>> > > The current behavior of git-daemon is to simply close the connection\n>> > > on any error condition. This leaves the client without any\n>> > > information as to the cause of the failed fetch/push/etc.\n>> > >\n>> > > This patch allows get_remote_heads to accept a line prefixed with\n>> > > \"ERR\" that it can display to the user in an informative fashion.\n>> > > Once clients can understand this ERR line, git-daemon can be made to\n>> > > properly report \"repository not found\", \"permission denied\", or\n>> > > other errors.\n>> > >\n>> > > Example\n>> > >\n>> > > S: ERR No matching repository.\n>> > > C: fatal: remote error: No matching repository.\n>> >\n>> > Makes sense to me.\n>>\n>> Note that this behavior of not returning any reason for failure was\n>> argued to be a security feature in the past, by Linus I think.\n>\n> Yes. And it might still be considered one. You do not need to patch\n> git-daemon to use that facility (note that Tom's patch was only for the\n> client side).\n>\n> But for hosting sites such as repo.or.cz or GitHub, that security feature\n> just does not make sense, but it makes for support requests that could be\n> resolved better with a proper error message.\n\nWe could also have the error messages sent back conditionally based on\na command line switch. I've begun porting the changes I made in our\nErlang git-daemon back to the C code, so maybe I'll give that a try.\nWe *definitely* need good error messages for GitHub and I see no\nsecurity risk in doing so.\n\nMaybe this is worth asking the question: does anybody use git-daemon\nfor private code? If so, why are they not using SSH instead? And in\nthat case, how are informative error messages a security risk?\n\nTom\n"},{"id":"94535","messageId":"7vy7043sy9.fsf@gitster.siamese.dyndns.org","threadId":"16118","inReplyTo":"b97024a40810311859t2e5a6102u31ad4480e7c75c03@mail.gmail.com","subject":"Re: [PATCH] connect.c: add a way for git-daemon to pass an error back to client","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-01T05:39:58Z","receivedAt":"2008-11-01T05:39:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Tom Preston-Werner\" writes:\n\n> Example\n>\n> S: ERR No matching repository.\n> C: fatal: remote error: No matching repository.\n\nI like what this tries to do.\n\nI briefly wondered if this should be restricted to the very first message\nfrom the other end, but I think it is not necessary. If the remote throws\na few valid looking \"SHA-1 SP refname\" lines and then said \"ERR\" (which\ncannot be the beginning of a valid SHA-1), we can safely and unambiguously\ndeclare that this is an error message from the remote end.\n\n> diff --git a/connect.c b/connect.c\n> index 0c50d0a..3af91d6 100644\n> --- a/connect.c\n> +++ b/connect.c\n> @@ -70,6 +70,9 @@ struct ref **get_remote_heads(int in, struct ref **list,\n> \t\tif (buffer[len-1] == '\\n')\n> \t\t\tbuffer[--len] = 0;\n>\n> +\t\tif (len > 4 && !memcmp(\"ERR\", buffer, 3))\n\nWould matching 4 bytes \"ERR \" here an improvement? You are expecting\nbuffer+4 is where the message begins in die() anyway, and otherwise you\nwould show the message without \"N\" if you got \"ERRNo matching repo\".\n\n> +\t\t\tdie(\"remote error: %s\", buffer + 4);\n> +\n\nIt was very considerate that you did not say \"server error\" in the error\nmessage. This code is shared between both the fetch side and the push\nside.\n"},{"id":"94536","messageId":"b97024a40810312329o53e37fd5td82aa69634ff1e6b@mail.gmail.com","threadId":"16118","inReplyTo":"7vy7043sy9.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] connect.c: add a way for git-daemon to pass an error back to client","fromName":"Tom Preston-Werner","fromEmail":"tom@github.com","sentAt":"2008-11-01T06:29:59Z","receivedAt":"2008-11-01T06:29:59Z","isPatch":true,"sender":{"key":"tom@github.com","avatar":"https://gravatar.com/avatar/e38396c4b4d7977fac079a61a1b22c7eca553a9aac50efb1862daac22c65fc58?d=mp&s=160"},"body":"On Fri, Oct 31, 2008 at 10:39 PM, Junio C Hamano wrote:\n> \"Tom Preston-Werner\" writes:\n>\n>> Example\n>>\n>> S: ERR No matching repository.\n>> C: fatal: remote error: No matching repository.\n>\n> I like what this tries to do.\n>\n> I briefly wondered if this should be restricted to the very first message\n> from the other end, but I think it is not necessary. If the remote throws\n> a few valid looking \"SHA-1 SP refname\" lines and then said \"ERR\" (which\n> cannot be the beginning of a valid SHA-1), we can safely and unambiguously\n> declare that this is an error message from the remote end.\n>\n>> diff --git a/connect.c b/connect.c\n>> index 0c50d0a..3af91d6 100644\n>> --- a/connect.c\n>> +++ b/connect.c\n>> @@ -70,6 +70,9 @@ struct ref **get_remote_heads(int in, struct ref **list,\n>> if (buffer[len-1] == '\\n')\n>> buffer[--len] = 0;\n>>\n>> + if (len > 4 && !memcmp(\"ERR\", buffer, 3))\n>\n> Would matching 4 bytes \"ERR \" here an improvement? You are expecting\n> buffer+4 is where the message begins in die() anyway, and otherwise you\n> would show the message without \"N\" if you got \"ERRNo matching repo\".\n\nI saw several methods of testing for a specific prefix in connect.c.\nLooking more closely at the source, the closest similar call is\nactually the test for ACK:\n\n\tif (!prefixcmp(line, \"ACK \")) {\n\t\tif (!get_sha1_hex(line+4, result_sha1)) {\n\t\t\tif (strstr(line+45, \"continue\"))\n\t\t\t\treturn 2;\n\t\t\treturn 1;\n\t\t}\n\t}\n\nExplicitly testing for \"ERR \" (including the space) does seem like the\nmore correct thing to do. Would you like me to resubmit a modified\npatch that uses prefixcmp()?\n\nTom\n"},{"id":"94551","messageId":"490C3DE2.3010808@op5.se","threadId":"16118","inReplyTo":"alpine.LFD.2.00.0810312218300.13034@xanadu.home","subject":"Re: [PATCH] connect.c: add a way for git-daemon to pass an error back to client","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2008-11-01T11:30:42Z","receivedAt":"2008-11-01T11:30:42Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Nicolas Pitre wrote:\n> On Sat, 1 Nov 2008, Johannes Schindelin wrote:\n> \n>> Hi,\n>>\n>> On Fri, 31 Oct 2008, Tom Preston-Werner wrote:\n>>\n>>> The current behavior of git-daemon is to simply close the connection on\n>>> any error condition. This leaves the client without any information as\n>>> to the cause of the failed fetch/push/etc.\n>>>\n>>> This patch allows get_remote_heads to accept a line prefixed with \"ERR\"\n>>> that it can display to the user in an informative fashion. Once clients\n>>> can understand this ERR line, git-daemon can be made to properly report\n>>> \"repository not found\", \"permission denied\", or other errors.\n>>>\n>>> Example\n>>>\n>>> S: ERR No matching repository.\n>>> C: fatal: remote error: No matching repository.\n>> Makes sense to me.\n> \n> Note that this behavior of not returning any reason for failure was \n> argued to be a security feature in the past, by Linus I think.\n> \n\nBy me actually. I wrote the patch for it. Showing \"no matching repository\"\nmeans git-daemon can be used to disclose information about the remote\nserver's filesystem layout. While I understand that it's sometimes a useful\nfeature, please don't ever enable it by default.\n\nThat said, I like this patch, as it only works client-side and just enables\nothers to write code that let's the daemon (if configured to do so) ship a\nmore informative error message.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"94552","messageId":"490C3ECC.1000301@op5.se","threadId":"16118","inReplyTo":"b97024a40810312035v5416b578v51b5bed528ca8d39@mail.gmail.com","subject":"Re: [PATCH] connect.c: add a way for git-daemon to pass an error back to client","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2008-11-01T11:34:36Z","receivedAt":"2008-11-01T11:34:36Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Tom Preston-Werner wrote:\n> On Fri, Oct 31, 2008 at 7:35 PM, Johannes Schindelin\n> wrote:\n>> Hi,\n>>\n>> On Fri, 31 Oct 2008, Nicolas Pitre wrote:\n>>\n>>> On Sat, 1 Nov 2008, Johannes Schindelin wrote:\n>>>\n>>>> On Fri, 31 Oct 2008, Tom Preston-Werner wrote:\n>>>>\n>>>>> The current behavior of git-daemon is to simply close the connection\n>>>>> on any error condition. This leaves the client without any\n>>>>> information as to the cause of the failed fetch/push/etc.\n>>>>>\n>>>>> This patch allows get_remote_heads to accept a line prefixed with\n>>>>> \"ERR\" that it can display to the user in an informative fashion.\n>>>>> Once clients can understand this ERR line, git-daemon can be made to\n>>>>> properly report \"repository not found\", \"permission denied\", or\n>>>>> other errors.\n>>>>>\n>>>>> Example\n>>>>>\n>>>>> S: ERR No matching repository.\n>>>>> C: fatal: remote error: No matching repository.\n>>>> Makes sense to me.\n>>> Note that this behavior of not returning any reason for failure was\n>>> argued to be a security feature in the past, by Linus I think.\n>> Yes. And it might still be considered one. You do not need to patch\n>> git-daemon to use that facility (note that Tom's patch was only for the\n>> client side).\n>>\n>> But for hosting sites such as repo.or.cz or GitHub, that security feature\n>> just does not make sense, but it makes for support requests that could be\n>> resolved better with a proper error message.\n> \n> We could also have the error messages sent back conditionally based on\n> a command line switch. I've begun porting the changes I made in our\n> Erlang git-daemon back to the C code, so maybe I'll give that a try.\n> We *definitely* need good error messages for GitHub and I see no\n> security risk in doing so.\n> \n> Maybe this is worth asking the question: does anybody use git-daemon\n> for private code? If so, why are they not using SSH instead? And in\n> that case, how are informative error messages a security risk?\n> \n\nBecause it can potentially allow attackers to gain a lot of information\nabout your system. For instance, if you have /var/lib/rpm on your system,\nyou're likely running an RPM-based installation. Otoh, if you have\n/usr/bin/apt-get, you're most likely running a dpkg-based one. Such info\nis vital for the attacker to know what version of a certain server-program\nyou're using, and can then be used to scan the very helpful world wide web\nfor security issues concerning your exact distribution.\n\nI'm not saying that's possible with git-daemon now (although I haven't tried),\nbut if, one day, a git-daemon were to accept a path such as ../../../, we'd\nbe in real trouble, and an attacker would have no problems what so ever\ndoing educated guesses on exactly what kind of software is running on your\nserver.\n\nSo, please don't enable any such feature by default. Bury it somewhere deep\nin documentation so that users do not enable it by default, or attach a big\nfat warning to the docs mentioning it.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"94561","messageId":"20081101143954.GB7157@steel.home","threadId":"16118","inReplyTo":"b97024a40810312035v5416b578v51b5bed528ca8d39@mail.gmail.com","subject":"Re: [PATCH] connect.c: add a way for git-daemon to pass an error back to client","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2008-11-01T14:39:54Z","receivedAt":"2008-11-01T14:39:54Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Tom Preston-Werner, Sat, Nov 01, 2008 04:35:20 +0100:\n> Maybe this is worth asking the question: does anybody use git-daemon\n> for private code? If so, why are they not using SSH instead? And in\n> that case, how are informative error messages a security risk?\n\nYes. I use both in my private network, with only ssh open to the\ninternet. git-daemon is smaller and faster (started from inetd). And\nI'm absolutely sure wont ever accidentally push something in the\nmirrored repos.\n\nI never had the error reporting problem in this setup, though. It is a\nfully controled environment and I can just look in syslog.\n\nI support the original reason for not doing the errors, BTW. It cannot\nbe on by default.\n\nHeh, try the patch for your private repos and private repos of your\nemployer, who can sack you for exposing confidential information, and\nopen them to internet. Than come back and tell us how safe you feel :)\n"},{"id":"94572","messageId":"20081101181035.GB14706@spearce.org","threadId":"16118","inReplyTo":"b97024a40810312329o53e37fd5td82aa69634ff1e6b@mail.gmail.com","subject":"Re: [PATCH] connect.c: add a way for git-daemon to pass an error back to client","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-11-01T18:10:35Z","receivedAt":"2008-11-01T18:10:35Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Tom Preston-Werner wrote:\n> \n> I saw several methods of testing for a specific prefix in connect.c.\n> Looking more closely at the source, the closest similar call is\n> actually the test for ACK:\n> \n> \tif (!prefixcmp(line, \"ACK \")) {\n> \t\tif (!get_sha1_hex(line+4, result_sha1)) {\n> \t\t\tif (strstr(line+45, \"continue\"))\n> \t\t\t\treturn 2;\n> \t\t\treturn 1;\n> \t\t}\n> \t}\n> \n> Explicitly testing for \"ERR \" (including the space) does seem like the\n> more correct thing to do. Would you like me to resubmit a modified\n> patch that uses prefixcmp()?\n\nYes, I think that is what Junio was hinting at. The pattern above\nis much more typical in Git sources, so keeping the new \"ERR \"\ncheck consistent would be appreciated.\n\n-- \nShawn.\n"},{"id":"94600","messageId":"7vej1v3vwz.fsf@gitster.siamese.dyndns.org","threadId":"16118","inReplyTo":"b97024a40810312329o53e37fd5td82aa69634ff1e6b@mail.gmail.com","subject":"Re: [PATCH] connect.c: add a way for git-daemon to pass an error back to client","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-01T22:48:12Z","receivedAt":"2008-11-01T22:48:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Tom Preston-Werner\" writes:\n\n> Explicitly testing for \"ERR \" (including the space) does seem like the\n> more correct thing to do. Would you like me to resubmit a modified\n> patch that uses prefixcmp()?\n\nNo need for resubmission to change something minor like that. Will queue\nwith a fixup.\n\nThanks.\n"}]}