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 - hann...@eis.cs.tu-bs.de - 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
