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

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jan 10, 2012, 17:44 UTC
Message-ID: <20120110174420.GA22184@burratino>
In-Reply-To: <20120110044430.GA23619@sigill.intra.peff.net>

Jeff King wrote:

Show 11 quoted lines

>   2. A hack in credential-cache won't help any unix-socket
>      users who come along later.
>
>   3. The chdir trickery isn't that likely to fail (basically
>      it's only a problem if your cwd is missing or goes away
>      while you're running).  And because we only enable the
>      hack when we get a too-long name, it can only fail in
>      cases that would have failed under the previous code
>      anyway.
>
> Signed-off-by: Jeff King <peff@peff.net>

A part of me worries that this assumption (3), and the additional assumption that nothing running concurrently cares about the cwd, might come back to bite us when the future (2) arrives. But I don't see a better approach.

Follow-on ideas: on platforms that support it, it would be nice to use

	ctx->orig_dirfd = open(".", O_RDONLY);
	if (ctx->orig_dirfd < 0)
		... error handling ...
	...
	if (ctx->orig_dirfd >= 0) {
		if (fchdir(ctx->orig_dirfd))
			die_errno("unable to restore original working directory");
		close(ctx->orig_dirfd);
	}

> --- a/unix-socket.c
> +++ b/unix-socket.c
> @@ -9,27 +9,86 @@ static int unix_stream_socket(void)

[...]

> +		dir = path;
> +		dir_len = slash - path;

[...]

> +		if (chdir_len(dir, dir_len) < 0)
> +			return -1;

Could avoid some complication by eliminating the dir_len var.

		if (chdir_len(dir, slash - dir))
			return -1;

Neither seems worth holding up the patch, so
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>