| Message ID | 20190328105006.17337-1-pvorel@suse.cz |
|---|---|
| State | Accepted |
| Delegated to: | Petr Vorel |
| Headers | show |
| Series | [1/1] Remove break after return | expand |
LGTM! Reviewed-by: Enji Cooper <yaneurabeya@gmail.com> > On Mar 28, 2019, at 03:50, Petr Vorel <pvorel@suse.cz> wrote: > > Reported-by: Xiao Yang <yangx.jy@cn.fujitsu.com> > Signed-off-by: Petr Vorel <pvorel@suse.cz> > --- > testcases/kernel/syscalls/select/select_var.h | 1 - > testcases/kernel/syscalls/sigpending/sigpending02.c | 3 --- > 2 files changed, 4 deletions(-) > > diff --git a/testcases/kernel/syscalls/select/select_var.h b/testcases/kernel/syscalls/select/select_var.h > index b19a1d1bf..29ebbc5ee 100644 > --- a/testcases/kernel/syscalls/select/select_var.h > +++ b/testcases/kernel/syscalls/select/select_var.h > @@ -20,7 +20,6 @@ static int do_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *except > switch (tst_variant) { > case 0: > return select(nfds, readfds, writefds, exceptfds, timeout); > - break; > case 1: { > #ifdef __LP64__ > return tst_syscall(__NR_select, nfds, readfds, writefds, exceptfds, timeout); > diff --git a/testcases/kernel/syscalls/sigpending/sigpending02.c b/testcases/kernel/syscalls/sigpending/sigpending02.c > index ce0d2ff79..d75807d77 100644 > --- a/testcases/kernel/syscalls/sigpending/sigpending02.c > +++ b/testcases/kernel/syscalls/sigpending/sigpending02.c > @@ -37,13 +37,10 @@ static int tested_sigpending(sigset_t *sigset) > switch (tst_variant) { > case 0: > return sigpending(sigset); > - break; > case 1: > return tst_syscall(__NR_sigpending, sigset); > - break; > case 2: > return tst_syscall(__NR_rt_sigpending, sigset, SIGSETSIZE); > - break; > } > return -1; > } > -- > 2.20.1 > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp
Hi Petr,
Thanks for your cleanup. :-)
Can we also remove break after calling tst_brk()?
For example:
1) The code at do_select(...) in select_var.h
-------------------------------------------------
case 3:
#ifdef __NR__newselect
return tst_syscall(__NR__newselect, nfds, readfds, writefds, exceptfds,
timeout);
#else
tst_brk(TCONF, "__NR__newselect not implemented");
#endif
break;
-------------------------------------------------
2) The code at do_test(...) in newlib_tests/variant.c
-------------------------------------------------
switch (tst_variant) {
case 0:
/* This is skipped after first iteration */
tst_brk(TCONF, "Test skipped");
break;
case 1:
/* This test is correctly looped with -i opt */
tst_res(TPASS, "Test passed");
break;
case 2:
/* This exits the test immediatelly */
tst_brk(TBROK, "Test broken");
break;
}
tst_res(TINFO, "test() function exitting normaly");
-------------------------------------------------
Best Regards,
Xiao Yang
On 2019/03/28 18:50, Petr Vorel wrote:
> Reported-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> testcases/kernel/syscalls/select/select_var.h | 1 -
> testcases/kernel/syscalls/sigpending/sigpending02.c | 3 ---
> 2 files changed, 4 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/select/select_var.h b/testcases/kernel/syscalls/select/select_var.h
> index b19a1d1bf..29ebbc5ee 100644
> --- a/testcases/kernel/syscalls/select/select_var.h
> +++ b/testcases/kernel/syscalls/select/select_var.h
> @@ -20,7 +20,6 @@ static int do_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *except
> switch (tst_variant) {
> case 0:
> return select(nfds, readfds, writefds, exceptfds, timeout);
> - break;
> case 1: {
> #ifdef __LP64__
> return tst_syscall(__NR_select, nfds, readfds, writefds, exceptfds, timeout);
> diff --git a/testcases/kernel/syscalls/sigpending/sigpending02.c b/testcases/kernel/syscalls/sigpending/sigpending02.c
> index ce0d2ff79..d75807d77 100644
> --- a/testcases/kernel/syscalls/sigpending/sigpending02.c
> +++ b/testcases/kernel/syscalls/sigpending/sigpending02.c
> @@ -37,13 +37,10 @@ static int tested_sigpending(sigset_t *sigset)
> switch (tst_variant) {
> case 0:
> return sigpending(sigset);
> - break;
> case 1:
> return tst_syscall(__NR_sigpending, sigset);
> - break;
> case 2:
> return tst_syscall(__NR_rt_sigpending, sigset, SIGSETSIZE);
> - break;
> }
> return -1;
> }
Hi! > Thanks for your cleanup. :-) > > Can we also remove break after calling tst_brk()? AFAIK that would generate warnings, since compiler cannot deduce on it's own that the call will not return.
Hi, > > Can we also remove break after calling tst_brk()? > AFAIK that would generate warnings, since compiler cannot deduce on it's > own that the call will not return. Agree. Kind regards, Petr
On 2019/04/03 20:06, Petr Vorel wrote: >>> Can we also remove break after calling tst_brk()? >> > AFAIK that would generate warnings, since compiler cannot deduce on it's >> > own that the call will not return. > Agree. > Hi, Thanks for your explanation. Other than the question, it looks good to me. Reviewed-by: Xiao Yang <yangx.jy@cn.fujitsu.com> Best Regards, Xiao Yang
On 2019/03/28 18:50, Petr Vorel wrote: > Reported-by: Xiao Yang <yangx.jy@cn.fujitsu.com> > Signed-off-by: Petr Vorel <pvorel@suse.cz> > --- > testcases/kernel/syscalls/select/select_var.h | 1 - > testcases/kernel/syscalls/sigpending/sigpending02.c | 3 --- > 2 files changed, 4 deletions(-) > > diff --git a/testcases/kernel/syscalls/select/select_var.h b/testcases/kernel/syscalls/select/select_var.h > index b19a1d1bf..29ebbc5ee 100644 > --- a/testcases/kernel/syscalls/select/select_var.h > +++ b/testcases/kernel/syscalls/select/select_var.h > @@ -20,7 +20,6 @@ static int do_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *except > switch (tst_variant) { > case 0: > return select(nfds, readfds, writefds, exceptfds, timeout); > - break; > case 1: { > #ifdef __LP64__ > return tst_syscall(__NR_select, nfds, readfds, writefds, exceptfds, timeout); > diff --git a/testcases/kernel/syscalls/sigpending/sigpending02.c b/testcases/kernel/syscalls/sigpending/sigpending02.c > index ce0d2ff79..d75807d77 100644 > --- a/testcases/kernel/syscalls/sigpending/sigpending02.c > +++ b/testcases/kernel/syscalls/sigpending/sigpending02.c > @@ -37,13 +37,10 @@ static int tested_sigpending(sigset_t *sigset) > switch (tst_variant) { > case 0: > return sigpending(sigset); > - break; Hi Petr, I have added libc sigpending() detection by the following patch: http://lists.linux.it/pipermail/ltp/2019-April/011590.html Perhaps, we should avoid potential compiler warnings by keeping the break here. Best Regards, Xiao Yang > case 1: > return tst_syscall(__NR_sigpending, sigset); > - break; > case 2: > return tst_syscall(__NR_rt_sigpending, sigset, SIGSETSIZE); > - break; > } > return -1; > }
Hi, > > > Can we also remove break after calling tst_brk()? > > AFAIK that would generate warnings, since compiler cannot deduce on it's > > own that the call will not return. > Agree. patchset merged (short variant). NOTE: Warning is produced only on gcc (to my surprise only new versions: up to gcc 6 it's ok, 7 and 8 warns): In file included from variant.c:6: variant.c: In function ‘do_test’: ../../include/tst_test.h:76:3: warning: this statement may fall through [-Wimplicit-fallthrough=] tst_brk_(__FILE__, __LINE__, (ttype), (arg_fmt), ##__VA_ARGS__);\ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ variant.c:13:3: note: in expansion of macro ‘tst_brk’ tst_brk(TCONF, "Test skipped"); Kind regards, Petr
Hi Xiao, > I have added libc sigpending() detection by the following patch: > http://lists.linux.it/pipermail/ltp/2019-April/011590.html > Perhaps, we should avoid potential compiler warnings by keeping the > break here. Oh yes, adding break after tst_brk in [1] (in case of #ifndef HAVE_STIME) is correct. Kind regards, Petr [1] https://patchwork.ozlabs.org/patch/1077969/
diff --git a/testcases/kernel/syscalls/select/select_var.h b/testcases/kernel/syscalls/select/select_var.h index b19a1d1bf..29ebbc5ee 100644 --- a/testcases/kernel/syscalls/select/select_var.h +++ b/testcases/kernel/syscalls/select/select_var.h @@ -20,7 +20,6 @@ static int do_select(int nfds, fd_set *readfds, fd_set *writefds, fd_set *except switch (tst_variant) { case 0: return select(nfds, readfds, writefds, exceptfds, timeout); - break; case 1: { #ifdef __LP64__ return tst_syscall(__NR_select, nfds, readfds, writefds, exceptfds, timeout); diff --git a/testcases/kernel/syscalls/sigpending/sigpending02.c b/testcases/kernel/syscalls/sigpending/sigpending02.c index ce0d2ff79..d75807d77 100644 --- a/testcases/kernel/syscalls/sigpending/sigpending02.c +++ b/testcases/kernel/syscalls/sigpending/sigpending02.c @@ -37,13 +37,10 @@ static int tested_sigpending(sigset_t *sigset) switch (tst_variant) { case 0: return sigpending(sigset); - break; case 1: return tst_syscall(__NR_sigpending, sigset); - break; case 2: return tst_syscall(__NR_rt_sigpending, sigset, SIGSETSIZE); - break; } return -1; }
Reported-by: Xiao Yang <yangx.jy@cn.fujitsu.com> Signed-off-by: Petr Vorel <pvorel@suse.cz> --- testcases/kernel/syscalls/select/select_var.h | 1 - testcases/kernel/syscalls/sigpending/sigpending02.c | 3 --- 2 files changed, 4 deletions(-)