[LTP] [PATCH 2/2-updated] syscalls/fstatfs02: Using temporary file description to test the syscall

[LTP] [PATCH 2/2-updated] syscalls/fstatfs02: Using temporary file description to test the syscall

[shuang.qiu]

From: Shuang Qiu <shuang.qiu@oracle.com>

It does not make sense to use stdout as the file description to test the
system call fstatfs().We may get unexpected result(i.e.
errno=38:Function not implemented) if redirect stdout when running ltp.
Using ltp temporary file instead.

Signed-off-by: Shuang Qiu <shuang.qiu@oracle.com>
---
 testcases/kernel/syscalls/fstatfs/fstatfs02.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/testcases/kernel/syscalls/fstatfs/fstatfs02.c b/testcases/kernel/syscalls/fstatfs/fstatfs02.c
index b243af3..88376a1 100644
--- a/testcases/kernel/syscalls/fstatfs/fstatfs02.c
+++ b/testcases/kernel/syscalls/fstatfs/fstatfs02.c
@@ -23,6 +23,7 @@
 
 #include <sys/vfs.h>
 #include <sys/types.h>
+#include <sys/fcntl.h>
 #include <sys/statfs.h>
 #include <errno.h>
 #include "test.h"
@@ -35,6 +36,7 @@ char *TCID = "fstatfs02";
 
 static int exp_enos[] = { EBADF, EFAULT, 0 };
 
+static char fname[255];
 static struct statfs buf;
 
 static struct test_case_t {
@@ -48,6 +50,7 @@ static struct test_case_t {
 #ifndef UCLINUX
 	    /* Skip since uClinux does not implement memory protection */
 	    /* EFAULT - address for buf is invalid */
+	    /* Use the init "1" as fd may cause unexpected ENOSYS error,will assign a tmp file later */
 	{
 	1, (void *)-1, EFAULT}
 #endif
@@ -109,11 +112,22 @@ static void setup(void)
 	TEST_PAUSE;
 
 	tst_tmpdir();
+
+#ifndef UCLINUX
+	sprintf(fname, "tfile_%d", getpid());
+	if ((TC[1].fd = open(fname, O_RDWR | O_CREAT, 0700)) == -1)
+		tst_brkm(TBROK | TERRNO, cleanup, "open failed");
+#endif
 }
 
 static void cleanup(void)
 {
 	TEST_CLEANUP;
 
+#ifndef UCLINUX
+	if (close(TC[1].fd) == -1)
+		tst_resm(TWARN | TERRNO, "close failed");
+#endif
+
 	tst_rmdir();
 }
-- 
1.7.7


------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.  Get 
unparalleled scalability from the best Selenium testing platform available.
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 2/2-updated] syscalls/fstatfs02: Using temporary file description to test the syscall

[Stanislav Kholmanskikh]

On 04/30/2014 02:57 PM, shuang.qiu@oracle.com wrote:
> From: Shuang Qiu <shuang.qiu@oracle.com>
>
> It does not make sense to use stdout as the file description to test the
> system call fstatfs().We may get unexpected result(i.e.
> errno=38:Function not implemented) if redirect stdout when running ltp.
> Using ltp temporary file instead.
>
> Signed-off-by: Shuang Qiu <shuang.qiu@oracle.com>

The problem is that with the 2.6.39 series of kernels ENOSYS is returned 
where EFAULT is expected.

The reproducer:
#include <error.h>
#include <stdio.h>
#include <unistd.h>
#include <sys/vfs.h>

int main(void)
{
   int fds[2];

   if (pipe(fds) < 0) {
     perror("pipe() failed");
     return 1;
   }

   if (fstatfs(fds[1], (void *)-1) < 0) {
     perror("fstatfs() failed");
     return 1;
   }

   return 0;
}

With 2.6.18, 2.6.32, 3.8.13 this reproducer returns EFAULT,
but with 2.6.39 - ENOSYS.

If we change '(void *)-1' to a vaild pointer, fstatfs() + 2.6.39 will 
succeed.

So it's a bug in 2.6.39, which is fixed in modern kernels.

 From one side, the above patch makes fstatfs02 pass by hiding an old 
(and already fixed) kernel bug.

But from the other side, it also enlarges the test scope by allowing to 
test different file systems.

So, personally, I would accept it.

Are there any thoughts about it?

Thanks.

------------------------------------------------------------------------------
Is your legacy SCM system holding you back? Join Perforce May 7 to find out:
&#149; 3 signs your SCM is hindering your productivity
&#149; Requirements for releasing software faster
&#149; Expert tips and advice for migrating your SCM now
http://p.sf.net/sfu/perforce
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 2/2-updated] syscalls/fstatfs02: Using temporary file description to test the syscall

[Stanislav Kholmanskikh]

On 05/06/2014 03:00 PM, Stanislav Kholmanskikh wrote:
>
>
> On 04/30/2014 02:57 PM, shuang.qiu@oracle.com wrote:
>> From: Shuang Qiu <shuang.qiu@oracle.com>
>>
>> It does not make sense to use stdout as the file description to test the
>> system call fstatfs().We may get unexpected result(i.e.
>> errno=38:Function not implemented) if redirect stdout when running ltp.
>> Using ltp temporary file instead.
>>
>> Signed-off-by: Shuang Qiu <shuang.qiu@oracle.com>
>
> The problem is that with the 2.6.39 series of kernels ENOSYS is returned
> where EFAULT is expected.
>
> The reproducer:
> #include <error.h>
> #include <stdio.h>
> #include <unistd.h>
> #include <sys/vfs.h>
>
> int main(void)
> {
>     int fds[2];
>
>     if (pipe(fds) < 0) {
>       perror("pipe() failed");
>       return 1;
>     }
>
>     if (fstatfs(fds[1], (void *)-1) < 0) {
>       perror("fstatfs() failed");
>       return 1;
>     }
>
>     return 0;
> }
>
> With 2.6.18, 2.6.32, 3.8.13 this reproducer returns EFAULT,
> but with 2.6.39 - ENOSYS.
>
> If we change '(void *)-1' to a vaild pointer, fstatfs() + 2.6.39 will
> succeed.
>
> So it's a bug in 2.6.39, which is fixed in modern kernels.
>
>   From one side, the above patch makes fstatfs02 pass by hiding an old
> (and already fixed) kernel bug.
>
> But from the other side, it also enlarges the test scope by allowing to
> test different file systems.
>
> So, personally, I would accept it.

I'm not convinced that it's worth to add a new scenario to test fstatfs 
+ pipe file descriptor + invalid address....

>
> Are there any thoughts about it?
>
> Thanks.
>
> ------------------------------------------------------------------------------
> Is your legacy SCM system holding you back? Join Perforce May 7 to find out:
> &#149; 3 signs your SCM is hindering your productivity
> &#149; Requirements for releasing software faster
> &#149; Expert tips and advice for migrating your SCM now
> http://p.sf.net/sfu/perforce
> _______________________________________________
> Ltp-list mailing list
> Ltp-list@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ltp-list
>

------------------------------------------------------------------------------
Is your legacy SCM system holding you back? Join Perforce May 7 to find out:
&#149; 3 signs your SCM is hindering your productivity
&#149; Requirements for releasing software faster
&#149; Expert tips and advice for migrating your SCM now
http://p.sf.net/sfu/perforce
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 2/2-updated] syscalls/fstatfs02: Using temporary file description to test the syscall

[chrubis]

Hi!
> > It does not make sense to use stdout as the file description to test the
> > system call fstatfs().We may get unexpected result(i.e.
> > errno=38:Function not implemented) if redirect stdout when running ltp.
> > Using ltp temporary file instead.
> >
> > Signed-off-by: Shuang Qiu <shuang.qiu@oracle.com>
> 
> The problem is that with the 2.6.39 series of kernels ENOSYS is returned 
> where EFAULT is expected.
> 
> The reproducer:
> #include <error.h>
> #include <stdio.h>
> #include <unistd.h>
> #include <sys/vfs.h>
> 
> int main(void)
> {
>    int fds[2];
> 
>    if (pipe(fds) < 0) {
>      perror("pipe() failed");
>      return 1;
>    }
> 
>    if (fstatfs(fds[1], (void *)-1) < 0) {
>      perror("fstatfs() failed");
>      return 1;
>    }
> 
>    return 0;
> }
> 
> With 2.6.18, 2.6.32, 3.8.13 this reproducer returns EFAULT,
> but with 2.6.39 - ENOSYS.
> 
> If we change '(void *)-1' to a vaild pointer, fstatfs() + 2.6.39 will 
> succeed.
> 
> So it's a bug in 2.6.39, which is fixed in modern kernels.

I've looked at fstatfs() and pipe() code in recent kernel.

The fstatfs() looks at dentry and if there is no statfs callback it
returns ENOSYS, otherwise it uses the callback to fill the buffer.

The pipe() mounts an "pipe:" kernel internal pseudo fs and uses
simple_statfs() for the statfs() callback. So statfs() works fine on
recent kernels. In 2.6.39 the pipefs_ops structure simply misses the
fstatfs() callback, it has been introduced in:

commit d70ef97baf048412c395bb5d65791d8fe133a52b
Author: Pavel Emelyanov <xemul@parallels.com>
Date:   Mon Oct 31 17:10:04 2011 -0700

    fs/pipe.c: add ->statfs callback for pipefs

I guess that we can add a test for fstatfs() on a pipe if kernel version
is newer than 3.2.

>  From one side, the above patch makes fstatfs02 pass by hiding an old 
> (and already fixed) kernel bug.
> 
> But from the other side, it also enlarges the test scope by allowing to 
> test different file systems.
> 
> So, personally, I would accept it.
> 
> Are there any thoughts about it?

I'm for fixing the testcase this way. If nothing else it makes it the
test actually test what it's expected to test.

Just cleanup the patch a bit before applying (use SAFE_OPEN(), etc).

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Time is money. Stop wasting it! Get your web API in 5 minutes.
www.restlet.com/download
http://p.sf.net/sfu/restlet
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list