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

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:44:11

"H.Merijn Brand" [off-list ref] writes:
HP-UX allows directories to be opened with fopen (path, "r"), which
will cause some translations that expect to read files, read dirs
instead. This patch makes sure the two fopen () calls in remote.c
only open the file if it is a file.
+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"));
+}
Can we make this a platform specific "compat" hack?

It is not fair to force stat() overhead to ports on platforms
that fails fopen() on directories, as I doubt we would ever want
from directory using fopen() anyway.

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

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:44:11

Hi,

On Fri, 8 Feb 2008, Junio C Hamano wrote:
"H.Merijn Brand" [off-list ref] writes:
quoted
HP-UX allows directories to be opened with fopen (path, "r"), which
will cause some translations that expect to read files, read dirs
instead. This patch makes sure the two fopen () calls in remote.c
only open the file if it is a file.
quoted
+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"));
+}
Can we make this a platform specific "compat" hack?
You mean something like

#ifdef FOPEN_OPENS_DIRECTORIES
inline static FILE *fopen_compat(const char *path, const char *mode)
{
       struct stat st_buf;
       if (stat(path, &st_buf) || !S_ISREG(st_buf.st_mode))
               return NULL;
       return (fopen(path, mode));
}
#define fopen fopen_compat
#endif

in git-compat-util.h, right?

Yeah, I can see that, even if I think the overhead would not be _that_ 
crucial.  But it is a nice way of fixing _all_ fopen() calls at the same 
time.

Ciao,
Dscho

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

From: H.Merijn Brand <hidden>
Date: 2016-06-15 22:44:12

On Fri, 08 Feb 2008 12:09:46 -0800, Junio C Hamano [off-list ref] wrote:
"H.Merijn Brand" [off-list ref] writes:
quoted
HP-UX allows directories to be opened with fopen (path, "r"), which
will cause some translations that expect to read files, read dirs
instead. This patch makes sure the two fopen () calls in remote.c
only open the file if it is a file.
quoted
+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"));
+}
Can we make this a platform specific "compat" hack?

It is not fair to force stat() overhead to ports on platforms
that fails fopen() on directories,
The two I patched were in remote.c and do not happen on every file if I
analyzed it correctly, so overhead would be minimal. However, as I read
the rest of the discussion already, your approach to fix all fopen ()
calls at once seems very reasonable.

Can I get the patch when it is submitted?
as I doubt we would ever want from directory using fopen() anyway.
I didn't check

-- 
H.Merijn Brand         Amsterdam Perl Mongers (http://amsterdam.pm.org/)
using & porting perl 5.6.2, 5.8.x, 5.10.x  on HP-UX 10.20, 11.00, 11.11,
& 11.23, SuSE 10.1 & 10.2, AIX 5.2, and Cygwin.       http://qa.perl.org
http://mirrors.develooper.com/hpux/            http://www.test-smoke.org
                        http://www.goldmark.org/jeff/stupid-disclaimers/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help