<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:small"><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jul 15, 2020 at 5:29 PM Cyril Hrubis <<a href="mailto:chrubis@suse.cz" target="_blank">chrubis@suse.cz</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail_default" style="font-size:small">...</span><br>
> + /*<br>
> + * Test the system call for possible privelege levels.<br>
> + * As the privelge level for a normal process is 0,<br>
> + * start by setting/changing the level to 0.<br>
> + */<br>
<br>
This comment should be moved to the test description at the start of the<br>
test, right?<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Agree, let's move it up.</div></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> -#else /* __i386__ */<br>
> -<br>
> -#include "test.h"<br>
> -<br>
> -int TST_TOTAL = 0;<br>
> -<br>
> -int main(void)<br>
> -{<br>
> - tst_resm(TPASS,<br>
> - "LSB v1.3 does not specify iopl() for this architecture.");<br>
> - tst_exit();<br>
> -}<br>
> +static struct tst_test test = {<br>
> + .test_all = verify_iopl,<br>
> + .needs_root = 1,<br>
> + .cleanup = cleanup,<br>
> +};<br>
> <br>
> -#endif /* __i386__ */<br>
> +#else /* __i386__, __x86_64__*/<br>
> +TST_TEST_TCONF("LSB v1.3 does not specify iopl() for this architecture. (only for i386 or x86_64)");<br>
> +#endif /* __i386_, __x86_64__*/<br>
> diff --git a/testcases/kernel/syscalls/iopl/iopl02.c b/testcases/kernel/syscalls/iopl/iopl02.c<br>
> index 35d239268..747a1ca56 100644<br>
> --- a/testcases/kernel/syscalls/iopl/iopl02.c<br>
> +++ b/testcases/kernel/syscalls/iopl/iopl02.c<br>
> @@ -1,221 +1,82 @@<br>
> <span class="gmail_default" style="font-size:small">...</span><br>
> +<br>
> +#if defined __i386__ || defined(__x86_64__)<br>
> <br>
> #define INVALID_LEVEL 4 /* Invalid privilege level */<br>
> #define EXP_RET_VAL -1<br>
<br>
Do we need to have macros for these? I doubt so...<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Yes, we can delete the macros.</div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> + if (i == 1) {<br>
> + /* setup Non super-user for second test */<br>
> + struct passwd *pw;<br>
> + pw = SAFE_GETPWNAM("nobody");<br>
<br>
This should be put into the test setup, also the comment is useless.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Good point, it doesn't matter to run all the tests under non-super-user.</div></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> + if (seteuid(pw->pw_uid) == -1) {<br>
> + tst_res(TWARN, "Failed to set effective"<br>
> + "uid to %d", pw->pw_uid);<br>
> + return;<br>
> }<br>
> }<br>
> <br>
> - /* cleanup and exit */<br>
> - cleanup();<br>
> -<br>
> - tst_exit();<br>
> -<br>
> -}<br>
> -<br>
> -/* setup1() - set up non-super user for second test case */<br>
> -int setup1(void)<br>
> -{<br>
> - /* switch to "nobody" user */<br>
> - if (seteuid(ltpuser->pw_uid) == -1) {<br>
> - tst_resm(TWARN, "Failed to set effective"<br>
> - "uid to %d", ltpuser->pw_uid);<br>
> - return 1;<br>
> + TEST(iopl(tcases[i].level));<br>
> +<br>
> + if ((TST_RET == EXP_RET_VAL) && (TST_ERR == tcases[i].exp_errno)) {<br>
> + tst_res(TPASS, "Expected failure for %s, "<br>
> + "errno: %d", tcases[i].desc,<br>
> + TST_ERR);<br>
<br>
Please use TTERRNO to print TST_ERR. Also the tst_res() prints TPASS so<br>
we can omit the "Expected failure" as well and print it with something<br>
as:<br>
<br>
tst_res(TPASS | TTERRNO, "%s", tcases[i].desc);<br>
<br>
> + } else {<br>
> + tst_res(TFAIL, "Unexpected results for %s ; "<br>
> + "returned %ld (expected %d), errno %d "<br>
> + "(expected errno %d)",<br>
> + tcases[i].desc,<br>
> + TST_RET, EXP_RET_VAL,<br>
> + TST_ERR, tcases[i].exp_errno);<br>
<br>
Here as well, the TST_ERR should be printed with TTERRNO and the<br>
exp_errno should be printed with tst_strerrno(). We can do this with<br>
somethign as:<br>
<br>
tst_res(TFAIL | TTERRNO,<br>
"%s returned %ld expected -1, expected %s got ",<br>
tcases[i].desc, tst_strerrno(tcases[i].exp_errno));<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Quite good suggestions. I missed that TTERRNO.</div></div></div><div><br></div>-- <br><div dir="ltr"><div dir="ltr"><div>Regards,<br></div><div>Li Wang<br></div></div></div></div>