Refactor with new API and merge fcntl27 + fcntl28

[ilmanzo]

Merging into one since these two tests are quite similar and differs only by the mode/flags they create testfile.

$ diff fcntl27.c fcntl28.c 
2,5c2
<  *
<  *   Copyright (C) Bull S.A. 2005
<  *   Copyright (c) International Business Machines  Corp., 2004
<  *
---
>  *   Copyright (C) Bull S.A.S  2006
23c20
<  *    TEST IDENTIFIER          : fcntl27
---
>  *    TEST IDENTIFIER          : fcntl28
101c98
< char *TCID = "fcntl27";
---
> char *TCID = "fcntl28";
143a141
> 
170c168
<       if ((fd = open(fname, O_RDWR | O_CREAT, 0777)) == -1) {
---
>       if ((fd = open(fname, O_WRONLY | O_CREAT, 0222)) == -1) {
172c170
<                        "open(%s, O_RDWR|O_CREAT,0777) Failed, errno=%d : %s",
---
>                        "open(%s, O_WRONLY|O_CREAT,0222) Failed, errno=%d : %s",

[metan-ucw]

You have to update the runtest files after test removal as well. In this case the runtest/syscalls file.

[ilmanzo]

You have to update the runtest files after test removal as well. In this case the runtest/syscalls file.

good point, thanks! do you prefer to have separate commits or a single squashed one ?

[Avinesh]

You have to update the runtest files after test removal as well. In this case the runtest/syscalls file.

good point, thanks! do you prefer to have separate commits or a single squashed one ?

Ideally it should be single commit.

[Avinesh]

I think we should improve the test description with what error scenario we are exactly testing here.

Maybe something like- "Verify that, when trying to take out a read lease using F_SETLEASE and F_RDLCK, but the file descriptor is not opened read‐only, fcntl() call fails with errno EAGAIN."

But fcntl() manpage does not explicitly mention this, so I am not sure what I wrote above is correct. @pevik over to you :)

[ilmanzo]

But fcntl() manpage does not explicitly mention this, so I am not sure what I wrote above is correct. @pevik over to you :)

yeah fcntl() man page seems not explicitly telling which errno we should expect. The original old tests only checked for return code == -1 . Should we do the same ?

[Avinesh]

But fcntl() manpage does not explicitly mention this, so I am not sure what I wrote above is correct. @pevik over to you :)

yeah fcntl() man page seems not explicitly telling which errno we should expect. The original old tests only checked for return code == -1 . Should we do the same ?

No, we should check the expected return value and errno both. I checked the kernel code for fcntl, I am not sure of code flow, but looks like it indeed sets the errno to EAGAIN when given an inode which is open for write. (in fs/locks.c)

[metan-ucw]

@Avinesh is right here. Unfortunately man pages are not complete enough and there is a lot of things undocumented, I try to help by sending man page patches when I find something like this during test development. However even when things are not documented in man-pages it does not change the fact that this is an kernel-userspace API that shouldn't change on whim, so we better assert that the call sets correct errno as well.

[pevik]

Merged with slightly modified version (added printing test parameters, if one runs with high -i, it would not be obvious the whether the failure was for read or write). Thanks!