This is the mail archive of the
libc-alpha@sources.redhat.com
mailing list for the glibc project.
Re: [patch] lockf() fix
On Sun, Oct 29, 2000 at 09:24:14AM +0100, Andreas Jaeger wrote:
> >>>>> Ben Collins writes:
>
> > This seems to only affect sparc, sparc64 and alpha, since they are the
> > only ones that define F_RDLCK to something other than 0 (1, in each case).
>
> > Thanks to Anton Blanchard for helping me track this down.
>
> > --- ./sysdeps/generic/lockf.c~ Fri Oct 27 18:43:40 2000
> > +++ ./sysdeps/generic/lockf.c Fri Oct 27 18:34:25 2000
> > @@ -32,6 +32,7 @@
> > memset ((char *) &fl, '\0', sizeof (fl));
> >
> > /* lockf is always relative to the current file position. */
> > + fl.l_type = F_RDLCK;
> > fl.l_whence = SEEK_CUR;
> > fl.l_start = 0;
> > fl.l_len = len;
>
> I don't understand why this is needed at all. Looking at the code
> each of the switch cases sets l_type explicitly. Is l_type needed for
> the fcntl (fd, F_GETLK) ?
F_TEST does not set the l_type explicitly. Perhaps, this needs to be
inside the F_TEST case block.
> If your patch would be right, the same patch would be needed for
> lockf64.
Sorry, didn't look, but it does.
> Please explain a bit more why this is needed - or send a simple test
> program that fails on those architectures.
In the case of F_TEST, lockf() wants to send an l_type of F_RDLCK.
Normally this is ok not to set explicitly, since on most architectures,
F_RDLCK == 0 (and the memset above sets this implicitly). However, on
alpha, sparc and sparc64, these types are defined as:
/* for posix fcntl() and lockf() */
#define F_RDLCK 1
#define F_WRLCK 2
#define F_UNLCK 3
So you can see, that sending 0 as l_type is just undefined, not to
mention, plain wrong and thus the kernel returns -1 and sets EINVAL.
Here's a new patch that puts this in the right place, and includes
lockf64.c.
Thanks for looking at the patch,
Ben
--
-----------=======-=-======-=========-----------=====------------=-=------
/ Ben Collins -- ...on that fantastic voyage... -- Debian GNU/Linux \
` bcollins@debian.org -- bcollins@openldap.org -- bcollins@linux.com '
`---=========------=======-------------=-=-----=-===-======-------=--=---'
--- ChangeLog~ Sun Oct 29 07:38:00 2000
+++ ChangeLog Sun Oct 29 08:07:30 2000
@@ -1,0 +1,6 @@
+2000-10-29 Ben Collins <bcollins@debian.org>
+
+ * sysdeps/generic/lockf.c: In the case of F_TEST, set l_type to
+ F_RDLCK explicitly.
+ * sysdeps/unix/sysv/linux/i386/lockf64.c: Likewise.
+
--- sysdeps/generic/lockf.c~ Sun Oct 29 07:28:58 2000
+++ sysdeps/generic/lockf.c Sun Oct 29 07:37:53 2000
@@ -41,6 +41,7 @@
case F_TEST:
/* Test the lock: return 0 if FD is unlocked or locked by this process;
return -1, set errno to EACCES, if another process holds the lock. */
+ fl.l_type = F_RDLCK;
if (__fcntl (fd, F_GETLK, &fl) < 0)
return -1;
if (fl.l_type == F_UNLCK || fl.l_pid == __getpid ())
--- sysdeps/unix/sysv/linux/i386/lockf64.c~ Sun Oct 29 07:34:47 2000
+++ sysdeps/unix/sysv/linux/i386/lockf64.c Sun Oct 29 07:37:28 2000
@@ -85,6 +85,7 @@
/* Test the lock: return 0 if FD is unlocked or locked by this process;
return -1, set errno to EACCES, if another process holds the lock. */
#if __ASSUME_FCNTL64 > 0
+ fl64.l_type = F_RDLCK;
if (INLINE_SYSCALL (fcntl64, 3, fd, F_GETLK64, &fl64) < 0)
return -1;
if (fl64.l_type == F_UNLCK || fl64.l_pid == __getpid ())
@@ -93,6 +94,7 @@
return -1;
#else
# ifdef __NR_fcntl64
+ fl64.l_type = F_RDLCK;
if (!__have_no_fcntl64)
{
int res = INLINE_SYSCALL (fcntl64, 3, fd, F_GETLK64, &fl64);
@@ -120,6 +122,7 @@
}
}
# endif
+ fl.l_type = F_RDLCK;
if (__fcntl (fd, F_GETLK, &fl) < 0)
return -1;
if (fl.l_type == F_UNLCK || fl.l_pid == __getpid ())