[LTP] [PATCH] diotest: set errno with zero before test with zero

[LTP] [PATCH] diotest: set errno with zero before test with zero

[Zorro Lang]

some code logic in diotest4.c likes below:
----
    ret = lseek(fd, offset, SEEK_SET);
    if ((ret >= 0) || (errno != EINVAL)) {
        tst_resm(TFAIL, "lseek allows negative offset. returns %d:%s", ret, strerror(errno));
        failed = TRUE;
        fail_count++;
    } else
        tst_resm(TPASS, "Negative Offset");

    ....
    ....

    ret = read(fd, buf, count);
    if (ret >= 0 || errno != errnum) {
        tst_resm(TFAIL, "read allows %s. returns %d: %s", msg, ret, strerror(errno));
        l_fail = TRUE;
    }
----

If lseek() EINVAL and read() return >= 0, the errno will not be setted to 0. The
errno will still be EINVAL.

I hit this problem when test on nfs. In NFS, lseek() return EINVAL as excepted,
but the read() will return 1, when count = 1. But the errno is still EINVAL.
That's incorrect.

So I set errno = 0, before read() and all other functions which like this.

Signed-off-by: Zorro Lang <zlang@redhat.com>
---

Hi,

The problem likes above commit messge. Please review.

Thank you,
Zorro Lang

 testcases/kernel/io/direct_io/diotest4.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/testcases/kernel/io/direct_io/diotest4.c b/testcases/kernel/io/direct_io/diotest4.c
index fa90a1e..82c5b33 100644
--- a/testcases/kernel/io/direct_io/diotest4.c
+++ b/testcases/kernel/io/direct_io/diotest4.c
@@ -107,6 +107,7 @@ runtest_f(int fd, char *buf, int offset, int count, int errnum, int testnum,
 			l_fail = TRUE;
 		}
 	} else {
+		errno = 0;
 		ret = read(fd, buf, count);
 		if (ret >= 0 || errno != errnum) {
 			tst_resm(TFAIL, "read allows %s. returns %d: %s",
@@ -121,6 +122,7 @@ runtest_f(int fd, char *buf, int offset, int count, int errnum, int testnum,
 			l_fail = TRUE;
 		}
 	} else {
+		errno = 0;
 		ret = write(fd, buf, count);
 		if (ret >= 0 || errno != errnum) {
 			tst_resm(TFAIL, "write allows %s.returns %d: %s",
@@ -247,6 +249,7 @@ int main(int argc, char *argv[])
 	/* Test-1: Negative Offset */
 	offset = -1;
 	count = bufsize;
+	errno = 0;
 	ret = lseek(fd, offset, SEEK_SET);
 	if ((ret >= 0) || (errno != EINVAL)) {
 		tst_resm(TFAIL, "lseek allows negative offset. returns %d:%s",
@@ -284,6 +287,7 @@ int main(int argc, char *argv[])
 		fail_count++;
 		tst_resm(TFAIL, "Read beyond the file size");
 	} else {
+		errno = 0;
 		ret = read(fd, buf2, count);
 		if (ret > 0 || (ret < 0 && errno != EINVAL)) {
 			tst_resm(TFAIL,
@@ -392,6 +396,7 @@ int main(int argc, char *argv[])
 		failed = TRUE;
 		fail_count++;
 	} else {
+		errno = 0;
 		ret = read(fd, buf2, count);
 		if (ret >= 0 || errno != EBADF) {
 			tst_resm(TFAIL,
@@ -417,6 +422,7 @@ int main(int argc, char *argv[])
 		failed = TRUE;
 		fail_count++;
 	} else {
+		errno = 0;
 		ret = write(fd, buf2, count);
 		if (ret >= 0 || errno != EBADF) {
 			tst_resm(TFAIL,
@@ -460,6 +466,7 @@ int main(int argc, char *argv[])
 			 strerror(errno));
 		l_fail = TRUE;
 	} else {
+		errno = 0;
 		ret = read(fd, (char *)((ulong) ADDRESS_OF_MAIN & pagemask),
 			 count);
 		if (ret >= 0 || errno != EFAULT) {
-- 
1.9.3


------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] diotest: set errno with zero before test with zero

[Cyril Hrubis]

Hi!
>     ret = read(fd, buf, count);
>     if (ret >= 0 || errno != errnum) {
>         tst_resm(TFAIL, "read allows %s. returns %d: %s", msg, ret, strerror(errno));
>         l_fail = TRUE;

As far as I can see this code tests that read() has returned negative
value and in this case errno must be set.

I.e.

if ret >= 0 -> read hasn't failed -> FAILURE

The errno != errnum is not evaluated unless the first condition has
failed (that means ret < 0) and in this case errno must be set.

The condition is equivalent to !(ret < 0 && errno == errnum)

Or am I missing something?

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] diotest: set errno with zero before test with zero

[Cyril Hrubis]

Hi!
> > >     ret = read(fd, buf, count);
> > >     if (ret >= 0 || errno != errnum) {
> > >         tst_resm(TFAIL, "read allows %s. returns %d: %s", msg, ret,
> > >         strerror(errno));
> > >         l_fail = TRUE;
> > 
> > As far as I can see this code tests that read() has returned negative
> > value and in this case errno must be set.
> > 
> > I.e.
> > 
> > if ret >= 0 -> read hasn't failed -> FAILURE
> > 
> > The errno != errnum is not evaluated unless the first condition has
> > failed (that means ret < 0) and in this case errno must be set.
> > 
> > The condition is equivalent to !(ret < 0 && errno == errnum)
> > 
> > Or am I missing something?
> 
> I think he refers to output of tst_resm being possibly misleading
> in case ret >= 0, that call is using strerror(errno).

Ah, now that makes sense.

Looks like a reasonable solution. Any other solution that I can think of
would add one more if statement...

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] diotest: set errno with zero before test with zero

[Cyril Hrubis]

Hi!
> some code logic in diotest4.c likes below:
> ----
>     ret = lseek(fd, offset, SEEK_SET);
>     if ((ret >= 0) || (errno != EINVAL)) {
>         tst_resm(TFAIL, "lseek allows negative offset. returns %d:%s", ret, strerror(errno));
>         failed = TRUE;
>         fail_count++;
>     } else
>         tst_resm(TPASS, "Negative Offset");
> 
>     ....
>     ....
> 
>     ret = read(fd, buf, count);
>     if (ret >= 0 || errno != errnum) {
>         tst_resm(TFAIL, "read allows %s. returns %d: %s", msg, ret, strerror(errno));
>         l_fail = TRUE;
>     }
> ----
> 
> If lseek() EINVAL and read() return >= 0, the errno will not be setted to 0. The
> errno will still be EINVAL.
> 
> I hit this problem when test on nfs. In NFS, lseek() return EINVAL as excepted,
> but the read() will return 1, when count = 1. But the errno is still EINVAL.
> That's incorrect.
> 
> So I set errno = 0, before read() and all other functions which like this.

Can you please clarify the commit message a bit so that it's clear that
it's the test failure message that is confusing in case that read()
haven't failed as expected and the errno is still set?

Otherwise it looks fine.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list