| Message ID | 1661329049-14309-1-git-send-email-liaohj.jy@fujitsu.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | syscalls/read04:add multiple read size | expand |
Hi Liao It improves test coverage a lot, thanks! LGTM, Reviewed-by: Yang Xu <xuyang2018.jy@fujitsu.com> Best Regards Yang Xu > From: Huangjie Liao <liaohj.jy@fujitsu.com> > > Signed-off-by: Huangjie Liao <liaohj.jy@fujitsu.com> > --- > testcases/kernel/syscalls/read/read04.c | 59 ++++++++++++++++++++++++--------- > 1 file changed, 43 insertions(+), 16 deletions(-) > > diff --git a/testcases/kernel/syscalls/read/read04.c b/testcases/kernel/syscalls/read/read04.c > index 47875c0..37c670f 100644 > --- a/testcases/kernel/syscalls/read/read04.c > +++ b/testcases/kernel/syscalls/read/read04.c > @@ -6,36 +6,49 @@ > /*\ > * [Description] > * > - * Testcase to check if read() returns the number of bytes read correctly. > + * Testcase to check if read() returns the correct number of bytes > + * when using multip sizes [0, 1/2*page_size, 3/2*page_size, 2*page_size]. > */ > > #include <sys/types.h> > #include <sys/stat.h> > #include <stdio.h> > #include <fcntl.h> > +#include <unistd.h> > #include "tst_test.h" > > +#define MNT_POINT "mntpoint" > +static int page_size; > static const char *fname = "test_file"; > -static const char palfa[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; > -#define PALFA_LEN (sizeof(palfa)-1) > +static char *write_buf[2], *read_buf; > > -static void verify_read(void) > +static void verify_read(unsigned int n) > { > int fd; > - char prbuf[BUFSIZ]; > - > + int size = n * page_size / 2; > fd = SAFE_OPEN(fname, O_RDONLY); > - TEST(read(fd, prbuf, BUFSIZ)); > + TEST(read(fd, read_buf, size)); > > - if (TST_RET != PALFA_LEN) { > - tst_res(TFAIL, "Bad read count - got %ld - expected %zu", > - TST_RET, PALFA_LEN); > + if (TST_RET != size) { > + tst_res(TFAIL, "Bad read count - got %ld - expected %d", > + TST_RET, size); > goto out; > } > > - if (memcmp(palfa, prbuf, PALFA_LEN)) { > - tst_res(TFAIL, "read buffer not equal to write buffer"); > - goto out; > + if (n <= 2) { > + if (memcmp(read_buf, write_buf[0], size)) { > + tst_res(TFAIL, "read buffer not equal to write buffer1"); > + goto out; > + } > + } else { > + if (memcmp(read_buf, write_buf[0], page_size)) { > + tst_res(TFAIL, "read buffer not equal to write buffer2"); > + goto out; > + } > + if (memcmp(read_buf + page_size, write_buf[1], size - page_size)) { > + tst_res(TFAIL, "read buffer not equal to write buffer3"); > + goto out; > + } > } > > tst_res(TPASS, "read() data correctly"); > @@ -48,13 +61,27 @@ static void setup(void) > { > int fd; > > + page_size = getpagesize(); > + write_buf[0] = tst_alloc(page_size); > + write_buf[1] = tst_alloc(page_size); > + read_buf = tst_alloc(2 * page_size); > + > + memset(write_buf[0], 'A', page_size); > + memset(write_buf[1], 'B', page_size); > + memset(read_buf, 0, 2 * page_size); > + > fd = SAFE_CREAT(fname, 0777); > - SAFE_WRITE(1, fd, palfa, PALFA_LEN); > + SAFE_WRITE(1, fd, write_buf[0], page_size); > + SAFE_WRITE(1, fd, write_buf[1], page_size); > SAFE_CLOSE(fd); > + > } > > static struct tst_test test = { > - .needs_tmpdir = 1, > .setup = setup, > - .test_all = verify_read, > + .test = verify_read, > + .tcnt = 5, > + .mount_device = 1, > + .mntpoint = MNT_POINT, > + .all_filesystems = 1, > };
Hello, Liao Huangjie <liaohj.jy@fujitsu.com> writes: > From: Huangjie Liao <liaohj.jy@fujitsu.com> > > Signed-off-by: Huangjie Liao <liaohj.jy@fujitsu.com> > --- > testcases/kernel/syscalls/read/read04.c | 59 ++++++++++++++++++++++++--------- > 1 file changed, 43 insertions(+), 16 deletions(-) > > diff --git a/testcases/kernel/syscalls/read/read04.c b/testcases/kernel/syscalls/read/read04.c > index 47875c0..37c670f 100644 > --- a/testcases/kernel/syscalls/read/read04.c > +++ b/testcases/kernel/syscalls/read/read04.c > @@ -6,36 +6,49 @@ > /*\ > * [Description] > * > - * Testcase to check if read() returns the number of bytes read correctly. > + * Testcase to check if read() returns the correct number of bytes Trailing whitespace at the end of this line > + * when using multip sizes [0, 1/2*page_size, 3/2*page_size, 2*page_size]. > */ > > #include <sys/types.h> > #include <sys/stat.h> > #include <stdio.h> > #include <fcntl.h> > +#include <unistd.h> > #include "tst_test.h" > > +#define MNT_POINT "mntpoint" > +static int page_size; > static const char *fname = "test_file"; > -static const char palfa[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; > -#define PALFA_LEN (sizeof(palfa)-1) > +static char *write_buf[2], *read_buf; > > -static void verify_read(void) > +static void verify_read(unsigned int n) > { > int fd; > - char prbuf[BUFSIZ]; > - > + int size = n * page_size / 2; Usually interesting things happen around page boundaries. It would be better to read (page_size - 1), page_size and (page_size + 1). And also use an offset starting at (page_size - 1), pages_size etc. > fd = SAFE_OPEN(fname, O_RDONLY); > - TEST(read(fd, prbuf, BUFSIZ)); > + TEST(read(fd, read_buf, size)); > > - if (TST_RET != PALFA_LEN) { > - tst_res(TFAIL, "Bad read count - got %ld - expected %zu", > - TST_RET, PALFA_LEN); > + if (TST_RET != size) { > + tst_res(TFAIL, "Bad read count - got %ld - expected %d", > + TST_RET, size); There is no requirement for read to return size bytes in a single call. A loop is required. The test presently passes because the buffer is small and the process is unlikely to be interrupted. However if you start to read one page at a time and use all filesystems it's much more likely to fail. > goto out; > } > > - if (memcmp(palfa, prbuf, PALFA_LEN)) { > - tst_res(TFAIL, "read buffer not equal to write buffer"); > - goto out; > + if (n <= 2) { > + if (memcmp(read_buf, write_buf[0], size)) { > + tst_res(TFAIL, "read buffer not equal to write buffer1"); > + goto out; > + } > + } else { > + if (memcmp(read_buf, write_buf[0], page_size)) { > + tst_res(TFAIL, "read buffer not equal to write buffer2"); > + goto out; > + } > + if (memcmp(read_buf + page_size, write_buf[1], size - page_size)) { > + tst_res(TFAIL, "read buffer not equal to write buffer3"); > + goto out; > + } > } > > tst_res(TPASS, "read() data correctly"); > @@ -48,13 +61,27 @@ static void setup(void) > { > int fd; > > + page_size = getpagesize(); > + write_buf[0] = tst_alloc(page_size); > + write_buf[1] = tst_alloc(page_size); > + read_buf = tst_alloc(2 * page_size); > + > + memset(write_buf[0], 'A', page_size); > + memset(write_buf[1], 'B', page_size); > + memset(read_buf, 0, 2 * page_size); > + > fd = SAFE_CREAT(fname, 0777); > - SAFE_WRITE(1, fd, palfa, PALFA_LEN); > + SAFE_WRITE(1, fd, write_buf[0], page_size); > + SAFE_WRITE(1, fd, write_buf[1], page_size); This patch fails to apply now because this was changed to use SAFE_WRITE_ALL. > SAFE_CLOSE(fd); > + More trailing whitespace. > } > > static struct tst_test test = { > - .needs_tmpdir = 1, > .setup = setup, > - .test_all = verify_read, > + .test = verify_read, > + .tcnt = 5, > + .mount_device = 1, > + .mntpoint = MNT_POINT, > + .all_filesystems = 1, > }; > -- > 1.8.3.1
diff --git a/testcases/kernel/syscalls/read/read04.c b/testcases/kernel/syscalls/read/read04.c index 47875c0..37c670f 100644 --- a/testcases/kernel/syscalls/read/read04.c +++ b/testcases/kernel/syscalls/read/read04.c @@ -6,36 +6,49 @@ /*\ * [Description] * - * Testcase to check if read() returns the number of bytes read correctly. + * Testcase to check if read() returns the correct number of bytes + * when using multip sizes [0, 1/2*page_size, 3/2*page_size, 2*page_size]. */ #include <sys/types.h> #include <sys/stat.h> #include <stdio.h> #include <fcntl.h> +#include <unistd.h> #include "tst_test.h" +#define MNT_POINT "mntpoint" +static int page_size; static const char *fname = "test_file"; -static const char palfa[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; -#define PALFA_LEN (sizeof(palfa)-1) +static char *write_buf[2], *read_buf; -static void verify_read(void) +static void verify_read(unsigned int n) { int fd; - char prbuf[BUFSIZ]; - + int size = n * page_size / 2; fd = SAFE_OPEN(fname, O_RDONLY); - TEST(read(fd, prbuf, BUFSIZ)); + TEST(read(fd, read_buf, size)); - if (TST_RET != PALFA_LEN) { - tst_res(TFAIL, "Bad read count - got %ld - expected %zu", - TST_RET, PALFA_LEN); + if (TST_RET != size) { + tst_res(TFAIL, "Bad read count - got %ld - expected %d", + TST_RET, size); goto out; } - if (memcmp(palfa, prbuf, PALFA_LEN)) { - tst_res(TFAIL, "read buffer not equal to write buffer"); - goto out; + if (n <= 2) { + if (memcmp(read_buf, write_buf[0], size)) { + tst_res(TFAIL, "read buffer not equal to write buffer1"); + goto out; + } + } else { + if (memcmp(read_buf, write_buf[0], page_size)) { + tst_res(TFAIL, "read buffer not equal to write buffer2"); + goto out; + } + if (memcmp(read_buf + page_size, write_buf[1], size - page_size)) { + tst_res(TFAIL, "read buffer not equal to write buffer3"); + goto out; + } } tst_res(TPASS, "read() data correctly"); @@ -48,13 +61,27 @@ static void setup(void) { int fd; + page_size = getpagesize(); + write_buf[0] = tst_alloc(page_size); + write_buf[1] = tst_alloc(page_size); + read_buf = tst_alloc(2 * page_size); + + memset(write_buf[0], 'A', page_size); + memset(write_buf[1], 'B', page_size); + memset(read_buf, 0, 2 * page_size); + fd = SAFE_CREAT(fname, 0777); - SAFE_WRITE(1, fd, palfa, PALFA_LEN); + SAFE_WRITE(1, fd, write_buf[0], page_size); + SAFE_WRITE(1, fd, write_buf[1], page_size); SAFE_CLOSE(fd); + } static struct tst_test test = { - .needs_tmpdir = 1, .setup = setup, - .test_all = verify_read, + .test = verify_read, + .tcnt = 5, + .mount_device = 1, + .mntpoint = MNT_POINT, + .all_filesystems = 1, };