git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Brandon Casey <casey@nrlssc.navy.mil>
Date
Feb 9, 2008, 01:20 UTC
Message-ID
<47ACFFD9.2030705@nrlssc.navy.mil>
In-Reply-To
<7v8x1vjiic.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
Show 41 quoted lines
> "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
Previous: Junio C HamanoNext: Brandon Casey
Message 22 of 27 in “opening files in remote.c should ensure it is opening a file”
  1. opening files in remote.c should ensure it is opening a fileH.Merijn Brand, Feb 8, 2008
  2. Mike RalphsonFeb 8, 2008
  3. H.Merijn BrandFeb 8, 2008
  4. H.Merijn BrandFeb 18, 2008
  5. Junio C HamanoFeb 18, 2008
  6. H.Merijn BrandFeb 18, 2008
  7. Daniel BarkalowFeb 8, 2008
  8. Johannes SchindelinFeb 8, 2008
  9. Junio C HamanoFeb 9, 2008
  10. Daniel BarkalowFeb 9, 2008
  11. Junio C HamanoFeb 8, 2008
  12. Johannes SchindelinFeb 8, 2008
  13. H.Merijn BrandFeb 9, 2008
  14. Morten WelinderFeb 8, 2008
  15. Junio C HamanoFeb 8, 2008
  16. Johannes SchindelinFeb 8, 2008
  17. Junio C HamanoFeb 8, 2008
  18. Johannes SchindelinFeb 8, 2008
  19. Brandon CaseyFeb 8, 2008
  20. Brandon CaseyFeb 8, 2008
  21. Junio C HamanoFeb 9, 2008
  22. Brandon CaseyFeb 9, 2008
  23. Add compat/fopen.c which returns NULL on attempt to open directoryBrandon Casey, Feb 9, 2008
  24. H.Merijn BrandFeb 11, 2008
  25. H.Merijn BrandFeb 11, 2008
  26. Junio C HamanoFeb 12, 2008
  27. H.Merijn BrandFeb 12, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.