Re: fcntl() F_GETLK semantics vs. test t_vnops.c

["J. Hannken-Illjes" <hannken%eis.cs.tu-bs.de@localhost>]

On Jul 22, 2011, at 12:13 PM, Alexander Nasonov wrote:

> J. Hannken-Illjes wrote:
>> The test fcntl_getlock_pids() from fs/vfs/t_vnops.c assumes
>> fcntl(fd, F_GETLK, &lock) returns the blocking lock with the
>> lowest start offset.
>> 
>> Our documentation and POSIX.1 document it returning the
>> "first lock that blocks" but doesn't call for any specific order.
> 
> I think it's an ambiguity in POSIX. I wrote the test in assumption
> that "first" means a lock with the lowest start offset but my
> intention was to interate over *all* locks in any order. I think
> it's still possible with the current behaviour but with a less
> straightforward implementation: rather then moving linearly through
> a file, you will have to build a tree.

As any program relying on a special order is not portable I would suggest to
either remove this test or change it as appended testing overlap only.

If noone objects I will commit this change during the weekend.

--
Juergen Hannken-Illjes - hannken%eis.cs.tu-bs.de@localhost - TU Braunschweig 
(Germany)

Index: t_vnops.c
===================================================================
RCS file: /cvsroot/src/tests/fs/vfs/t_vnops.c,v
retrieving revision 1.25
diff -p -u -4 -r1.25 t_vnops.c
--- t_vnops.c   20 Jul 2011 11:52:00 -0000      1.25
+++ t_vnops.c   22 Jul 2011 12:27:55 -0000
@@ -635,8 +635,15 @@ flock_compare(const void *p, const void 
        int b = ((const struct flock *)q)->l_start;
        return a < b ? -1 : (a > b ? 1 : 0);
 }
 
+static bool
+flock_overlap(off_t start_a, off_t len_a, off_t start_b, off_t len_b)
+{
+
+       return (start_a <= start_b + len_b && start_a + len_a >= start_b);
+}
+
 static void
 fcntl_getlock_pids(const atf_tc_t *tc, const char *mp)
 {
        /* test non-overlaping ranges */
@@ -682,17 +689,15 @@ fcntl_getlock_pids(const atf_tc_t *tc, c
                RL(rump_sys_ftruncate(fd[i], sz));
                RL(rump_sys_fcntl(fd[i], F_SETLK, &lock[i]));
        }
 
-       atf_tc_expect_fail("PR kern/44494");
        /*
         * In the context of each pid , do GETLK for a readlock from
-        * i = [0,__arraycount(locks)].  If we try to lock from the same
-        * start offset as the lock our current process holds, check
-        * that we fail on the offset of the next lock ("else if" branch).
-        * Otherwise, expect to get a lock for the current offset
-        * ("if" branch).  The "else" branch is purely for the last
-        * process where we expect no blocking locks.
+        * i = [0,__arraycount(locks)].  If we expect an overlapping lock
+        * test the result for overlap. Note that we get the first lock that
+        * blocks which is not necessarily the lock with the lowest start
+        * offset.  The "else" branch is purely for the last process where
+        * we expect no blocking locks.
         */
        for(i = 0; i < __arraycount(lwp); i++) {
                rump_pub_lwproc_switch(lwp[i]);
 
@@ -708,17 +713,19 @@ fcntl_getlock_pids(const atf_tc_t *tc, c
                                /*
                                 * lock set by another process
                                 */
                                ATF_CHECK(l.l_type != F_UNLCK);
-                               ATF_CHECK_EQ(l.l_start, expect[j].l_start);
-                               ATF_CHECK_EQ(l.l_len,   expect[j].l_len);
+                               ATF_CHECK_MSG(flock_overlap(l.l_start,
+                                   l.l_len, expect[j].l_start, sz),
+                                   "locks don't overlap");
                        } else if (j != __arraycount(lwp) - 1) {
                                /*
                                 * lock set by the current process
                                 */
                                ATF_CHECK(l.l_type != F_UNLCK);
-                               ATF_CHECK_EQ(l.l_start, expect[j+1].l_start);
-                               ATF_CHECK_EQ(l.l_len,   expect[j+1].l_len);
+                               ATF_CHECK_MSG(flock_overlap(l.l_start,
+                                   l.l_len, expect[j].l_start, sz),
+                                   "locks don't overlap");
                        } else {
                                /*
                                 * there are no other locks after the
                                 * current process lock