From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 15 Apr 2016 01:37:16 +0000 (-0700)
Subject: Merge branch 'jk/credential-cache-comment-exit' into maint
X-Git-Tag: v2.8.2~24
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/485c7ade03c219e1b6003d566b6d0eae7bd44e1b?ds=inline;hp=-c

Merge branch 'jk/credential-cache-comment-exit' into maint

A code clarification.

* jk/credential-cache-comment-exit:
credential-cache--daemon: clarify "exit" action semantics
---

485c7ade03c219e1b6003d566b6d0eae7bd44e1b
diff --combined credential-cache--daemon.c
index caef21e4fc,8a3d0b7e2c..291c0fd5e9
--- a/credential-cache--daemon.c
+++ b/credential-cache--daemon.c
@@@ -96,12 -96,12 +96,12 @@@ static int read_request(FILE *fh, struc
  	static struct strbuf item = STRBUF_INIT;
  	const char *p;
  
 -	strbuf_getline(&item, fh, '\n');
 +	strbuf_getline_lf(&item, fh);
  	if (!skip_prefix(item.buf, "action=", &p))
  		return error("client sent bogus action line: %s", item.buf);
  	strbuf_addstr(action, p);
  
 -	strbuf_getline(&item, fh, '\n');
 +	strbuf_getline_lf(&item, fh);
  	if (!skip_prefix(item.buf, "timeout=", &p))
  		return error("client sent bogus timeout line: %s", item.buf);
  	*timeout = atoi(p);
@@@ -126,8 -126,17 +126,17 @@@ static void serve_one_client(FILE *in, 
  			fprintf(out, "password=%s\n", e->item.password);
  		}
  	}
- 	else if (!strcmp(action.buf, "exit"))
+ 	else if (!strcmp(action.buf, "exit")) {
+ 		/*
+ 		 * It's important that we clean up our socket first, and then
+ 		 * signal the client only once we have finished the cleanup.
+ 		 * Calling exit() directly does this, because we clean up in
+ 		 * our atexit() handler, and then signal the client when our
+ 		 * process actually ends, which closes the socket and gives
+ 		 * them EOF.
+ 		 */
  		exit(0);
+ 	}
  	else if (!strcmp(action.buf, "erase"))
  		remove_credential(&c);
  	else if (!strcmp(action.buf, "store")) {
@@@ -215,7 -224,7 +224,7 @@@ static const char permissions_advice[] 
  "users may be able to read your cached credentials. Consider running:\n"
  "\n"
  "	chmod 0700 %s";
 -static void check_socket_directory(const char *path)
 +static void init_socket_directory(const char *path)
  {
  	struct stat st;
  	char *path_copy = xstrdup(path);
@@@ -224,27 -233,20 +233,27 @@@
  	if (!stat(dir, &st)) {
  		if (st.st_mode & 077)
  			die(permissions_advice, dir);
 -		free(path_copy);
 -		return;
 +	} else {
 +		/*
 +		 * We must be sure to create the directory with the correct mode,
 +		 * not just chmod it after the fact; otherwise, there is a race
 +		 * condition in which somebody can chdir to it, sleep, then try to open
 +		 * our protected socket.
 +		 */
 +		if (safe_create_leading_directories_const(dir) < 0)
 +			die_errno("unable to create directories for '%s'", dir);
 +		if (mkdir(dir, 0700) < 0)
 +			die_errno("unable to mkdir '%s'", dir);
  	}
  
 -	/*
 -	 * We must be sure to create the directory with the correct mode,
 -	 * not just chmod it after the fact; otherwise, there is a race
 -	 * condition in which somebody can chdir to it, sleep, then try to open
 -	 * our protected socket.
 -	 */
 -	if (safe_create_leading_directories_const(dir) < 0)
 -		die_errno("unable to create directories for '%s'", dir);
 -	if (mkdir(dir, 0700) < 0)
 -		die_errno("unable to mkdir '%s'", dir);
 +	if (chdir(dir))
 +		/*
 +		 * We don't actually care what our cwd is; we chdir here just to
 +		 * be a friendly daemon and avoid tying up our original cwd.
 +		 * If this fails, it's OK to just continue without that benefit.
 +		 */
 +		;
 +
  	free(path_copy);
  }
  
@@@ -271,10 -273,7 +280,10 @@@ int main(int argc, const char **argv
  	if (!socket_path)
  		usage_with_options(usage, options);
  
 -	check_socket_directory(socket_path);
 +	if (!is_absolute_path(socket_path))
 +		die("socket directory must be an absolute path");
 +
 +	init_socket_directory(socket_path);
  	register_tempfile(&socket_file, socket_path);
  
  	if (ignore_sighup)