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

[Brandon Casey <casey@nrlssc.navy.mil>]

Junio C Hamano wrote:
> "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);
> 	}

Can we use fileno()? Something like:

FILE *gitfopen(const char *path, const char *mode)
{   
        FILE *fp;
        struct stat st;

        if (strpbrk(mode, "wa"))
                return fopen(path, mode);

        if (!(fp = fopen(path, mode)))
                return NULL;

        if (fstat(fileno(fp), &st)) {
                fclose(fp);
                return NULL;
        }

        if (S_ISDIR(st.st_mode)) {
                fclose(fp);
                errno = EISDIR;
                return NULL;
        }

        return fp;
}   

-brandon