Re: [PATCH glibc] Add file record locking support
On Thu, 2015-01-08 at 16:56 +0100, Guillem Jover wrote:
> On Thu, 2015-01-08 at 12:40:12 +0100, Svante Signell wrote:
> > Index: glibc-2.19/sysdeps/mach/hurd/fcntl.c
> > ===================================================================
> > --- glibc-2.19.orig/sysdeps/mach/hurd/fcntl.c
> > +++ glibc-2.19/sysdeps/mach/hurd/fcntl.c
> > @@ -128,56 +127,87 @@ __libc_fcntl (int fd, int cmd, ...)
> > case F_SETLK:
> > case F_SETLKW:
> > {
> […]
> > struct flock *fl = va_arg (ap, struct flock *);
> […]
> > + struct flock64 *fl64 = malloc (sizeof (struct flock64));
>
> You are not checking if malloc failed, but in any case there's no need
> at all to malloc the struct, just use «struct flock64 fl64».
Yes you are right, no checks are made. I removed the malloc part. What
about freeing fl64 later on?
> > + switch (fl->l_whence)
> > + {
> > + case SEEK_SET:
> > + case SEEK_CUR:
> > + case SEEK_END:
> > + break;
> > + default:
> > + errno = EINVAL;
> > + return -1;
> > + break;
> > + }
>
> The indentation here and in previous places seems messed up, check for
> space vs tab and similar.
Fixed! (hopefully)
Updated patch attached.
sysdeps/mach/hurd/Changelog
2014-08-21 Svante Signell <svante.signell@gmail.com>
* fcntl.c: Add support for file-record-lock RPC fixing posix
file locking using the flock64 version of struct flock.
* bits/fcntl.h: Since MIG cannot mix 32 bit and 64 bit
integers define unique numbers for F_GETLK64, F_SETLK64 and
F_SETLKW64 to prepare for a flock64 implementation of file
record locking in hurd.
Index: glibc-2.19/sysdeps/mach/hurd/bits/fcntl.h
===================================================================
--- glibc-2.19.orig/sysdeps/mach/hurd/bits/fcntl.h
+++ glibc-2.19/sysdeps/mach/hurd/bits/fcntl.h
@@ -163,9 +163,18 @@
# define F_GETOWN 5 /* Get owner (receiver of SIGIO). */
# define F_SETOWN 6 /* Set owner (receiver of SIGIO). */
#endif
+#ifdef __USE_FILE_OFFSET64
+# define F_GETLK F_GETLK64
+# define F_SETLK F_SETLK64
+# define F_SETLKW F_SETLKW64
+#else
#define F_GETLK 7 /* Get record locking info. */
#define F_SETLK 8 /* Set record locking info (non-blocking). */
#define F_SETLKW 9 /* Set record locking info (blocking). */
+#endif
+#define F_GETLK64 10 /* Get record locking info. */
+#define F_SETLK64 11 /* Set record locking info (non-blocking). */
+#define F_SETLKW64 12 /* Set record locking info (blocking). */
#ifdef __USE_XOPEN2K8
# define F_DUPFD_CLOEXEC 1030 /* Duplicate, set FD_CLOEXEC on new one. */
Index: glibc-2.19/sysdeps/mach/hurd/fcntl.c
===================================================================
--- glibc-2.19.orig/sysdeps/mach/hurd/fcntl.c
+++ glibc-2.19/sysdeps/mach/hurd/fcntl.c
@@ -20,7 +20,6 @@
#include <hurd.h>
#include <hurd/fd.h>
#include <stdarg.h>
-#include <sys/file.h> /* XXX for LOCK_* */
/* Perform file control operations on FD. */
int
@@ -128,56 +127,87 @@ __libc_fcntl (int fd, int cmd, ...)
case F_SETLK:
case F_SETLKW:
{
- /* XXX
- We need new RPCs to support POSIX.1 fcntl file locking!!
- For the time being we support the whole-file case only,
- with all kinds of WRONG WRONG WRONG semantics,
- by using flock. This is definitely the Wrong Thing,
- but it might be better than nothing (?). */
struct flock *fl = va_arg (ap, struct flock *);
- va_end (ap);
- switch (cmd)
+ struct flock64 fl64;
+
+ if (cmd == F_GETLK)
+ cmd = F_GETLK64;
+ if (cmd == F_SETLK)
+ cmd = F_SETLK64;
+ if (cmd == F_SETLKW)
+ cmd = F_SETLKW64;
+
+ switch (fl->l_type)
{
- case F_GETLK:
- errno = ENOSYS;
- return -1;
- case F_SETLK:
- cmd = LOCK_NB;
+ case F_RDLCK:
+ case F_WRLCK:
+ case F_UNLCK:
break;
default:
- cmd = 0;
+ errno = EINVAL;
+ return -1;
break;
}
- switch (fl->l_type)
+ switch (fl->l_whence)
{
- case F_RDLCK: cmd |= LOCK_SH | __LOCK_ATOMIC; break;
- case F_WRLCK: cmd |= LOCK_EX | __LOCK_ATOMIC; break;
- case F_UNLCK: cmd |= LOCK_UN; break;
+ case SEEK_SET:
+ case SEEK_CUR:
+ case SEEK_END:
+ break;
default:
errno = EINVAL;
return -1;
+ break;
+ }
+
+ fl64->l_type = fl->l_type;
+ fl64->l_whence = fl->l_whence;
+ fl64->l_start = fl->l_start;
+ fl64->l_len = fl->l_len;
+ fl64->l_pid = fl->l_pid;
+ err = HURD_FD_PORT_USE (d, __file_record_lock (port, cmd, fl64));
+ fl->l_type = fl64->l_type;
+ fl->l_whence = fl64->l_whence;
+ fl->l_start = fl64->l_start;
+ fl->l_len = fl64->l_len;
+ fl->l_pid = fl64->l_pid;
+ free (fl64);
+ result = err ? __hurd_dfail (fd, err) : 0;
+ break;
+ }
+
+ case F_GETLK64:
+ case F_SETLK64:
+ case F_SETLKW64:
+ {
+ struct flock64 *fl = va_arg (ap, struct flock64 *);
+
+ switch (fl->l_type)
+ {
+ case F_RDLCK:
+ case F_WRLCK:
+ case F_UNLCK:
+ break;
+ default:
+ errno = EINVAL;
+ return -1;
+ break;
}
switch (fl->l_whence)
{
case SEEK_SET:
- if (fl->l_start == 0 && fl->l_len == 0) /* Whole file request. */
- break;
- /* It seems to be common for applications to lock the first
- byte of the file when they are really doing whole-file locking.
- So, since it's so wrong already, might as well do that too. */
- if (fl->l_start == 0 && fl->l_len == 1)
- break;
- /* FALLTHROUGH */
case SEEK_CUR:
case SEEK_END:
- errno = ENOTSUP;
- return -1;
+ break;
default:
errno = EINVAL;
return -1;
+ break;
}
- return __flock (fd, cmd);
+ err = HURD_FD_PORT_USE (d, __file_record_lock (port, cmd, fl));
+ result = err ? __hurd_dfail (fd, err) : 0;
+ break;
}
case F_GETFL: /* Get per-open flags. */
Reply to: