[LTP] Fwd: pwrite04 ltp bug?

[LTP] Fwd: pwrite04 ltp bug?

[Mike Marshall]

Hi...

I think I found a bug in pwrite04.c. I mentioned it on the linux
filesystem mailing list and it was suggested that I mention it to
you...

-Mike

Forwarded conversation
Subject: pwrite04 ltp bug?
------------------------

From: *Mike Marshall* <hubcap@omnibond.com>
Date: Fri, May 20, 2016 at 4:51 PM
To: linux-fsdevel <linux-fsdevel@vger.kernel.org>, Mike Marshall <
hubcap@omnibond.com>


Hi everyone...

While fixing a couple of LTP regressions in the out-of-tree Orangefs
kernel module, I think I found a problem with the pwrite04 test,
here's what I think was meant:

# git diff testcases/kernel/syscalls/pwrite/pwrite04.c
diff --git a/testcases/kernel/syscalls/pwrite/pwrite04.c
b/testcases/kernel/syscalls/pwrite/pwrite04.c
index b297682..4ca7dc1 100644
--- a/testcases/kernel/syscalls/pwrite/pwrite04.c
+++ b/testcases/kernel/syscalls/pwrite/pwrite04.c
@@ -196,9 +196,9 @@ int main(int ac, char *av[])

                }
                if (statbuf.st_size != K5) {
-                       tst_resm(TFAIL, "file size is %ld != K4",
+                       tst_resm(TFAIL, "file size is %ld != K5",
                                 statbuf.st_size);
-
+                       cleanup();
                }
                tst_resm(TPASS, "O_APPEND test passed.");

Should I even be talking about this here, or should I go find
the github maintainers of LTP and ask them about it?

-Mike

----------
From: *Zorro Lang* <zorro.lang@gmail.com>
Date: Fri, May 20, 2016 at 8:56 PM
To: Mike Marshall <hubcap@omnibond.com>



2016年5月21日 04:52，"Mike Marshall" <hubcap@omnibond.com>写道：
>
> Hi everyone...
>
> While fixing a couple of LTP regressions in the out-of-tree Orangefs
> kernel module, I think I found a problem with the pwrite04 test,
> here's what I think was meant:
>
> # git diff testcases/kernel/syscalls/pwrite/pwrite04.c
> diff --git a/testcases/kernel/syscalls/pwrite/pwrite04.c
> b/testcases/kernel/syscalls/pwrite/pwrite04.c
> index b297682..4ca7dc1 100644
> --- a/testcases/kernel/syscalls/pwrite/pwrite04.c
> +++ b/testcases/kernel/syscalls/pwrite/pwrite04.c
> @@ -196,9 +196,9 @@ int main(int ac, char *av[])
>
>                 }
>                 if (statbuf.st_size != K5) {
> -                       tst_resm(TFAIL, "file size is %ld != K4",
> +                       tst_resm(TFAIL, "file size is %ld != K5",
>                                  statbuf.st_size);
> -
> +                       cleanup();
>                 }
>                 tst_resm(TPASS, "O_APPEND test passed.");
>
> Should I even be talking about this here, or should I go find
> the github maintainers of LTP and ask them about it?

Hi Mike,

Generally, we send LTP related patches to: ltp@lists.linux.it

Thanks,
Zorro

>
> -Mike
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel"
in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20160520/a1d8e5d4/attachment.html>

[LTP] Fwd: pwrite04 ltp bug?

[Cyril Hrubis]

Hi!
> While fixing a couple of LTP regressions in the out-of-tree Orangefs
> kernel module, I think I found a problem with the pwrite04 test,
> here's what I think was meant:
> 
> # git diff testcases/kernel/syscalls/pwrite/pwrite04.c
> diff --git a/testcases/kernel/syscalls/pwrite/pwrite04.c
> b/testcases/kernel/syscalls/pwrite/pwrite04.c
> index b297682..4ca7dc1 100644
> --- a/testcases/kernel/syscalls/pwrite/pwrite04.c
> +++ b/testcases/kernel/syscalls/pwrite/pwrite04.c
> @@ -196,9 +196,9 @@ int main(int ac, char *av[])
> 
>                 }
>                 if (statbuf.st_size != K5) {
> -                       tst_resm(TFAIL, "file size is %ld != K4",
> +                       tst_resm(TFAIL, "file size is %ld != K5",
>                                  statbuf.st_size);

This is obvious typo fix.

> -
> +                       cleanup();

This does not look right. Looking at the test source the rest of the ifs
has the cleanup right after tst_resm() but the cleanup() fuction is
supposed to be called exactly once at the end of the test at the end of
the main.

If it's called in the testing loop the test will fail when cleanup() is
invoked second time at the test exit since it will attempt to remove
allready deleted test temporary directory. So the correct solution to
this problem would rather be to remove the cleanup() from the test loop.

>                 }
>                 tst_resm(TPASS, "O_APPEND test passed.");

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] Fwd: pwrite04 ltp bug?

[Mike Marshall]

 > This does not look right...

OK, I was just copying the file size test
loop above the "Finally" comment, I assumed
that cleanup exited. As it is, if you fail the
file size loop I edited,  you'll get both a FAIL
and a PASS...

Maybe in all the cases that resolve to FAIL
we should goto some kind of out-fail: ?

Thanks for looking...

-Mike

On Mon, May 23, 2016 at 8:39 AM, Cyril Hrubis <chrubis@suse.cz> wrote:
> Hi!
>> While fixing a couple of LTP regressions in the out-of-tree Orangefs
>> kernel module, I think I found a problem with the pwrite04 test,
>> here's what I think was meant:
>>
>> # git diff testcases/kernel/syscalls/pwrite/pwrite04.c
>> diff --git a/testcases/kernel/syscalls/pwrite/pwrite04.c
>> b/testcases/kernel/syscalls/pwrite/pwrite04.c
>> index b297682..4ca7dc1 100644
>> --- a/testcases/kernel/syscalls/pwrite/pwrite04.c
>> +++ b/testcases/kernel/syscalls/pwrite/pwrite04.c
>> @@ -196,9 +196,9 @@ int main(int ac, char *av[])
>>
>>                 }
>>                 if (statbuf.st_size != K5) {
>> -                       tst_resm(TFAIL, "file size is %ld != K4",
>> +                       tst_resm(TFAIL, "file size is %ld != K5",
>>                                  statbuf.st_size);
>
> This is obvious typo fix.
>
>> -
>> +                       cleanup();
>
> This does not look right. Looking at the test source the rest of the ifs
> has the cleanup right after tst_resm() but the cleanup() fuction is
> supposed to be called exactly once at the end of the test at the end of
> the main.
>
> If it's called in the testing loop the test will fail when cleanup() is
> invoked second time at the test exit since it will attempt to remove
> allready deleted test temporary directory. So the correct solution to
> this problem would rather be to remove the cleanup() from the test loop.
>
>>                 }
>>                 tst_resm(TPASS, "O_APPEND test passed.");
>
> --
> Cyril Hrubis
> chrubis@suse.cz

[LTP] Fwd: pwrite04 ltp bug?

[Cyril Hrubis]

Hi!
>  > This does not look right...
> 
> OK, I was just copying the file size test
> loop above the "Finally" comment, I assumed
> that cleanup exited. As it is, if you fail the
> file size loop I edited,  you'll get both a FAIL
> and a PASS...
> 
> Maybe in all the cases that resolve to FAIL
> we should goto some kind of out-fail: ?

The best solution would to move the actual test to a separate function
called from the for() loop in main(), then we can just do return in case
of failure.

-- 
Cyril Hrubis
chrubis@suse.cz