git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Morten Welinder" <mwelinder@gmail.com>
Cc: "H.Merijn Brand" <h.m.brand@xs4all.nl>, git@vger.kernel.org
Subject: Re: [PATCH] opening files in remote.c should ensure it is opening a file
Date: Fri, 08 Feb 2008 12:33:15 -0800	[thread overview]
Message-ID: <7v8x1vjiic.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <118833cc0802081215t380587f6w7b5c0aba66a55799@mail.gmail.com> (Morten Welinder's message of "Fri, 8 Feb 2008 15:15:40 -0500")

"Morten Welinder" <mwelinder@gmail.com> writes:

>> +/* Helper function to ensure that we are opening a file and not a directory */
>> +static FILE *open_file(char *full_path)
>> +{
>> +       struct stat st_buf;
>> +       if (stat(full_path, &st_buf) || !S_ISREG(st_buf.st_mode))
>> +               return NULL;
>> +       return (fopen(full_path, "r"));
>> +}
>
> That looks wrong.  stat+fopen has a pointless race condition that
> open+fstat+fdopen would not have.

That's true.  How about doing something like this?

 (1) in a new file "compat/gitfopen.c" have this:

	#include "../git-compat-util.h"
	#undef fopen
	FILE *gitfopen(const char *path, const char *mode)
        {
		int fd, flags;
                struct stat st;
        	if (mode[0] == 'w')
                	return fopen(path, mode);
		switch (mode[0]) {
                case 'r': flags = O_RDONLY; break;
                case 'a': flags = O_APPEND; break;
		default:
			errno = EINVAL;
                	return NULL;
		}
		fd = open(path, flags);
		if (fd < 0 || fstat(fd, &st))
                	return NULL;
		if (S_ISDIR(st_buf.st_mode)) {
                	errno = EISDIR;
                        return NULL;
		}
		return fdopen(fd, mode);
	}

  (2) in "git-compat-util.h" have this:

	#ifdef FOPEN_OPENS_DIRECTORIES
        #define fopen(a,b) gitfopen(a,b)
	extern FILE *gitfopen(const char *, const char *);
        #endif

And have Makefile set FOPEN_OPENS_DIRECTORIES on appropriate
platforms.

  reply	other threads:[~2008-02-08 20:34 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-08 16:46 [PATCH] opening files in remote.c should ensure it is opening a file H.Merijn Brand
2008-02-08 17:25 ` Mike Ralphson
2008-02-08 20:04   ` H.Merijn Brand
2008-02-18  9:10     ` H.Merijn Brand
2008-02-18  9:30       ` Junio C Hamano
2008-02-18 11:31         ` H.Merijn Brand
2008-02-08 20:36   ` Daniel Barkalow
2008-02-08 20:44     ` Johannes Schindelin
2008-02-09  5:27     ` Junio C Hamano
2008-02-09  5:54       ` Daniel Barkalow
2008-02-08 20:09 ` Junio C Hamano
2008-02-08 20:38   ` Johannes Schindelin
2008-02-09 10:03   ` H.Merijn Brand
2008-02-08 20:15 ` Morten Welinder
2008-02-08 20:33   ` Junio C Hamano [this message]
2008-02-08 20:40     ` Johannes Schindelin
2008-02-08 21:19       ` Junio C Hamano
2008-02-08 21:47         ` Johannes Schindelin
2008-02-08 20:58     ` Brandon Casey
2008-02-08 21:14       ` Brandon Casey
2008-02-09  5:12       ` Junio C Hamano
2008-02-09  1:20     ` Brandon Casey
2008-02-09  2:32       ` [PATCH] Add compat/fopen.c which returns NULL on attempt to open directory Brandon Casey
2008-02-11  9:29         ` H.Merijn Brand
2008-02-11 10:15           ` H.Merijn Brand
2008-02-12  0:20             ` Junio C Hamano
2008-02-12 15:27               ` H.Merijn Brand

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=7v8x1vjiic.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=h.m.brand@xs4all.nl \
    --cc=mwelinder@gmail.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).