Re: [PATCH v4 07/12] unix-socket: elimiate static unix_stream_socket() helper function
From: Junio C Hamano <hidden>
Date: 2021-03-04 00:24:02
"Jeff Hostetler via GitGitGadget" [off-list ref] writes:
quoted hunk
int unix_stream_connect(const char *path) { - int fd, saved_errno; + int fd = -1, saved_errno; struct sockaddr_un sa; struct unix_sockaddr_context ctx; if (unix_sockaddr_init(&sa, path, &ctx) < 0) return -1; - fd = unix_stream_socket(); + fd = socket(AF_UNIX, SOCK_STREAM, 0); + if (fd < 0) + goto fail; + if (connect(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0) goto fail; unix_sockaddr_cleanup(&ctx);@@ -87,15 +82,16 @@ int unix_stream_connect(const char *path) fail: saved_errno = errno; + if (fd != -1) + close(fd); unix_sockaddr_cleanup(&ctx); - close(fd); errno = saved_errno; return -1; }
So, the difference is that the caller must be prepared to see and handle error return from this function when creating socket fails, but existing callers must be prepared to handle error returns from this function for different reasons (e.g. we may successfully make a socket, but connect may fail) already anyway, so this should be a fairly safe thing to do. The sole caller send_request() in credential-cache.c will relay the error return back to do_cache() which cares what errno it got, and that code does seem to care what kind of error caused unix_stream_connect() to fail. And the new error case introduced by this patch won't result in ENOENT or ECONNREFUSED to cause the code to fall back to "if the thing is not running, let's try starting it and try again". OK.
int unix_stream_listen(const char *path)
{This one is simpler to vet its caller. It immediately dies upon any error return. Thanks.