Re: [PATCH 2/3] credential-cache: check for windows specific errors
From: Junio C Hamano <hidden>
Date: 2021-09-13 01:10:50
Carlo Marcelo Arenas Belón [off-list ref] writes:
Connect and reset errors aren't what will be expected by POSIX but are compatible with the ones used by WinSock. Alternatively a translation layer for read could be added (similar to the one provided by mingw-write()) to translate the odd EINVAL, into a more reasonable EPIPE, but this is more localized and still unlikely to cause confusion in other systems.
Are we sure EINVAL or ENETDOWN from outside the Windows world is tolerable after read_in_full() failure in this context? I'd rather see something like if (!r || (r < 0 && connection_closed(errno))) break; with a local helper function connection_closed() allowing EINVAL only with "#ifdef WINDOWS" to be safe. The same for "the socket is not yet open" codepath with another local helper function. Thanks.
quoted hunk
Signed-off-by: Carlo Marcelo Arenas Belón <redacted> --- builtin/credential-cache.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)diff --git a/builtin/credential-cache.c b/builtin/credential-cache.c index 76a6ba3722..b12a79ae01 100644 --- a/builtin/credential-cache.c +++ b/builtin/credential-cache.c@@ -28,7 +28,8 @@ static int send_request(const char *socket, const struct strbuf *out) int r; r = read_in_full(fd, in, sizeof(in)); - if (r == 0 || (r < 0 && errno == ECONNRESET)) + if (r == 0 || + (r < 0 && (errno == ECONNRESET || errno == EINVAL))) break; if (r < 0) die_errno("read error from cache daemon");@@ -75,7 +76,7 @@ static void do_cache(const char *socket, const char *action, int timeout, } if (send_request(socket, &buf) < 0) { - if (errno != ENOENT && errno != ECONNREFUSED) + if (errno != ENOENT && errno != ECONNREFUSED && errno != ENETDOWN) die_errno("unable to connect to cache daemon"); if (flags & FLAG_SPAWN) { spawn_daemon(socket);