Messages in this thread | | | Subject | Re: [PATCH] mnt: remove unneeded conversion to bool | From | Shuah Khan <> | Date | Mon, 6 Dec 2021 12:00:51 -0700 |
| |
On 12/3/21 8:06 PM, Joe Perches wrote: > On Fri, 2021-12-03 at 17:24 -0700, Shuah Khan wrote: >> On 12/3/21 12:50 PM, Joe Perches wrote: >>> On Fri, 2021-12-03 at 11:26 -0700, Shuah Khan wrote: >>>> On 11/24/21 5:56 PM, davidcomponentone@gmail.com wrote: >>>>> From: Yang Guang <yang.guang5@zte.com.cn> >>>>> >>>>> The coccinelle report >>>>> ./tools/testing/selftests/mount/unprivileged-remount-test.c:285:54-59: >>>>> WARNING: conversion to bool not needed here >>>>> ./tools/testing/selftests/mount/unprivileged-remount-test.c:207:54-59: >>>>> WARNING: conversion to bool not needed here >>>>> Relational and logical operators evaluate to bool, >>>>> explicit conversion is overly verbose and unneeded. >>>>> >>>>> Reported-by: Zeal Robot <zealci@zte.com.cn> >>>>> Signed-off-by: Yang Guang <yang.guang5@zte.com.cn> >>>>> --- >>>>> tools/testing/selftests/mount/unprivileged-remount-test.c | 4 ++-- >>>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/tools/testing/selftests/mount/unprivileged-remount-test.c b/tools/testing/selftests/mount/unprivileged-remount-test.c >>>>> index 584dc6bc3b06..d2917054fe3a 100644 >>>>> --- a/tools/testing/selftests/mount/unprivileged-remount-test.c >>>>> +++ b/tools/testing/selftests/mount/unprivileged-remount-test.c >>>>> @@ -204,7 +204,7 @@ bool test_unpriv_remount(const char *fstype, const char *mount_options, >>>>> if (!WIFEXITED(status)) { >>>>> die("child did not terminate cleanly\n"); >>>>> } >>>>> - return WEXITSTATUS(status) == EXIT_SUCCESS ? true : false; >>>>> + return WEXITSTATUS(status) == EXIT_SUCCESS; >>>>> } >>>>> >>>>> create_and_enter_userns(); >>>>> @@ -282,7 +282,7 @@ static bool test_priv_mount_unpriv_remount(void) >>>>> if (!WIFEXITED(status)) { >>>>> die("child did not terminate cleanly\n"); >>>>> } >>>>> - return WEXITSTATUS(status) == EXIT_SUCCESS ? true : false; >>>>> + return WEXITSTATUS(status) == EXIT_SUCCESS; >>>>> } >>>>> >>>>> orig_mnt_flags = read_mnt_flags(orig_path); >>>>> >>>> >>>> This change doesn't look right. WEXITSTATUS(status) return could be >>>>> 1 or 0 or negative. >>> >>> The change is at least logically correct. >>> >>> And isn't WEXITSTATUS range limited from 0->255 ? >>> >>> https://www.gnu.org/software/libc/manual/html_node/Exit-Status.html >>> >> >> You are right. In any case, I don't see any value in changing the current >> logic. The way it is coded is cryptic enough :) > > Well, it'd be more like the rest of the kernel when changed. > > bool function() > { > ... > return <foo> ? true : false; > } > > is pretty redundant. > >
Fair enough.
Yang Guang,
Applied patch. In the future, make sure to use selftests/<test>: convention for patch summary. I fixed this one for now.
thanks, -- Shuah
| |