# [PATCH 1/2] credential-cache--daemon.c: Don't leave stale socket on --exit 4 messages from 2011-10-22 to 2011-10-27. Participants: Ramsay Jones, Jeff King. Thread: https://gitlist.dev/t/28745 ## Ramsay Jones, 2011-10-22 17:24 Subject: [PATCH 1/2] credential-cache--daemon.c: Don't leave stale socket on --exit Message-ID: <4EA2FC63.9060602@ramsay1.demon.co.uk> URL: https://gitlist.dev/e/4EA2FC63.9060602%40ramsay1.demon.co.uk ``` Signed-off-by: Ramsay Jones --- credential-cache--daemon.c | 23 ++++++++++++----------- 1 files changed, 12 insertions(+), 11 deletions(-) diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c index 128c5ce..ee2c15a 100644 --- a/credential-cache--daemon.c +++ b/credential-cache--daemon.c @@ -137,22 +137,22 @@ static int read_credential_request(FILE *fh, struct credential *c, return 0; } -static void serve_one_client(FILE *in, FILE *out) +static int serve_one_client(FILE *in, FILE *out) { struct credential c = { NULL }; int timeout = -1; char *action = NULL; if (read_credential_request(in, &c, &action, &timeout) < 0) - return; + return 1; if (!action) { warning("cache client didn't specify an action"); - return; + return 1; } if (!strcmp(action, "exit")) - exit(0); + return 0; if (!strcmp(action, "get")) { struct credential_cache_entry *e = lookup_credential(&c); @@ -160,27 +160,27 @@ static void serve_one_client(FILE *in, FILE *out) fprintf(out, "username=%s\n", e->item.username); fprintf(out, "password=%s\n", e->item.password); } - return; + return 1; } if (!strcmp(action, "erase")) { remove_credential(&c); - return; + return 1; } if (!strcmp(action, "store")) { if (timeout < 0) { warning("cache client didn't specify a timeout"); - return; + return 1; } remove_credential(&c); cache_credential(&c, timeout); - return; + return 1; } warning("cache client sent unknown action: %s", action); - return; + return 1; } static int serve_cache_loop(int fd) @@ -201,7 +201,7 @@ static int serve_cache_loop(int fd) } if (pfd.revents & POLLIN) { - int client, client2; + int client, client2, ret; FILE *in, *out; client = accept(fd, NULL, NULL); @@ -218,9 +218,10 @@ static int serve_cache_loop(int fd) in = xfdopen(client, "r"); out = xfdopen(client2, "w"); - serve_one_client(in, out); + ret = serve_one_client(in, out); fclose(in); fclose(out); + return ret; } return 1; } -- 1.7.7 ``` ## Jeff King, 2011-10-22 19:17 Subject: Re: [PATCH 1/2] credential-cache--daemon.c: Don't leave stale socket on --exit Message-ID: <20111022191711.GC1785@sigill.intra.peff.net> URL: https://gitlist.dev/e/20111022191711.GC1785%40sigill.intra.peff.net In-Reply-To: <4EA2FC63.9060602@ramsay1.demon.co.uk> ``` On Sat, Oct 22, 2011 at 06:24:51PM +0100, Ramsay Jones wrote: > Signed-off-by: Ramsay Jones > --- > credential-cache--daemon.c | 23 ++++++++++++----------- > 1 files changed, 12 insertions(+), 11 deletions(-) Looks sane, and I'll probably squash it in. Alternatively, we could also set a signal/exit handler to clean up our socket when we die. That would also cover the error exit cases. In either case, I think we need to handle stale sockets better. They will happen eventually due to power loss or kill -9, anyway. -Peff ``` ## Ramsay Jones, 2011-10-27 17:29 Subject: Re: [PATCH 1/2] credential-cache--daemon.c: Don't leave stale socket on --exit Message-ID: <4EA9950A.9020208@ramsay1.demon.co.uk> URL: https://gitlist.dev/e/4EA9950A.9020208%40ramsay1.demon.co.uk In-Reply-To: <20111022191711.GC1785@sigill.intra.peff.net> ``` Jeff King wrote: > On Sat, Oct 22, 2011 at 06:24:51PM +0100, Ramsay Jones wrote: > >> Signed-off-by: Ramsay Jones >> --- >> credential-cache--daemon.c | 23 ++++++++++++----------- >> 1 files changed, 12 insertions(+), 11 deletions(-) > > Looks sane, and I'll probably squash it in. Alternatively, we could also > set a signal/exit handler to clean up our socket when we die. That would > also cover the error exit cases. I considered this, *very* briefly, but decided it wasn't worth the effort or complexity. > In either case, I think we need to handle stale sockets better. They > will happen eventually due to power loss or kill -9, anyway. Indeed, hence patch #2. ;-) I suspect that, given the current code, the *vast* majority of stale sockets would be (somewhat gratuitously) created by the --exit action - hence this patch. :-P ATB, Ramsay Jones ``` ## Jeff King, 2011-10-27 17:42 Subject: Re: [PATCH 1/2] credential-cache--daemon.c: Don't leave stale socket on --exit Message-ID: <20111027174208.GB1967@sigill.intra.peff.net> URL: https://gitlist.dev/e/20111027174208.GB1967%40sigill.intra.peff.net In-Reply-To: <4EA9950A.9020208@ramsay1.demon.co.uk> ``` On Thu, Oct 27, 2011 at 06:29:46PM +0100, Ramsay Jones wrote: > Jeff King wrote: > > On Sat, Oct 22, 2011 at 06:24:51PM +0100, Ramsay Jones wrote: > > > >> Signed-off-by: Ramsay Jones > >> --- > >> credential-cache--daemon.c | 23 ++++++++++++----------- > >> 1 files changed, 12 insertions(+), 11 deletions(-) > > > > Looks sane, and I'll probably squash it in. Alternatively, we could also > > set a signal/exit handler to clean up our socket when we die. That would > > also cover the error exit cases. > > I considered this, *very* briefly, but decided it wasn't worth the effort > or complexity. Actually, I think with the sigchain code, it would only end up as a few lines. I'll probably take a look when I re-roll. Thanks for your patches. -Peff ```