git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCHv3 10/13] credentials: add "cache" helper

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Jan 11, 2012, 23:50 UTC
Message-ID
<20120111235009.GB30243@burratino>
In-Reply-To
<20120110175312.GA7289@sigill.intra.peff.net>
Jeff King wrote:
> Still, it would be slightly more robust. I wonder how portable fchdir
> is in practice (I guess we could always fall back to the getcwd code
> path). Do you want to prepare a patch on top?

I've been wanting to get around to doing something similar for setup.c for a while. I'm happy enough to forget about it for now. ;-)

Thanks again for the fix.  Here's another quick nit.
-- >8 --
Subject: unix-socket: do not let close() or chdir() clobber errno during cleanup

unix_stream_connect and unix_stream_listen return -1 on error, with errno set by the failing underlying call to allow the caller to write a useful diagnosis.

Unfortunately the error path involves a few system calls itself, such as close(), that can themselves touch errno.

This is not as worrisome as it might sound. If close() fails, this just means substituting one meaningful error message for another, which is perfectly fine. However, when the call _succeeds_, it is allowed to (and sometimes might) clobber errno along the way with some undefined value, so it is good higiene to save errno and restore it immediately before returning to the caller. Do so.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 unix-socket.c |   39 ++++++++++++++++++++++-----------------
 1 files changed, 22 insertions(+), 17 deletions(-)
diff --git a/unix-socket.c b/unix-socket.c
index 7d8bec61..01f119f9 100644
--- a/unix-socket.c
+++ b/unix-socket.c
@@ -73,25 +73,29 @@ static int unix_sockaddr_init(struct sockaddr_un *sa, const char *path,
 
 int unix_stream_connect(const char *path)
 {
-	int fd;
+	int fd, 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();
-	if (connect(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0) {
-		unix_sockaddr_cleanup(&ctx);
-		close(fd);
-		return -1;
-	}
+	if (connect(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0)
+		goto fail;
 	unix_sockaddr_cleanup(&ctx);
 	return fd;
+
+fail:
+	saved_errno = errno;
+	unix_sockaddr_cleanup(&ctx);
+	close(fd);
+	errno = saved_errno;
+	return -1;
 }
 
 int unix_stream_listen(const char *path)
 {
-	int fd;
+	int fd, saved_errno;
 	struct sockaddr_un sa;
 	struct unix_sockaddr_context ctx;
 
@@ -100,18 +104,19 @@ int unix_stream_listen(const char *path)
 	fd = unix_stream_socket();
 
 	unlink(path);
-	if (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0) {
-		unix_sockaddr_cleanup(&ctx);
-		close(fd);
-		return -1;
-	}
+	if (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0)
+		goto fail;
 
-	if (listen(fd, 5) < 0) {
-		unix_sockaddr_cleanup(&ctx);
-		close(fd);
-		return -1;
-	}
+	if (listen(fd, 5) < 0)
+		goto fail;
 
 	unix_sockaddr_cleanup(&ctx);
 	return fd;
+
+fail:
+	saved_errno = errno;
+	unix_sockaddr_cleanup(&ctx);
+	close(fd);
+	errno = saved_errno;
+	return -1;
 }
-- 
1.7.8.3
Previous: Jeff KingNext: Jeff King
Message 24 of 54 in “[PATCHv3 0/13] credential helpers”
  1. Jeff KingDec 10, 2011
  2. 01/13 test-lib: add test_config_global variantJeff King, Dec 10, 2011
  3. 02/13 t5550: fix typoJeff King, Dec 10, 2011
  4. 03/13 introduce credentials APIJeff King, Dec 10, 2011
  5. Jakub NarebskiDec 10, 2011
  6. Jeff KingDec 10, 2011
  7. 04/13 credential: add function for parsing url componentsJeff King, Dec 10, 2011
  8. 05/13 http: use credential API to get passwordsJeff King, Dec 10, 2011
  9. 06/13 credential: apply helper configJeff King, Dec 10, 2011
  10. 07/13 credential: add credential.*.usernameJeff King, Dec 10, 2011
  11. 08/13 credential: make relevance of http path configurableJeff King, Dec 10, 2011
  12. Jakub NarebskiDec 10, 2011
  13. Jeff KingDec 10, 2011
  14. 09/13 docs: end-user documentation for the credential subsystemJeff King, Dec 10, 2011
  15. 10/13 credentials: add "cache" helperJeff King, Dec 10, 2011
  16. Jonathan NiederJan 10, 2012
  17. Jeff KingJan 10, 2012
  18. Jeff KingJan 10, 2012
  19. Junio C HamanoJan 10, 2012
  20. Jeff KingJan 17, 2012
  21. Junio C HamanoJan 17, 2012
  22. Jonathan NiederJan 10, 2012
  23. Jeff KingJan 10, 2012
  24. Jonathan NiederJan 11, 2012
  25. Jeff KingJan 12, 2012
  26. 11/13 strbuf: add strbuf_add*_urlencodeJeff King, Dec 10, 2011
  27. Jakub NarebskiDec 10, 2011
  28. Jeff KingDec 10, 2011
  29. 12/13 credentials: add "store" helperJeff King, Dec 10, 2011
  30. 13/13 t: add test harness for external credential helpersJeff King, Dec 10, 2011
  31. 0/9 echo characters in username promptJeff King, Dec 10, 2011
  32. 1/9 imap-send: avoid buffer overflowJeff King, Dec 10, 2011
  33. 2/9 imap-send: don't check return value of git_getpassJeff King, Dec 10, 2011
  34. 3/9 move git_getpass to its own source fileJeff King, Dec 10, 2011
  35. 4/9 refactor git_getpass into generic prompt functionJeff King, Dec 10, 2011
  36. 5/9 add generic terminal prompt functionJeff King, Dec 10, 2011
  37. Pete WyckoffDec 15, 2011
  38. Jeff KingDec 15, 2011
  39. Pete WyckoffDec 15, 2011
  40. 6/9 prompt: use git_terminal_promptJeff King, Dec 10, 2011
  41. 7/9 credential: use git_prompt instead of git_getpassJeff King, Dec 10, 2011
  42. 8/9 Makefile: linux has /dev/ttyJeff King, Dec 10, 2011
  43. 9/9 Makefile: OS X has /dev/ttyJeff King, Dec 10, 2011
  44. 1/2 Makefile: Windows lacks /dev/ttyJohannes Sixt, Dec 12, 2011
  45. 2/2 Makefile: optionally exclude code that needs Unix socketsJohannes Sixt, Dec 12, 2011
  46. Jeff KingDec 12, 2011
  47. Junio C HamanoDec 12, 2011
  48. Junio C HamanoDec 13, 2011
  49. Jeff KingDec 13, 2011
  50. Johannes SixtDec 13, 2011
  51. Junio C HamanoDec 14, 2011
  52. Jeff KingDec 12, 2011
  53. Johannes SixtDec 12, 2011
  54. 1/1 contrib: add credential helper for OS X KeychainJeff King, Dec 10, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.