Re: [PATCH] opening files in remote.c should ensure it is opening a file

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 8, 2008, 20:33 UTC
Message-ID: <7v8x1vjiic.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <118833cc0802081215t380587f6w7b5c0aba66a55799@mail.gmail.com>

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

Show 11 quoted lines

>> +/* 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.