public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Jeff King <peff@peff•net>
To: "René Scharfe" <l.s.r@web•de>
Cc: Karsten Blees <karsten.blees@gmail•com>,
	Duy Nguyen <pclouds@gmail•com>,
	Git Mailing List <git@vger•kernel.org>,
	Junio C Hamano <gitster@pobox•com>
Subject: Re: [PATCH] abspath.c: use PATH_MAX in real_path_internal()
Date: Sun, 20 Jul 2014 22:25:01 -0400	[thread overview]
Message-ID: <20140721022501.GB22750@peff.net> (raw)
In-Reply-To: <53CB7729.40101@web.de>

On Sun, Jul 20, 2014 at 10:00:41AM +0200, René Scharfe wrote:

> -- >8 --
> Subject: [PATCH] unix-socket: remove stale socket before calling chdir()
> 
> unix_stream_listen() is given a path.  It calls unix_sockaddr_init(),
> which in turn can call chdir().  After that a relative path doesn't
> mean the same as before.  Any use of the original path should thus
> happen before that call.  For that reason, unlink the given path
> (to get rid of a possibly existing stale socket) right at the
> beginning of the function.

Thanks, I think this ordering problem was just missed in 1eb10f4
(unix-socket: handle long socket pathnames, 2012-01-09).

Your solution looks OK, though I think also just using:

  unlink(sa.sun_path);

would work, too (that is the path we are feeding to bind(), whether we
have chdir'd or not, so perhaps it is a little more obviously correct?).
I'm OK with either.

> diff --git a/unix-socket.c b/unix-socket.c
> index 01f119f..91bd6b8 100644
> --- a/unix-socket.c
> +++ b/unix-socket.c
> @@ -99,11 +99,12 @@ int unix_stream_listen(const char *path)
>  	struct sockaddr_un sa;
>  	struct unix_sockaddr_context ctx;
>  
> +	unlink(path);
> +
>  	if (unix_sockaddr_init(&sa, path, &ctx) < 0)
>  		return -1;
>  	fd = unix_stream_socket();
>  
> -	unlink(path);

I briefly wondered if this should be unlinking only when we get EEXIST,
but I don't think it is worth caring about. The only caller is
credential-cache, and it always wants to unconditionally replace
whatever is there (it will already have tried to contact any existing
socket).

-Peff

next prev parent reply	other threads:[~2014-07-21  2:25 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-17 12:45 [PATCH] abspath.c: use PATH_MAX in real_path_internal() Nguyễn Thái Ngọc Duy
2014-07-17 17:05 ` René Scharfe
2014-07-17 18:13   ` Junio C Hamano
2014-07-17 23:03   ` Karsten Blees
2014-07-18 10:49     ` Duy Nguyen
2014-07-18 15:08       ` René Scharfe
2014-07-19 12:51         ` Duy Nguyen
2014-07-20  0:29       ` Karsten Blees
2014-07-20  8:00         ` René Scharfe
2014-07-21  2:25           ` Jeff King [this message]
2014-07-18 11:32     ` René Scharfe
2014-07-19 23:55       ` Karsten Blees
2014-07-20 11:17         ` René Scharfe
2014-07-17 18:03 ` Junio C Hamano
2014-07-17 23:02   ` Karsten Blees
2014-07-17 23:03 ` Karsten Blees
2014-07-18 16:45   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140721022501.GB22750@peff.net \
    --to=peff@peff$(echo .)net \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=karsten.blees@gmail$(echo .)com \
    --cc=l.s.r@web$(echo .)de \
    --cc=pclouds@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox