[LTP] [PATCH v2] fstat_02: Increase test coverage by creating hard link to file and validate using fstat

[LTP] [PATCH v2] fstat_02: Increase test coverage by creating hard link to file and validate using fstat

[Kushal Chand]

Implements: #517

This patch creates a hard link for a file during setup and checks if number of hardlinks
match with the expected number.
---
 testcases/kernel/syscalls/fstat/fstat02.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/testcases/kernel/syscalls/fstat/fstat02.c b/testcases/kernel/syscalls/fstat/fstat02.c
index c0229de44..2f9632edf 100644
--- a/testcases/kernel/syscalls/fstat/fstat02.c
+++ b/testcases/kernel/syscalls/fstat/fstat02.c
@@ -17,8 +17,10 @@
 #include "tst_safe_macros.h"
 
 #define TESTFILE        "test_file"
+#define LINK_TESTFILE   "link_test_file"
 #define FILE_SIZE       1024
 #define FILE_MODE	0644
+#define NLINK	        2
 
 static struct stat stat_buf;
 static uid_t user_id;
@@ -61,6 +63,12 @@ static void run(void)
 		fail++;
 	}
 
+	if (stat_buf.st_nlink != NLINK) {
+		tst_res(TFAIL, "stat_buf.st_nlink = %li expected %i",
+			(long)stat_buf.st_nlink, NLINK);
+		fail++;
+	}
+
 	if (fail)
 		return;
 
@@ -78,6 +86,8 @@ static void setup(void)
 
 	if (tst_fill_file(TESTFILE, 'a', FILE_SIZE, 1))
 		tst_brk(TBROK, "Could not fill Testfile!");
+
+	SAFE_LINK(TESTFILE, LINK_TESTFILE);
 }
 
 static void cleanup(void)
-- 
2.25.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] fstat_02: Increase test coverage by creating hard link to file and validate using fstat

[Cyril Hrubis]

Hi!
The subject should be shorter and more to the point, something as:

"fstat02: Validate st_nlink as well"

> Implements: #517

The code looks good now, but it's not complete solution to #517, there
are still many fields of the structure that are not checked and a few
more patches would be required to complete it.

For instance the st_blocks should be more or less equal size/512

And we should check the atime/mtime/ctime as well, but maybe it would be
easier if we do that in a separate test.

> This patch creates a hard link for a file during setup and checks if number of hardlinks
> match with the expected number.

This is missing Signed-off-by: line see:

https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin

> ---
>  testcases/kernel/syscalls/fstat/fstat02.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/testcases/kernel/syscalls/fstat/fstat02.c b/testcases/kernel/syscalls/fstat/fstat02.c
> index c0229de44..2f9632edf 100644
> --- a/testcases/kernel/syscalls/fstat/fstat02.c
> +++ b/testcases/kernel/syscalls/fstat/fstat02.c
> @@ -17,8 +17,10 @@
>  #include "tst_safe_macros.h"
>  
>  #define TESTFILE        "test_file"
> +#define LINK_TESTFILE   "link_test_file"
>  #define FILE_SIZE       1024
>  #define FILE_MODE	0644
> +#define NLINK	        2
>  
>  static struct stat stat_buf;
>  static uid_t user_id;
> @@ -61,6 +63,12 @@ static void run(void)
>  		fail++;
>  	}
>  
> +	if (stat_buf.st_nlink != NLINK) {
> +		tst_res(TFAIL, "stat_buf.st_nlink = %li expected %i",
> +			(long)stat_buf.st_nlink, NLINK);
> +		fail++;
> +	}
> +
>  	if (fail)
>  		return;
>  
> @@ -78,6 +86,8 @@ static void setup(void)
>  
>  	if (tst_fill_file(TESTFILE, 'a', FILE_SIZE, 1))
>  		tst_brk(TBROK, "Could not fill Testfile!");
> +
> +	SAFE_LINK(TESTFILE, LINK_TESTFILE);
>  }
>  
>  static void cleanup(void)
> -- 
> 2.25.1
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] fstat_02: Increase test coverage by creating hard link to file and validate using fstat

[Kushal Chand]

[-- Attachment #1.1: Type: text/plain, Size: 4908 bytes --]

Hi,

Apology for the late reply.

I am not sure about the st_blocks size. If you can guide me how to check
that I might work on that?

Are you planning to merge this patch with the st_blocks check or the
current version is ready to be merged?

On Thu, Feb 10, 2022 at 9:04 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> Hi!
> The subject should be shorter and more to the point, something as:
>
> "fstat02: Validate st_nlink as well"
>
> > Implements: #517
>
> The code looks good now, but it's not complete solution to #517, there
> are still many fields of the structure that are not checked and a few
> more patches would be required to complete it.
>
> For instance the st_blocks should be more or less equal size/512
>
> And we should check the atime/mtime/ctime as well, but maybe it would be
> easier if we do that in a separate test.
>
> > This patch creates a hard link for a file during setup and checks if
> number of hardlinks
> > match with the expected number.
>
> This is missing Signed-off-by: line see:
>
>
> https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
>
> > ---
> >  testcases/kernel/syscalls/fstat/fstat02.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/testcases/kernel/syscalls/fstat/fstat02.c
> b/testcases/kernel/syscalls/fstat/fstat02.c
> > index c0229de44..2f9632edf 100644
> > --- a/testcases/kernel/syscalls/fstat/fstat02.c
> > +++ b/testcases/kernel/syscalls/fstat/fstat02.c
> > @@ -17,8 +17,10 @@
> >  #include "tst_safe_macros.h"
> >
> >  #define TESTFILE        "test_file"
> > +#define LINK_TESTFILE   "link_test_file"
> >  #define FILE_SIZE       1024
> >  #define FILE_MODE    0644
> > +#define NLINK                2
> >
> >  static struct stat stat_buf;
> >  static uid_t user_id;
> > @@ -61,6 +63,12 @@ static void run(void)
> >               fail++;
> >       }
> >
> > +     if (stat_buf.st_nlink != NLINK) {
> > +             tst_res(TFAIL, "stat_buf.st_nlink = %li expected %i",
> > +                     (long)stat_buf.st_nlink, NLINK);
> > +             fail++;
> > +     }
> > +
> >       if (fail)
> >               return;
> >
> > @@ -78,6 +86,8 @@ static void setup(void)
> >
> >       if (tst_fill_file(TESTFILE, 'a', FILE_SIZE, 1))
> >               tst_brk(TBROK, "Could not fill Testfile!");
> > +
> > +     SAFE_LINK(TESTFILE, LINK_TESTFILE);
> >  }
> >
> >  static void cleanup(void)
> > --
> > 2.25.1
> >
> >
> > --
> > Mailing list info: https://lists.linux.it/listinfo/ltp
>
> --
> Cyril Hrubis
> chrubis@suse.cz


On Thu, Feb 10, 2022 at 9:04 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> Hi!
> The subject should be shorter and more to the point, something as:
>
> "fstat02: Validate st_nlink as well"
>
> > Implements: #517
>
> The code looks good now, but it's not complete solution to #517, there
> are still many fields of the structure that are not checked and a few
> more patches would be required to complete it.
>
> For instance the st_blocks should be more or less equal size/512
>
> And we should check the atime/mtime/ctime as well, but maybe it would be
> easier if we do that in a separate test.
>
> > This patch creates a hard link for a file during setup and checks if
> number of hardlinks
> > match with the expected number.
>
> This is missing Signed-off-by: line see:
>
>
> https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
>
> > ---
> >  testcases/kernel/syscalls/fstat/fstat02.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/testcases/kernel/syscalls/fstat/fstat02.c
> b/testcases/kernel/syscalls/fstat/fstat02.c
> > index c0229de44..2f9632edf 100644
> > --- a/testcases/kernel/syscalls/fstat/fstat02.c
> > +++ b/testcases/kernel/syscalls/fstat/fstat02.c
> > @@ -17,8 +17,10 @@
> >  #include "tst_safe_macros.h"
> >
> >  #define TESTFILE        "test_file"
> > +#define LINK_TESTFILE   "link_test_file"
> >  #define FILE_SIZE       1024
> >  #define FILE_MODE    0644
> > +#define NLINK                2
> >
> >  static struct stat stat_buf;
> >  static uid_t user_id;
> > @@ -61,6 +63,12 @@ static void run(void)
> >               fail++;
> >       }
> >
> > +     if (stat_buf.st_nlink != NLINK) {
> > +             tst_res(TFAIL, "stat_buf.st_nlink = %li expected %i",
> > +                     (long)stat_buf.st_nlink, NLINK);
> > +             fail++;
> > +     }
> > +
> >       if (fail)
> >               return;
> >
> > @@ -78,6 +86,8 @@ static void setup(void)
> >
> >       if (tst_fill_file(TESTFILE, 'a', FILE_SIZE, 1))
> >               tst_brk(TBROK, "Could not fill Testfile!");
> > +
> > +     SAFE_LINK(TESTFILE, LINK_TESTFILE);
> >  }
> >
> >  static void cleanup(void)
> > --
> > 2.25.1
> >
> >
> > --
> > Mailing list info: https://lists.linux.it/listinfo/ltp
>
> --
> Cyril Hrubis
> chrubis@suse.cz
>

[-- Attachment #1.2: Type: text/html, Size: 7067 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] fstat_02: Increase test coverage by creating hard link to file and validate using fstat

[Cyril Hrubis]

Hi!
> Apology for the late reply.
> 
> I am not sure about the st_blocks size. If you can guide me how to check
> that I might work on that?

Just read the man 2 stat:

...
               blkcnt_t  st_blocks;      /* Number of 512B blocks allocated */
...

For large enough files the size/512 rounded up should be close to the
number of blocks (there would be usually a few more blocks preallocated
for the file than the actuall size is). Filesystems tend to allocate
more than one block when file needs to be extended, it looks like common
value is PAGE_SIZE/512. So I guess that a reasonable test would create a
few files somewhere between half a megabyte and a few megabytes in size
and then check that the size/blocks ratio is somewhere between 512 and
512-epsilon. As long as the file size is order of magnitude larger than
PAGE_SIZE it should be safe to set the epsion to 10% of the blocksize
i.e. 51.

> Are you planning to merge this patch with the st_blocks check or the
> current version is ready to be merged?

The patch has been already merged, I was just pointing out other fields
that are not validated in the test.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp