{"thread":{"id":"28745","subject":"[PATCH 1/2] credential-cache--daemon.c: Don't leave stale socket on --exit","startedAt":"2011-10-22T17:24:51Z","lastAt":"2011-10-27T17:42:08Z","messageCount":4,"participants":["Ramsay Jones","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"178165","messageId":"4EA2FC63.9060602@ramsay1.demon.co.uk","threadId":"28745","inReplyTo":null,"subject":"[PATCH 1/2] credential-cache--daemon.c: Don't leave stale socket on --exit","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2011-10-22T17:24:51Z","receivedAt":"2011-10-22T17:24:51Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nSigned-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>\n---\n credential-cache--daemon.c |   23 ++++++++++++-----------\n 1 files changed, 12 insertions(+), 11 deletions(-)\n\ndiff --git a/credential-cache--daemon.c b/credential-cache--daemon.c\nindex 128c5ce..ee2c15a 100644\n--- a/credential-cache--daemon.c\n+++ b/credential-cache--daemon.c\n@@ -137,22 +137,22 @@ static int read_credential_request(FILE *fh, struct credential *c,\n \treturn 0;\n }\n \n-static void serve_one_client(FILE *in, FILE *out)\n+static int serve_one_client(FILE *in, FILE *out)\n {\n \tstruct credential c = { NULL };\n \tint timeout = -1;\n \tchar *action = NULL;\n \n \tif (read_credential_request(in, &c, &action, &timeout) < 0)\n-\t\treturn;\n+\t\treturn 1;\n \n \tif (!action) {\n \t\twarning(\"cache client didn't specify an action\");\n-\t\treturn;\n+\t\treturn 1;\n \t}\n \n \tif (!strcmp(action, \"exit\"))\n-\t\texit(0);\n+\t\treturn 0;\n \n \tif (!strcmp(action, \"get\")) {\n \t\tstruct credential_cache_entry *e = lookup_credential(&c);\n@@ -160,27 +160,27 @@ static void serve_one_client(FILE *in, FILE *out)\n \t\t\tfprintf(out, \"username=%s\\n\", e->item.username);\n \t\t\tfprintf(out, \"password=%s\\n\", e->item.password);\n \t\t}\n-\t\treturn;\n+\t\treturn 1;\n \t}\n \n \tif (!strcmp(action, \"erase\")) {\n \t\tremove_credential(&c);\n-\t\treturn;\n+\t\treturn 1;\n \t}\n \n \tif (!strcmp(action, \"store\")) {\n \t\tif (timeout < 0) {\n \t\t\twarning(\"cache client didn't specify a timeout\");\n-\t\t\treturn;\n+\t\t\treturn 1;\n \t\t}\n \n \t\tremove_credential(&c);\n \t\tcache_credential(&c, timeout);\n-\t\treturn;\n+\t\treturn 1;\n \t}\n \n \twarning(\"cache client sent unknown action: %s\", action);\n-\treturn;\n+\treturn 1;\n }\n \n static int serve_cache_loop(int fd)\n@@ -201,7 +201,7 @@ static int serve_cache_loop(int fd)\n \t}\n \n \tif (pfd.revents & POLLIN) {\n-\t\tint client, client2;\n+\t\tint client, client2, ret;\n \t\tFILE *in, *out;\n \n \t\tclient = accept(fd, NULL, NULL);\n@@ -218,9 +218,10 @@ static int serve_cache_loop(int fd)\n \n \t\tin = xfdopen(client, \"r\");\n \t\tout = xfdopen(client2, \"w\");\n-\t\tserve_one_client(in, out);\n+\t\tret = serve_one_client(in, out);\n \t\tfclose(in);\n \t\tfclose(out);\n+\t\treturn ret;\n \t}\n \treturn 1;\n }\n-- \n1.7.7\n"},{"id":"178179","messageId":"20111022191711.GC1785@sigill.intra.peff.net","threadId":"28745","inReplyTo":"4EA2FC63.9060602@ramsay1.demon.co.uk","subject":"Re: [PATCH 1/2] credential-cache--daemon.c: Don't leave stale socket on --exit","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-22T19:17:14Z","receivedAt":"2011-10-22T19:17:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Oct 22, 2011 at 06:24:51PM +0100, Ramsay Jones wrote:\n\n> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>\n> ---\n>  credential-cache--daemon.c |   23 ++++++++++++-----------\n>  1 files changed, 12 insertions(+), 11 deletions(-)\n\nLooks sane, and I'll probably squash it in. Alternatively, we could also\nset a signal/exit handler to clean up our socket when we die. That would\nalso cover the error exit cases.\n\nIn either case, I think we need to handle stale sockets better. They\nwill happen eventually due to power loss or kill -9, anyway.\n\n-Peff\n"},{"id":"178361","messageId":"4EA9950A.9020208@ramsay1.demon.co.uk","threadId":"28745","inReplyTo":"20111022191711.GC1785@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] credential-cache--daemon.c: Don't leave stale socket on --exit","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2011-10-27T17:29:46Z","receivedAt":"2011-10-27T17:29:46Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Jeff King wrote:\n> On Sat, Oct 22, 2011 at 06:24:51PM +0100, Ramsay Jones wrote:\n> \n>> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>\n>> ---\n>>  credential-cache--daemon.c |   23 ++++++++++++-----------\n>>  1 files changed, 12 insertions(+), 11 deletions(-)\n> \n> Looks sane, and I'll probably squash it in. Alternatively, we could also\n> set a signal/exit handler to clean up our socket when we die. That would\n> also cover the error exit cases.\n\nI considered this, *very* briefly, but decided it wasn't worth the effort\nor complexity.\n\n> In either case, I think we need to handle stale sockets better. They\n> will happen eventually due to power loss or kill -9, anyway.\n\nIndeed, hence patch #2. ;-)\n\nI suspect that, given the current code, the *vast* majority of stale sockets\nwould be (somewhat gratuitously) created by the --exit action - hence this\npatch. :-P\n\nATB,\nRamsay Jones\n"},{"id":"178365","messageId":"20111027174208.GB1967@sigill.intra.peff.net","threadId":"28745","inReplyTo":"4EA9950A.9020208@ramsay1.demon.co.uk","subject":"Re: [PATCH 1/2] credential-cache--daemon.c: Don't leave stale socket on --exit","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-27T17:42:08Z","receivedAt":"2011-10-27T17:42:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 27, 2011 at 06:29:46PM +0100, Ramsay Jones wrote:\n\n> Jeff King wrote:\n> > On Sat, Oct 22, 2011 at 06:24:51PM +0100, Ramsay Jones wrote:\n> > \n> >> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>\n> >> ---\n> >>  credential-cache--daemon.c |   23 ++++++++++++-----------\n> >>  1 files changed, 12 insertions(+), 11 deletions(-)\n> > \n> > Looks sane, and I'll probably squash it in. Alternatively, we could also\n> > set a signal/exit handler to clean up our socket when we die. That would\n> > also cover the error exit cases.\n> \n> I considered this, *very* briefly, but decided it wasn't worth the effort\n> or complexity.\n\nActually, I think with the sigchain code, it would only end up as a few\nlines. I'll probably take a look when I re-roll.\n\nThanks for your patches.\n\n-Peff\n"}]}