2023-05-29 23:02:33

by Min-Hua Chen

[permalink] [raw]
Subject: [PATCH] capabilities: use logical OR

Use logical OR to fix the following sparse warnings:

security/commoncap.c:1358:41: sparse: warning: dubious: !x | y

No functional changes intended.

Signed-off-by: Min-Hua Chen <[email protected]>
---
security/commoncap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/commoncap.c b/security/commoncap.c
index 0b3fc2f3afe7..b8e34f6204b2 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -1355,7 +1355,7 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
return commit_creds(new);
}

- if (((!cap_valid(arg3)) | arg4 | arg5))
+ if (((!cap_valid(arg3)) || arg4 || arg5))
return -EINVAL;

if (arg2 == PR_CAP_AMBIENT_IS_SET) {
--
2.34.1



2023-05-30 14:19:37

by David Laight

[permalink] [raw]
Subject: RE: [PATCH] capabilities: use logical OR

From: Min-Hua Chen <[email protected]>
> Sent: 29 May 2023 23:55
>
> Use logical OR to fix the following sparse warnings:
>
> security/commoncap.c:1358:41: sparse: warning: dubious: !x | y
>
> No functional changes intended.

Except it will run just that teeny, weeny bit slower.

David

> Signed-off-by: Min-Hua Chen <[email protected]>
> ---
> security/commoncap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 0b3fc2f3afe7..b8e34f6204b2 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -1355,7 +1355,7 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
> return commit_creds(new);
> }
>
> - if (((!cap_valid(arg3)) | arg4 | arg5))
> + if (((!cap_valid(arg3)) || arg4 || arg5))
> return -EINVAL;
>
> if (arg2 == PR_CAP_AMBIENT_IS_SET) {
> --
> 2.34.1

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


2023-05-31 19:50:10

by Min-Hua Chen

[permalink] [raw]
Subject: RE: [PATCH] capabilities: use logical OR

>From: Min-Hua Chen <[email protected]>
>> Sent: 29 May 2023 23:55
>>
>> Use logical OR to fix the following sparse warnings:
>>
>> security/commoncap.c:1358:41: sparse: warning: dubious: !x | y
>>
>> No functional changes intended.
>
>Except it will run just that teeny, weeny bit slower.
>
> David
>

Thanks for the comment.

I wrote a test case on my x86 machine to see the difference
between '|' and '||' versions.

The '|' version does 2 OR operations and 1 test. (all cases)
The '||' (logical or) version does 3 cmpl (worst case)
and 1 cmpl and 1 jump if the first argument is TRUE (best case).

For the readers, I think the logical OR version is clearer.


thanks,
Min-Hua

my test case:

void foo(void)
{
}

void bar(void)
{
}

int main(void)
{
int a, b, c;
if (a || b || c)
foo();
else
bar();
}

if (a || b || c)
logical OR
Dump of assembler code for function main:
0x000000000000113f <+0>: endbr64
0x0000000000001143 <+4>: push %rbp
0x0000000000001144 <+5>: mov %rsp,%rbp
0x0000000000001147 <+8>: sub $0x10,%rsp
0x000000000000114b <+12>: cmpl $0x0,-0xc(%rbp)
0x000000000000114f <+16>: jne 0x115d <main+30>
0x0000000000001151 <+18>: cmpl $0x0,-0x8(%rbp)
0x0000000000001155 <+22>: jne 0x115d <main+30>
0x0000000000001157 <+24>: cmpl $0x0,-0x4(%rbp)
0x000000000000115b <+28>: je 0x1164 <main+37>
0x000000000000115d <+30>: call 0x1129 <foo>
0x0000000000001162 <+35>: jmp 0x1169 <main+42>
0x0000000000001164 <+37>: call 0x1134 <bar>
0x0000000000001169 <+42>: mov $0x0,%eax
0x000000000000116e <+47>: leave
0x000000000000116f <+48>: ret
End of assembler dump.

if (a | b | c)
Dump of assembler code for function main:
0x000000000000113f <+0>: endbr64
0x0000000000001143 <+4>: push %rbp
0x0000000000001144 <+5>: mov %rsp,%rbp
0x0000000000001147 <+8>: sub $0x10,%rsp
0x000000000000114b <+12>: mov -0xc(%rbp),%eax
0x000000000000114e <+15>: or -0x8(%rbp),%eax
0x0000000000001151 <+18>: or -0x4(%rbp),%eax
0x0000000000001154 <+21>: test %eax,%eax
0x0000000000001156 <+23>: je 0x115f <main+32>
0x0000000000001158 <+25>: call 0x1129 <foo>
0x000000000000115d <+30>: jmp 0x1164 <main+37>
0x000000000000115f <+32>: call 0x1134 <bar>
0x0000000000001164 <+37>: mov $0x0,%eax
0x0000000000001169 <+42>: leave
0x000000000000116a <+43>: ret
End of assembler dump.