Don't fake open() and fcntl() just for ef_open()

Put the Windows code into new open_locked() instead.  It's ugly having
that in file.c, but the fakes are ugly too, and somewhat brittle.
Remove posix_open(), F_SETLK, F_RDLCK, F_WRLCK, struct flock, and
simplify fcntl().

diff --git a/src/lib/common/file.c b/src/lib/common/file.c
index eee28cef446dfbc775f0864c0f56dfe6db1f2174..25e644f4c4ab771305d66c779322d88cc33ad174 100644 (file)
--- a/src/lib/common/file.c
+++ b/src/lib/common/file.c
@@ -40,12 +40,17 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <unistd.h>
+#ifdef _WIN32
+#include <io.h>
+#include <share.h>
+#endif
 #include "file.h"
 #include "match.h"
 #include "misc.h"
 #include "nsc.h"
 #include "prototypes.h"
 
+static int open_locked(char *, int, mode_t);
 static int ef_realloc_cache(struct empfile *, int);
 static int fillcache(struct empfile *, int);
 static int do_read(struct empfile *, void *, int, int);
@@ -67,7 +72,6 @@ int
 ef_open(int type, int how, int nelt)
 {
     struct empfile *ep;
-    struct flock lock;
     int oflags, fd, fsiz, nslots;
 
     if (ef_check(type) < 0)
@@ -84,24 +88,12 @@ ef_open(int type, int how, int nelt)
        oflags = O_RDONLY;
     if (how & EFF_CREATE)
        oflags |= O_CREAT | O_TRUNC;
-#if defined(_WIN32)
-    oflags |= O_BINARY;
-#endif
-    fd = open(ep->file, oflags, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP);
+    fd = open_locked(ep->file, oflags, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP);
     if (fd < 0) {
        logerror("Can't open %s (%s)", ep->file, strerror(errno));
        return 0;
     }
 
-    lock.l_type = how & EFF_PRIVATE ? F_RDLCK : F_WRLCK;
-    lock.l_whence = SEEK_SET;
-    lock.l_start = lock.l_len = 0;
-    if (fcntl(fd, F_SETLK, &lock) == -1) {
-       logerror("Can't lock %s (%s)", ep->file, strerror(errno));
-       close(fd);
-       return 0;
-    }
-
     /* get file size */
     fsiz = fsize(fd);
     if (fsiz % ep->size) {
@@ -162,6 +154,35 @@ ef_open(int type, int how, int nelt)
     return 1;
 }
 
+static int
+open_locked(char *name, int oflags, mode_t mode)
+{
+    int rdlonly = (oflags & O_ACCMODE) == O_RDONLY;
+    int fd;
+
+#ifdef _WIN32
+    fd = _sopen(name, oflags | O_BINARY, rdlonly ? SH_DENYNO : SH_DENYWR,
+               mode);
+    if (fd < 0)
+       return -1;
+#else  /* !_WIN32 */
+    struct flock lock;
+
+    fd = open(name, oflags, mode);
+    if (fd < 0)
+       return -1;
+
+    lock.l_type = rdlonly ? F_RDLCK : F_WRLCK;
+    lock.l_whence = SEEK_SET;
+    lock.l_start = lock.l_len = 0;
+    if (fcntl(fd, F_SETLK, &lock) == -1) {
+       close(fd);
+       return -1;
+    }
+#endif /* !_WIN32 */
+    return fd;
+}
+
 /*
  * Reallocate cache for table EP to hold COUNT slots.
  * The table must not be allocated statically.

diff --git a/src/lib/w32/posixio.c b/src/lib/w32/posixio.c
index 0a6f449efe010b9dd57ce40ced823fe060d4ee8b..b2b7f3d7eed79b794ec29266c025b0b570076386 100644 (file)
--- a/src/lib/w32/posixio.c
+++ b/src/lib/w32/posixio.c
@@ -45,9 +45,7 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <io.h>
-#include <share.h>
 #include <stdarg.h>
-#include <stdio.h>
 #include <stdlib.h>
 /*
  * Need to include winsock2.h before ws2tcpip.h.
@@ -279,32 +277,6 @@ posix_close(int fd)
     return _close(fd);
 }
 
-/*
- * POSIX equivalent for open().
- * Implements file locks when opening files to provide equivalent
- * F_GETLK/F_SETLK.
- */
-int
-posix_open(const char *fname, int oflag, ...)
-{
-    va_list ap;
-    int pmode = 0;
-
-    if (oflag & O_CREAT) {
-       va_start(ap, oflag);
-       pmode = va_arg(ap, int);
-       va_end(ap);
-    }
-
-    /*
-     * We don't implement fcntl() for F_SETLK.  Instead, we lock *all*
-     * files we open.  Not ideal, but it works for Empire.
-     */
-    return _sopen(fname, oflag,
-                 oflag & O_RDONLY ? SH_DENYNO : SH_DENYWR,
-                 pmode);
-}
-
 /*
  * POSIX equivalent for read().
  */
@@ -429,7 +401,6 @@ writev(int fd, const struct iovec *iov, int iovcnt)
  * Horrible hacks, just good enough support Empire's use of fcntl().
  * F_GETFL / F_SETFL support making a socket (non-)blocking by getting
  * flags, adding or removing O_NONBLOCK, and setting the result.
- * F_SETLK does nothing.  Instead, we lock in posix_open().
  */
 int
 fcntl(int fd, int cmd, ...)
@@ -455,8 +426,6 @@ fcntl(int fd, int cmd, ...)
            return -1;
        }
        return 0;
-    case F_SETLK:
-       return 0;
     }
     errno = EINVAL;
     return -1;

diff --git a/src/lib/w32/unistd.h b/src/lib/w32/unistd.h
index 528bb19ba4d1a8c1236245808261e286eaca9a4a..963c07b0862d132e633cb095e7b5097362d8f8db 100644 (file)
--- a/src/lib/w32/unistd.h
+++ b/src/lib/w32/unistd.h
@@ -89,26 +89,9 @@ extern int posix_mkdir(const char *dirname, mode_t perm);
 /* Should be in fcntl.h */
 #define O_NONBLOCK  1
 
-#define F_RDLCK            0
-#define F_WRLCK            1
 #define F_GETFL            1
 #define F_SETFL            2
-#define F_SETLK            3
 
-struct flock
-{
-    short l_type;
-    short l_whence;
-    off_t l_start;
-    off_t l_len;
-    /* intentionally missing: pid_t l_pid */
-};
-
-#define creat(fname, pmode) \
-    posix_open((fname), _O_WRONLY | _O_CREAT |_O_TRUNC, (pmode))
-#define open(fname, oflag, ...) \
-    posix_open((fname), (oflag), __VA_ARGS__)
-extern int posix_open(const char *fname, int oflag, ...);
 extern int fcntl(int fd, int cmd, ...);
 
 /* Stuff that actually belongs here */

diff --git a/src/lib/w32/w32misc.h b/src/lib/w32/w32misc.h
index 2116579bcb5b3dc3ee0732c4f4212b739dacd460..b008ea4105ce309542ae2807f2a4308fe3198ffc 100644 (file)
--- a/src/lib/w32/w32misc.h
+++ b/src/lib/w32/w32misc.h
@@ -46,6 +46,11 @@
 /* errno.h */
 #define EWOULDBLOCK EAGAIN
 
+/* fcntl.h */
+#ifdef _MSC_VER
+#define O_ACCMODE (_O_RDONLY|_O_WRONLY|_O_RDWR)
+#endif
+
 /* stdio.h */
 #define vsnprintf _vsnprintf
 #define snprintf _snprintf