On Fri, Aug 12, 2016 at 12:35:44PM +0300, Alex Peshkoff wrote:
> On 08/11/2016 03:13 PM, Dimitry Sibiryakov wrote:
> >     Hello, All.
> >
> >     If posix/os_utils.cpp I see following code:
> >
> >> // force file descriptor to have O_CLOEXEC set
> >> int open(const char* pathname, int flags, mode_t mode)
> >> {
> >>    int fd;
> >>    fd = openFile(pathname, flags | O_CLOEXEC, mode);
> >>
> >>    if (fd < 0 && errno == EINVAL)  // probably O_CLOEXEC not accepted
> >>            fd = openFile(pathname, flags | O_CLOEXEC, mode);
> >>
> >>    setCloseOnExec(fd);
> >>    return fd;
> >> }
> >     Shouldn't second call of openFile have "flags & ~O_CLOEXEC" parameter, 
> > because in
> > current form it doesn't differ from the first call and has to fail as 
> > well?..
> >
> 
> Thanks fixed.
> Luckily this flags does not work only for old kernels (<= 2.6.23) which 
> are anyway not recommended for running FB3 and hard to find in the real 
> world, therefore I do not mention this in the tracker.

Hmm, grepped though source and there's a lot of calls to fopen, which since
glibc 2.7 accepts "e" mode to open file with O_CLOEXEC. Also, what's that
thing with SYSCALL_INTERRUPTED? Wouldn't it be more feasible to set signal
mask for process and do not bother with that later? Or is there blocking
i/o used somewhere and signal is used to abort it? Patch bellow gives you
idea, but it is not even compile tested. I'd rather see that signal thing
sorted out first. Btw, I'm considering very bad practice to hide posix
system calls beyond some homebrew functions...

regards,
        ladis

diff --git a/src/common/os/posix/os_utils.cpp b/src/common/os/posix/os_utils.cpp
index bbd5651..51caa4a 100644
--- a/src/common/os/posix/os_utils.cpp
+++ b/src/common/os/posix/os_utils.cpp
@@ -266,11 +266,8 @@ bool isIPv6supported()
 // setting flag is not absolutely required, therefore ignore errors here
 void setCloseOnExec(int fd)
 {
-       if (fd >= 0)
-       {
-               while (fcntl(fd, F_SETFD, O_CLOEXEC) < 0 && 
SYSCALL_INTERRUPTED(errno))
-                       ;
-       }
+       while (fcntl(fd, F_SETFD, O_CLOEXEC) < 0 && SYSCALL_INTERRUPTED(errno))
+               ;
 }
 
 // force file descriptor to have O_CLOEXEC set
@@ -279,16 +276,35 @@ int open(const char* pathname, int flags, mode_t mode)
        int fd;
        fd = openFile(pathname, flags | O_CLOEXEC, mode);
 
-       if (fd < 0 && errno == EINVAL)  // probably O_CLOEXEC not accepted
+       if (fd < 0 && errno == EINVAL) {        // probably O_CLOEXEC not 
accepted
                fd = openFile(pathname, flags, mode);
+               if (fd >= 0)
+                       setCloseOnExec(fd);
+       }
 
-       setCloseOnExec(fd);
        return fd;
 }
 
 FILE* fopen(const char* pathname, const char* mode)
 {
        FILE* f = NULL;
+       char _mode[8] = "e";
+
+       strncpy(_mode + 1, mode, sizeof(_mode) - 1);
+       do
+       {
+#ifdef LSB_BUILD
+               // TODO: use open + fdopen to avoid races
+               f = fopen64(pathname, _mode);
+#else
+               f = ::fopen(pathname, _mode);
+#endif
+       } while (f == NULL && SYSCALL_INTERRUPTED(errno));
+
+       if (f)
+               return f;
+
+       // probably mode "e" (O_CLOEXEC) not accepted
        do
        {
 #ifdef LSB_BUILD

------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity 
planning reports. http://sdm.link/zohodev2dev
Firebird-Devel mailing list, web interface at 
https://lists.sourceforge.net/lists/listinfo/firebird-devel

Reply via email to