All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andreas Ericsson <ae@op5.se>
To: git@vger.kernel.org
Subject: Re: [PATCH 1/3] C implementation of the 'git' program, take two.
Date: Wed, 16 Nov 2005 01:42:15 +0100	[thread overview]
Message-ID: <437A8067.9050308@op5.se> (raw)
In-Reply-To: <Pine.LNX.4.64.0511151603510.11232@g5.osdl.org>

Linus Torvalds wrote:
> 
> On Wed, 16 Nov 2005, Andreas Ericsson wrote:
> 
>>+
>>+	/* allow relative paths, but run with exact */
>>+	if (chdir(exec_path)) {
>>+		printf("git: '%s': %s\n", exec_path, strerror(errno));
>>+		exit (1);
>>+	}
>>+
>>+	getcwd(git_command, sizeof(git_command));
>>+	chdir(wd);
> 
> 
> Argh. This is pretty horrible way to turn a path into an absolute one. 


It was your idea to begin with, actually, in the thread on how to do 
proper path validation in the git-daemon. :)

> 
> Why don't you just do
> 
> 	if (exec_path[0] != '/') {
> 		.. prepend "cwd/" to exec_path ..
> 

You mean
	setenv("PATH", concat3(cwd, exec_path, old_path), 1);

?

Because that can fail too. Every solution is bad if you twist and turn 
it enough.

> 
> The reason to avoid "chdir(relative) + chdir(back)" is that it totally 
> unnecessarily breaks under some extreme cases. For example, if the 
> exec_path is already absolute, and we just happen to be in a really deep 
> subdirectory, then the getcwd() could have failed due to the PATH_MAX 
> limitations.
> 

True. So, would something like
char *try_goddamn_hard_to_get_working_dir(char *rel_path)
{
	size_t plen = PATH_MAX;
	char *p = malloc(PATH_MAX);
	while(p && plen < 1 << 20 && !(p = getcwd(p, plen))) {
		plen += PATH_MAX;
		p = realloc(p, plen);
	}

	return p;
}

be considered safe from this particular point of view?


> Also, depending on getcwd() will not work if any parent directory is 
> unreadable or non-executable (well, under Linux it will, as long as it's 
> executable, since getcwd() is actually a system call. Not in UNIX in 
> general, though). Again, that means that unless you _have_ to know what 
> the cwd is, you should try to avoid relying on it.
> 

True. I'll redo that part (tomorrow).

> 
> and the nicer thign to do is to just not try to be clever.
> 

I never try. It just comes to me naturally. ;)

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

  reply	other threads:[~2005-11-16  0:42 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-11-15 23:31 [PATCH 1/3] C implementation of the 'git' program, take two Andreas Ericsson
2005-11-15 23:45 ` Junio C Hamano
2005-11-16  0:10   ` Andreas Ericsson
2005-11-16  0:17     ` Junio C Hamano
2005-11-16  0:18 ` Linus Torvalds
2005-11-16  0:42   ` Andreas Ericsson [this message]
2005-11-16  2:02     ` Linus Torvalds
2005-11-16  2:18       ` Johannes Schindelin
2005-11-16 21:04         ` Alex Riesen

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=437A8067.9050308@op5.se \
    --to=ae@op5.se \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.