* [LTP] [PATCH v2] open08: rewrite to newlib
@ 2018-06-14 12:48 Jan Stancek
2018-06-14 13:47 ` Li Wang
0 siblings, 1 reply; 6+ messages in thread
From: Jan Stancek @ 2018-06-14 12:48 UTC (permalink / raw)
To: ltp
Fixes: https://github.com/linux-test-project/ltp/issues/330
EACCES testcase changed:
Test now creates file (0600) in tmpdir, that is owned by privileged user,
and unprivileged user tries to open it for writing.
EFAULT testcase changed:
Comments say the intent is to use address outside of process mapped space.
So, test is changed to map/unmap rather than map as PROT_NONE.
uclinux ifdefs dropped.
Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
testcases/kernel/syscalls/open/open08.c | 140 +++++++++++++-------------------
1 file changed, 58 insertions(+), 82 deletions(-)
diff --git a/testcases/kernel/syscalls/open/open08.c b/testcases/kernel/syscalls/open/open08.c
index d55d8e6bdd53..d749d1e7100e 100644
--- a/testcases/kernel/syscalls/open/open08.c
+++ b/testcases/kernel/syscalls/open/open08.c
@@ -1,5 +1,6 @@
/*
* Copyright (c) 2013 Wanlong Gao <gaowanlong@cn.fujitsu.com>
+ * Copyright (c) 2018 Linux Test Project
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -39,7 +40,7 @@
* 4. Attempt to open() a filename which is more than VFS_MAXNAMLEN, and
* check for errno to be ENAMETOOLONG.
*
- * 5. Attempt to open a test executable in WRONLY mode,
+ * 5. Attempt to open a (0600) file owned by different user in WRONLY mode,
* open(2) should fail with EACCES.
*
* 6. Attempt to pass an invalid pathname with an address pointing outside
@@ -56,113 +57,88 @@
#include <fcntl.h>
#include <signal.h>
#include <pwd.h>
-#include "test.h"
-#include "safe_macros.h"
-
-static void setup(void);
-static void cleanup(void);
-
-char *TCID = "open08";
-
-static char nobody_uid[] = "nobody";
-static struct passwd *ltpuser;
-
-static char *bad_addr;
+#include "tst_test.h"
static char filename[40] = "";
-static char fname[] = "/bin/cat";
+static char user2_file[] = "user2_0600";
static char bad_file[] = "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyz";
+struct test_case_t;
+static void setup_badfname_buf(struct test_case_t *t);
+
static struct test_case_t {
+ void (*setup)(struct test_case_t *t);
char *fname;
int flags;
int error;
-} TC[] = {
- {filename, O_CREAT | O_EXCL, EEXIST},
- {"/tmp", O_RDWR, EISDIR},
- {filename, O_DIRECTORY, ENOTDIR},
- {bad_file, O_RDWR, ENAMETOOLONG},
- {fname, O_WRONLY, EACCES},
-#if !defined(UCLINUX)
- {(char *)-1, O_CREAT, EFAULT}
-#endif
+} tcases[] = {
+ {NULL, filename, O_CREAT | O_EXCL, EEXIST},
+ {NULL, "/tmp", O_RDWR, EISDIR},
+ {NULL, filename, O_DIRECTORY, ENOTDIR},
+ {NULL, bad_file, O_RDWR, ENAMETOOLONG},
+ {NULL, user2_file, O_WRONLY, EACCES},
+ {setup_badfname_buf, NULL, O_CREAT, EFAULT}
};
-int TST_TOTAL = sizeof(TC) / sizeof(TC[0]);
-
-int main(int ac, char **av)
+void verify_open(unsigned int i)
{
- int lc;
- int i;
-
- tst_parse_opts(ac, av, NULL, NULL);
-
- setup();
-
- for (lc = 0; TEST_LOOPING(lc); lc++) {
- tst_count = 0;
-
- for (i = 0; i < TST_TOTAL; i++) {
- TEST(open(TC[i].fname, TC[i].flags,
- S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH));
-
- if (TEST_RETURN != -1) {
- tst_resm(TFAIL, "call succeeded unexpectedly");
- continue;
- }
-
- if (TEST_ERRNO == TC[i].error) {
- tst_resm(TPASS, "expected failure - "
- "errno = %d : %s", TEST_ERRNO,
- strerror(TEST_ERRNO));
- } else {
- tst_resm(TFAIL, "unexpected error - %d : %s - "
- "expected %d", TEST_ERRNO,
- strerror(TEST_ERRNO), TC[i].error);
- }
- }
+ if (tcases[i].setup)
+ tcases[i].setup(&tcases[i]);
+
+ TEST(open(tcases[i].fname, tcases[i].flags,
+ S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH));
+
+ if (TEST_RETURN != -1) {
+ tst_res(TFAIL, "call succeeded unexpectedly");
+ return;
}
- cleanup();
- tst_exit();
+ if (TEST_ERRNO == tcases[i].error) {
+ tst_res(TPASS, "expected failure - "
+ "errno = %d : %s", TEST_ERRNO,
+ strerror(TEST_ERRNO));
+ } else {
+ tst_res(TFAIL, "unexpected error - %d : %s - "
+ "expected %d", TEST_ERRNO,
+ strerror(TEST_ERRNO), tcases[i].error);
+ }
}
-static void setup(void)
+static void setup_badfname_buf(struct test_case_t *t)
{
- int fildes;
+ char *bad_addr;
+ int len = getpagesize();
- tst_require_root();
+ bad_addr = SAFE_MMAP(0, len, PROT_READ|PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
+ t->fname = bad_addr;
+ SAFE_MUNMAP(bad_addr, len);
+}
- tst_sig(NOFORK, DEF_HANDLER, cleanup);
+static void setup(void)
+{
+ int fildes;
+ char nobody_uid[] = "nobody";
+ struct passwd *ltpuser;
umask(0);
- TEST_PAUSE;
+ SAFE_CREAT(user2_file, 0600);
/* Switch to nobody user for correct error code collection */
ltpuser = getpwnam(nobody_uid);
- SAFE_SETGID(NULL, ltpuser->pw_gid);
- SAFE_SETUID(NULL, ltpuser->pw_uid);
-
- tst_tmpdir();
+ SAFE_SETGID(ltpuser->pw_gid);
+ SAFE_SETUID(ltpuser->pw_uid);
sprintf(filename, "open3.%d", getpid());
-
- fildes = SAFE_CREAT(cleanup, filename, 0600);
-
+ fildes = SAFE_CREAT(filename, 0600);
close(fildes);
-
-#if !defined(UCLINUX)
- bad_addr = mmap(0, 1, PROT_NONE,
- MAP_PRIVATE_EXCEPT_UCLINUX | MAP_ANONYMOUS, 0, 0);
- if (bad_addr == MAP_FAILED)
- tst_brkm(TBROK, cleanup, "mmap failed");
-
- TC[5].fname = bad_addr;
-#endif
}
-static void cleanup(void)
-{
- tst_rmdir();
-}
+static struct tst_test test = {
+ .tcnt = ARRAY_SIZE(tcases),
+ .needs_tmpdir = 1,
+ .needs_root = 1,
+ .setup = setup,
+ .test = verify_open,
+};
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [LTP] [PATCH v2] open08: rewrite to newlib
2018-06-14 12:48 [LTP] [PATCH v2] open08: rewrite to newlib Jan Stancek
@ 2018-06-14 13:47 ` Li Wang
2018-06-15 3:04 ` Li Wang
0 siblings, 1 reply; 6+ messages in thread
From: Li Wang @ 2018-06-14 13:47 UTC (permalink / raw)
To: ltp
On Thu, Jun 14, 2018 at 8:48 PM, Jan Stancek <jstancek@redhat.com> wrote:
> Fixes: https://github.com/linux-test-project/ltp/issues/330
>
> EACCES testcase changed:
> Test now creates file (0600) in tmpdir, that is owned by privileged user,
> and unprivileged user tries to open it for writing.
>
> EFAULT testcase changed:
> Comments say the intent is to use address outside of process mapped space.
> So, test is changed to map/unmap rather than map as PROT_NONE.
>
> uclinux ifdefs dropped.
>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
> testcases/kernel/syscalls/open/open08.c | 140 +++++++++++++-------------------
> 1 file changed, 58 insertions(+), 82 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/open/open08.c b/testcases/kernel/syscalls/open/open08.c
> index d55d8e6bdd53..d749d1e7100e 100644
> --- a/testcases/kernel/syscalls/open/open08.c
> +++ b/testcases/kernel/syscalls/open/open08.c
> @@ -1,5 +1,6 @@
> /*
> * Copyright (c) 2013 Wanlong Gao <gaowanlong@cn.fujitsu.com>
> + * Copyright (c) 2018 Linux Test Project
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License as published by
> @@ -39,7 +40,7 @@
> * 4. Attempt to open() a filename which is more than VFS_MAXNAMLEN, and
> * check for errno to be ENAMETOOLONG.
> *
> - * 5. Attempt to open a test executable in WRONLY mode,
> + * 5. Attempt to open a (0600) file owned by different user in WRONLY mode,
> * open(2) should fail with EACCES.
> *
> * 6. Attempt to pass an invalid pathname with an address pointing outside
> @@ -56,113 +57,88 @@
> #include <fcntl.h>
> #include <signal.h>
> #include <pwd.h>
> -#include "test.h"
> -#include "safe_macros.h"
> -
> -static void setup(void);
> -static void cleanup(void);
> -
> -char *TCID = "open08";
> -
> -static char nobody_uid[] = "nobody";
> -static struct passwd *ltpuser;
> -
> -static char *bad_addr;
> +#include "tst_test.h"
>
> static char filename[40] = "";
> -static char fname[] = "/bin/cat";
> +static char user2_file[] = "user2_0600";
> static char bad_file[] = "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyz";
>
> +struct test_case_t;
> +static void setup_badfname_buf(struct test_case_t *t);
> +
> static struct test_case_t {
> + void (*setup)(struct test_case_t *t);
This function pointer named setup make me a little bit confused at first sight.
It makes me thinking about what's the relationship to ltp steup()
function for a while, actually it does not have.
To avoid the situation to other person, how about just using
void (*func) (struct test_case_t *t);
here?
And my propose change like:
--- a/testcases/kernel/syscalls/open/open08.c
+++ b/testcases/kernel/syscalls/open/open08.c
@@ -64,10 +64,10 @@ static char user2_file[] = "user2_0600";
static char bad_file[] =
"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghi
struct test_case_t;
-static void setup_badfname_buf(struct test_case_t *t);
+static void get_badfname_buf(struct test_case_t *t);
static struct test_case_t {
- void (*setup)(struct test_case_t *t);
+ void (*func)(struct test_case_t *t);
char *fname;
int flags;
int error;
@@ -77,13 +77,14 @@ static struct test_case_t {
{NULL, filename, O_DIRECTORY, ENOTDIR},
{NULL, bad_file, O_RDWR, ENAMETOOLONG},
{NULL, user2_file, O_WRONLY, EACCES},
- {setup_badfname_buf, NULL, O_CREAT, EFAULT}
+ {get_badfname_buf, NULL, O_CREAT, EFAULT}
};
+
void verify_open(unsigned int i)
{
- if (tcases[i].setup)
- tcases[i].setup(&tcases[i]);
+ if (tcases[i].func)
+ tcases[i].func(&tcases[i]);
TEST(open(tcases[i].fname, tcases[i].flags,
S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH));
@@ -104,7 +105,7 @@ void verify_open(unsigned int i)
}
}
-static void setup_badfname_buf(struct test_case_t *t)
+static void get_badfname_buf(struct test_case_t *t)
{
char *bad_addr;
int len = getpagesize();
And, I have to say this is a nice cleanup work. Others looks good to me.
--
Regards,
Li Wang
^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH v2] open08: rewrite to newlib
2018-06-14 13:47 ` Li Wang
@ 2018-06-15 3:04 ` Li Wang
2018-06-15 6:11 ` Jan Stancek
0 siblings, 1 reply; 6+ messages in thread
From: Li Wang @ 2018-06-15 3:04 UTC (permalink / raw)
To: ltp
On Thu, Jun 14, 2018 at 9:47 PM, Li Wang <liwang@redhat.com> wrote:
> On Thu, Jun 14, 2018 at 8:48 PM, Jan Stancek <jstancek@redhat.com> wrote:
>> Fixes: https://github.com/linux-test-project/ltp/issues/330
>>
>> EACCES testcase changed:
>> Test now creates file (0600) in tmpdir, that is owned by privileged user,
>> and unprivileged user tries to open it for writing.
>>
>> EFAULT testcase changed:
>> Comments say the intent is to use address outside of process mapped space.
>> So, test is changed to map/unmap rather than map as PROT_NONE.
>>
>> uclinux ifdefs dropped.
>>
>> Signed-off-by: Jan Stancek <jstancek@redhat.com>
>> ---
>> testcases/kernel/syscalls/open/open08.c | 140 +++++++++++++-------------------
>> 1 file changed, 58 insertions(+), 82 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/open/open08.c b/testcases/kernel/syscalls/open/open08.c
>> index d55d8e6bdd53..d749d1e7100e 100644
>> --- a/testcases/kernel/syscalls/open/open08.c
>> +++ b/testcases/kernel/syscalls/open/open08.c
>> @@ -1,5 +1,6 @@
>> /*
>> * Copyright (c) 2013 Wanlong Gao <gaowanlong@cn.fujitsu.com>
>> + * Copyright (c) 2018 Linux Test Project
>> *
>> * This program is free software; you can redistribute it and/or modify
>> * it under the terms of the GNU General Public License as published by
>> @@ -39,7 +40,7 @@
>> * 4. Attempt to open() a filename which is more than VFS_MAXNAMLEN, and
>> * check for errno to be ENAMETOOLONG.
>> *
>> - * 5. Attempt to open a test executable in WRONLY mode,
>> + * 5. Attempt to open a (0600) file owned by different user in WRONLY mode,
>> * open(2) should fail with EACCES.
>> *
>> * 6. Attempt to pass an invalid pathname with an address pointing outside
>> @@ -56,113 +57,88 @@
>> #include <fcntl.h>
>> #include <signal.h>
>> #include <pwd.h>
>> -#include "test.h"
>> -#include "safe_macros.h"
>> -
>> -static void setup(void);
>> -static void cleanup(void);
>> -
>> -char *TCID = "open08";
>> -
>> -static char nobody_uid[] = "nobody";
>> -static struct passwd *ltpuser;
>> -
>> -static char *bad_addr;
>> +#include "tst_test.h"
>>
>> static char filename[40] = "";
>> -static char fname[] = "/bin/cat";
>> +static char user2_file[] = "user2_0600";
>> static char bad_file[] = "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyz";
>>
>> +struct test_case_t;
>> +static void setup_badfname_buf(struct test_case_t *t);
>> +
>> static struct test_case_t {
>> + void (*setup)(struct test_case_t *t);
>
> This function pointer named setup make me a little bit confused at first sight.
> It makes me thinking about what's the relationship to ltp steup()
> function for a while, actually it does not have.
>
> To avoid the situation to other person, how about just using
> void (*func) (struct test_case_t *t);
> here?
Or, maybe we could drop the func pointer in the test structure, and
reset the badfname only in verify_open(). It's hard to say which
method is better, but seems this can make things more easier I guess.
Which something differ with your patch too:
--- a/testcases/kernel/syscalls/open/open08.c
+++ b/testcases/kernel/syscalls/open/open08.c
@@ -63,27 +63,30 @@ static char filename[40] = "";
static char user2_file[] = "user2_0600";
static char bad_file[] =
"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyz";
-struct test_case_t;
-static void setup_badfname_buf(struct test_case_t *t);
-
static struct test_case_t {
- void (*setup)(struct test_case_t *t);
char *fname;
int flags;
int error;
} tcases[] = {
- {NULL, filename, O_CREAT | O_EXCL, EEXIST},
- {NULL, "/tmp", O_RDWR, EISDIR},
- {NULL, filename, O_DIRECTORY, ENOTDIR},
- {NULL, bad_file, O_RDWR, ENAMETOOLONG},
- {NULL, user2_file, O_WRONLY, EACCES},
- {setup_badfname_buf, NULL, O_CREAT, EFAULT}
+ {filename, O_CREAT | O_EXCL, EEXIST},
+ {"/tmp", O_RDWR, EISDIR},
+ {filename, O_DIRECTORY, ENOTDIR},
+ {bad_file, O_RDWR, ENAMETOOLONG},
+ {user2_file, O_WRONLY, EACCES},
+ {NULL, O_CREAT, EFAULT}
};
void verify_open(unsigned int i)
{
- if (tcases[i].setup)
- tcases[i].setup(&tcases[i]);
+ if (i == ARRAY_SIZE(tcases) - 1) {
+ char *bad_addr;
+ int len = getpagesize();
+
+ bad_addr = SAFE_MMAP(0, len, PROT_READ|PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
+ tcases[i].fname = bad_addr;
+ SAFE_MUNMAP(bad_addr, len);
+ }
TEST(open(tcases[i].fname, tcases[i].flags,
S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH));
@@ -104,17 +107,6 @@ void verify_open(unsigned int i)
}
}
-static void setup_badfname_buf(struct test_case_t *t)
-{
- char *bad_addr;
- int len = getpagesize();
-
- bad_addr = SAFE_MMAP(0, len, PROT_READ|PROT_WRITE,
- MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
- t->fname = bad_addr;
- SAFE_MUNMAP(bad_addr, len);
-}
-
static void setup(void)
{
int fildes;
--
Regards,
Li Wang
^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH v2] open08: rewrite to newlib
2018-06-15 3:04 ` Li Wang
@ 2018-06-15 6:11 ` Jan Stancek
2018-06-15 8:16 ` Li Wang
0 siblings, 1 reply; 6+ messages in thread
From: Jan Stancek @ 2018-06-15 6:11 UTC (permalink / raw)
To: ltp
----- Original Message -----
> On Thu, Jun 14, 2018 at 9:47 PM, Li Wang <liwang@redhat.com> wrote:
> > On Thu, Jun 14, 2018 at 8:48 PM, Jan Stancek <jstancek@redhat.com> wrote:
> >> Fixes: https://github.com/linux-test-project/ltp/issues/330
> >>
> >> EACCES testcase changed:
> >> Test now creates file (0600) in tmpdir, that is owned by privileged user,
> >> and unprivileged user tries to open it for writing.
> >>
> >> EFAULT testcase changed:
> >> Comments say the intent is to use address outside of process mapped space.
> >> So, test is changed to map/unmap rather than map as PROT_NONE.
> >>
> >> uclinux ifdefs dropped.
> >>
> >> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> >> ---
> >> testcases/kernel/syscalls/open/open08.c | 140
> >> +++++++++++++-------------------
> >> 1 file changed, 58 insertions(+), 82 deletions(-)
> >>
> >> diff --git a/testcases/kernel/syscalls/open/open08.c
> >> b/testcases/kernel/syscalls/open/open08.c
> >> index d55d8e6bdd53..d749d1e7100e 100644
> >> --- a/testcases/kernel/syscalls/open/open08.c
> >> +++ b/testcases/kernel/syscalls/open/open08.c
> >> @@ -1,5 +1,6 @@
> >> /*
> >> * Copyright (c) 2013 Wanlong Gao <gaowanlong@cn.fujitsu.com>
> >> + * Copyright (c) 2018 Linux Test Project
> >> *
> >> * This program is free software; you can redistribute it and/or
> >> modify
> >> * it under the terms of the GNU General Public License as published by
> >> @@ -39,7 +40,7 @@
> >> * 4. Attempt to open() a filename which is more than VFS_MAXNAMLEN,
> >> and
> >> * check for errno to be ENAMETOOLONG.
> >> *
> >> - * 5. Attempt to open a test executable in WRONLY mode,
> >> + * 5. Attempt to open a (0600) file owned by different user in WRONLY
> >> mode,
> >> * open(2) should fail with EACCES.
> >> *
> >> * 6. Attempt to pass an invalid pathname with an address pointing
> >> outside
> >> @@ -56,113 +57,88 @@
> >> #include <fcntl.h>
> >> #include <signal.h>
> >> #include <pwd.h>
> >> -#include "test.h"
> >> -#include "safe_macros.h"
> >> -
> >> -static void setup(void);
> >> -static void cleanup(void);
> >> -
> >> -char *TCID = "open08";
> >> -
> >> -static char nobody_uid[] = "nobody";
> >> -static struct passwd *ltpuser;
> >> -
> >> -static char *bad_addr;
> >> +#include "tst_test.h"
> >>
> >> static char filename[40] = "";
> >> -static char fname[] = "/bin/cat";
> >> +static char user2_file[] = "user2_0600";
> >> static char bad_file[] =
> >> "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyz";
> >>
> >> +struct test_case_t;
> >> +static void setup_badfname_buf(struct test_case_t *t);
> >> +
> >> static struct test_case_t {
> >> + void (*setup)(struct test_case_t *t);
> >
> > This function pointer named setup make me a little bit confused at first
> > sight.
> > It makes me thinking about what's the relationship to ltp steup()
> > function for a while, actually it does not have.
> >
> > To avoid the situation to other person, how about just using
> > void (*func) (struct test_case_t *t);
> > here?
>
> Or, maybe we could drop the func pointer in the test structure, and
> reset the badfname only in verify_open(). It's hard to say which
> method is better, but seems this can make things more easier I guess.
>
> Which something differ with your patch too:
>
> --- a/testcases/kernel/syscalls/open/open08.c
> +++ b/testcases/kernel/syscalls/open/open08.c
> @@ -63,27 +63,30 @@ static char filename[40] = "";
> static char user2_file[] = "user2_0600";
> static char bad_file[] =
> "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyz";
>
> -struct test_case_t;
> -static void setup_badfname_buf(struct test_case_t *t);
> -
> static struct test_case_t {
> - void (*setup)(struct test_case_t *t);
> char *fname;
> int flags;
> int error;
> } tcases[] = {
> - {NULL, filename, O_CREAT | O_EXCL, EEXIST},
> - {NULL, "/tmp", O_RDWR, EISDIR},
> - {NULL, filename, O_DIRECTORY, ENOTDIR},
> - {NULL, bad_file, O_RDWR, ENAMETOOLONG},
> - {NULL, user2_file, O_WRONLY, EACCES},
> - {setup_badfname_buf, NULL, O_CREAT, EFAULT}
> + {filename, O_CREAT | O_EXCL, EEXIST},
> + {"/tmp", O_RDWR, EISDIR},
> + {filename, O_DIRECTORY, ENOTDIR},
> + {bad_file, O_RDWR, ENAMETOOLONG},
> + {user2_file, O_WRONLY, EACCES},
> + {NULL, O_CREAT, EFAULT}
> };
>
> void verify_open(unsigned int i)
> {
> - if (tcases[i].setup)
> - tcases[i].setup(&tcases[i]);
> + if (i == ARRAY_SIZE(tcases) - 1) {
It's same as hardcoding index to array - if someone adds
new testcase it breaks.
But we could set it to some special value (NULL) and check all cases against that.
If it's NULL, then make a bad_addr.
> + char *bad_addr;
> + int len = getpagesize();
> +
> + bad_addr = SAFE_MMAP(0, len, PROT_READ|PROT_WRITE,
> + MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
> + tcases[i].fname = bad_addr;
> + SAFE_MUNMAP(bad_addr, len);
> + }
>
> TEST(open(tcases[i].fname, tcases[i].flags,
> S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH));
> @@ -104,17 +107,6 @@ void verify_open(unsigned int i)
> }
> }
>
> -static void setup_badfname_buf(struct test_case_t *t)
> -{
> - char *bad_addr;
> - int len = getpagesize();
> -
> - bad_addr = SAFE_MMAP(0, len, PROT_READ|PROT_WRITE,
> - MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
> - t->fname = bad_addr;
> - SAFE_MUNMAP(bad_addr, len);
> -}
> -
> static void setup(void)
> {
> int fildes;
>
>
>
> --
> Regards,
> Li Wang
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH v2] open08: rewrite to newlib
2018-06-15 6:11 ` Jan Stancek
@ 2018-06-15 8:16 ` Li Wang
2018-06-26 13:49 ` Cyril Hrubis
0 siblings, 1 reply; 6+ messages in thread
From: Li Wang @ 2018-06-15 8:16 UTC (permalink / raw)
To: ltp
On Fri, Jun 15, 2018 at 2:11 PM, Jan Stancek <jstancek@redhat.com> wrote:
>
>> void verify_open(unsigned int i)
>> {
>> - if (tcases[i].setup)
>> - tcases[i].setup(&tcases[i]);
>> + if (i == ARRAY_SIZE(tcases) - 1) {
>
> It's same as hardcoding index to array - if someone adds
> new testcase it breaks.
Yes, to avoid that, new testcase should not be as the last one.
>
> But we could set it to some special value (NULL) and check all cases against that.
> If it's NULL, then make a bad_addr.
Agreed.
--
Regards,
Li Wang
^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH v2] open08: rewrite to newlib
2018-06-15 8:16 ` Li Wang
@ 2018-06-26 13:49 ` Cyril Hrubis
0 siblings, 0 replies; 6+ messages in thread
From: Cyril Hrubis @ 2018-06-26 13:49 UTC (permalink / raw)
To: ltp
Hi!
> > But we could set it to some special value (NULL) and check all cases against that.
> > If it's NULL, then make a bad_addr.
>
> Agreed.
Either we do that or make the addr pointer to a pointer in the test
structure and make it to point to a global variable initialized in the
test setup as we do in some tests as well.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-06-26 13:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-14 12:48 [LTP] [PATCH v2] open08: rewrite to newlib Jan Stancek
2018-06-14 13:47 ` Li Wang
2018-06-15 3:04 ` Li Wang
2018-06-15 6:11 ` Jan Stancek
2018-06-15 8:16 ` Li Wang
2018-06-26 13:49 ` Cyril Hrubis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox