| Message ID | 20220210105101.38337-1-kushalkataria5@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2] fstat_02: Increase test coverage by creating hard link to file and validate using fstat | expand |
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
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 >
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.
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)