* [PATCH] libbpf: Remove unnecessary conversion to bool
@ 2021-02-20 9:10 Jiapeng Chong
2021-02-23 1:49 ` Andrii Nakryiko
0 siblings, 1 reply; 7+ messages in thread
From: Jiapeng Chong @ 2021-02-20 9:10 UTC (permalink / raw)
To: ast
Cc: daniel, andrii, kafai, songliubraving, yhs, john.fastabend,
kpsingh, davem, kuba, hawk, nathan, ndesaulniers, netdev, bpf,
linux-kernel, clang-built-linux, Jiapeng Chong
Fix the following coccicheck warnings:
./tools/lib/bpf/libbpf.c:1487:43-48: WARNING: conversion to bool not
needed here.
Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
---
tools/lib/bpf/libbpf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 6ae748f..5dfdbf3 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1484,7 +1484,7 @@ static int set_kcfg_value_tri(struct extern_desc *ext, void *ext_val,
ext->name, value);
return -EINVAL;
}
- *(bool *)ext_val = value == 'y' ? true : false;
+ *(bool *)ext_val = value == 'y';
break;
case KCFG_TRISTATE:
if (value == 'y')
--
1.8.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] libbpf: Remove unnecessary conversion to bool
2021-02-20 9:10 [PATCH] libbpf: Remove unnecessary conversion to bool Jiapeng Chong
@ 2021-02-23 1:49 ` Andrii Nakryiko
0 siblings, 0 replies; 7+ messages in thread
From: Andrii Nakryiko @ 2021-02-23 1:49 UTC (permalink / raw)
To: Jiapeng Chong
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
Song Liu, Yonghong Song, john fastabend, KP Singh,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
Nathan Chancellor, Nick Desaulniers, Networking, bpf, open list,
clang-built-linux
On Sat, Feb 20, 2021 at 1:11 AM Jiapeng Chong
<jiapeng.chong@linux.alibaba.com> wrote:
>
> Fix the following coccicheck warnings:
>
> ./tools/lib/bpf/libbpf.c:1487:43-48: WARNING: conversion to bool not
> needed here.
>
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
> ---
I think this came up before already. I did this on purpose and I'd
like to keep it that way in the code.
> tools/lib/bpf/libbpf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 6ae748f..5dfdbf3 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1484,7 +1484,7 @@ static int set_kcfg_value_tri(struct extern_desc *ext, void *ext_val,
> ext->name, value);
> return -EINVAL;
> }
> - *(bool *)ext_val = value == 'y' ? true : false;
> + *(bool *)ext_val = value == 'y';
> break;
> case KCFG_TRISTATE:
> if (value == 'y')
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] libbpf: Remove unnecessary conversion to bool
@ 2020-11-06 7:12 xiakaixu1987
2020-11-06 21:32 ` Andrii Nakryiko
0 siblings, 1 reply; 7+ messages in thread
From: xiakaixu1987 @ 2020-11-06 7:12 UTC (permalink / raw)
To: ast, daniel, kafai, songliubraving, yhs, andrii
Cc: netdev, bpf, linux-kernel, Kaixu Xia
From: Kaixu Xia <kaixuxia@tencent.com>
Fix following warning from coccinelle:
./tools/lib/bpf/libbpf.c:1478:43-48: WARNING: conversion to bool not needed here
Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
---
tools/lib/bpf/libbpf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 313034117070..fb9625077983 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1475,7 +1475,7 @@ static int set_kcfg_value_tri(struct extern_desc *ext, void *ext_val,
ext->name, value);
return -EINVAL;
}
- *(bool *)ext_val = value == 'y' ? true : false;
+ *(bool *)ext_val = value == 'y';
break;
case KCFG_TRISTATE:
if (value == 'y')
--
2.20.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] libbpf: Remove unnecessary conversion to bool
2020-11-06 7:12 xiakaixu1987
@ 2020-11-06 21:32 ` Andrii Nakryiko
2020-11-06 21:50 ` Joe Perches
0 siblings, 1 reply; 7+ messages in thread
From: Andrii Nakryiko @ 2020-11-06 21:32 UTC (permalink / raw)
To: xiakaixu1987
Cc: Alexei Starovoitov, Daniel Borkmann, Martin Lau, Song Liu,
Yonghong Song, Andrii Nakryiko, Networking, bpf, open list,
Kaixu Xia
On Thu, Nov 5, 2020 at 11:12 PM <xiakaixu1987@gmail.com> wrote:
>
> From: Kaixu Xia <kaixuxia@tencent.com>
>
> Fix following warning from coccinelle:
>
> ./tools/lib/bpf/libbpf.c:1478:43-48: WARNING: conversion to bool not needed here
>
> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
> ---
> tools/lib/bpf/libbpf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 313034117070..fb9625077983 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1475,7 +1475,7 @@ static int set_kcfg_value_tri(struct extern_desc *ext, void *ext_val,
> ext->name, value);
> return -EINVAL;
> }
> - *(bool *)ext_val = value == 'y' ? true : false;
> + *(bool *)ext_val = value == 'y';
I actually did this intentionally. x = y == z; pattern looked too
obscure to my taste, tbh.
> break;
> case KCFG_TRISTATE:
> if (value == 'y')
> --
> 2.20.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] libbpf: Remove unnecessary conversion to bool
2020-11-06 21:32 ` Andrii Nakryiko
@ 2020-11-06 21:50 ` Joe Perches
2020-11-06 22:19 ` Andrii Nakryiko
2020-11-07 10:46 ` David Laight
0 siblings, 2 replies; 7+ messages in thread
From: Joe Perches @ 2020-11-06 21:50 UTC (permalink / raw)
To: Andrii Nakryiko, xiakaixu1987
Cc: Alexei Starovoitov, Daniel Borkmann, Martin Lau, Song Liu,
Yonghong Song, Andrii Nakryiko, Networking, bpf, open list,
Kaixu Xia
On Fri, 2020-11-06 at 13:32 -0800, Andrii Nakryiko wrote:
> On Thu, Nov 5, 2020 at 11:12 PM <xiakaixu1987@gmail.com> wrote:
> > Fix following warning from coccinelle:
> > ./tools/lib/bpf/libbpf.c:1478:43-48: WARNING: conversion to bool not needed here
[]
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
[]
> > @@ -1475,7 +1475,7 @@ static int set_kcfg_value_tri(struct extern_desc *ext, void *ext_val,
> > ext->name, value);
> > return -EINVAL;
> > }
> > - *(bool *)ext_val = value == 'y' ? true : false;
> > + *(bool *)ext_val = value == 'y';
>
> I actually did this intentionally. x = y == z; pattern looked too
> obscure to my taste, tbh.
It's certainly a question of taste and obviously there is nothing
wrong with yours.
Maybe adding parentheses makes the below look less obscure to you?
x = (y == z);
My taste would run to something like:
---
tools/lib/bpf/libbpf.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 313034117070..5d9c9c8d50c9 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1469,25 +1469,34 @@ static int set_kcfg_value_tri(struct extern_desc *ext, void *ext_val,
char value)
{
switch (ext->kcfg.type) {
- case KCFG_BOOL:
+ case KCFG_BOOL: {
+ bool *p = ext_val;
+
if (value == 'm') {
pr_warn("extern (kcfg) %s=%c should be tristate or char\n",
ext->name, value);
return -EINVAL;
}
- *(bool *)ext_val = value == 'y' ? true : false;
+ *p = (value == 'y');
break;
- case KCFG_TRISTATE:
+ }
+ case KCFG_TRISTATE: {
+ enum libbpf_tristate *p = ext_val;
+
if (value == 'y')
- *(enum libbpf_tristate *)ext_val = TRI_YES;
+ *p = TRI_YES;
else if (value == 'm')
- *(enum libbpf_tristate *)ext_val = TRI_MODULE;
+ *p = TRI_MODULE;
else /* value == 'n' */
- *(enum libbpf_tristate *)ext_val = TRI_NO;
+ *p = TRI_NO;
break;
- case KCFG_CHAR:
- *(char *)ext_val = value;
+ }
+ case KCFG_CHAR: {
+ char *p = ext_val;
+
+ *p = value;
break;
+ }
case KCFG_UNKNOWN:
case KCFG_INT:
case KCFG_CHAR_ARR:
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] libbpf: Remove unnecessary conversion to bool
2020-11-06 21:50 ` Joe Perches
@ 2020-11-06 22:19 ` Andrii Nakryiko
2020-11-07 10:46 ` David Laight
1 sibling, 0 replies; 7+ messages in thread
From: Andrii Nakryiko @ 2020-11-06 22:19 UTC (permalink / raw)
To: Joe Perches
Cc: xiakaixu1987, Alexei Starovoitov, Daniel Borkmann, Martin Lau,
Song Liu, Yonghong Song, Andrii Nakryiko, Networking, bpf,
open list, Kaixu Xia
On Fri, Nov 6, 2020 at 1:50 PM Joe Perches <joe@perches.com> wrote:
>
> On Fri, 2020-11-06 at 13:32 -0800, Andrii Nakryiko wrote:
> > On Thu, Nov 5, 2020 at 11:12 PM <xiakaixu1987@gmail.com> wrote:
> > > Fix following warning from coccinelle:
> > > ./tools/lib/bpf/libbpf.c:1478:43-48: WARNING: conversion to bool not needed here
> []
> > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> []
> > > @@ -1475,7 +1475,7 @@ static int set_kcfg_value_tri(struct extern_desc *ext, void *ext_val,
> > > ext->name, value);
> > > return -EINVAL;
> > > }
> > > - *(bool *)ext_val = value == 'y' ? true : false;
> > > + *(bool *)ext_val = value == 'y';
> >
> > I actually did this intentionally. x = y == z; pattern looked too
> > obscure to my taste, tbh.
>
> It's certainly a question of taste and obviously there is nothing
> wrong with yours.
>
> Maybe adding parentheses makes the below look less obscure to you?
>
> x = (y == z);
Yeah, I think this would be explicit enough. But let's keep the *(bool
*) cast and keep switch code shorter and without extra {} block.
>
> My taste would run to something like:
> ---
> tools/lib/bpf/libbpf.c | 25 +++++++++++++++++--------
> 1 file changed, 17 insertions(+), 8 deletions(-)
>
[...]
^ permalink raw reply [flat|nested] 7+ messages in thread* RE: [PATCH] libbpf: Remove unnecessary conversion to bool
2020-11-06 21:50 ` Joe Perches
2020-11-06 22:19 ` Andrii Nakryiko
@ 2020-11-07 10:46 ` David Laight
1 sibling, 0 replies; 7+ messages in thread
From: David Laight @ 2020-11-07 10:46 UTC (permalink / raw)
To: 'Joe Perches', Andrii Nakryiko, xiakaixu1987
Cc: Alexei Starovoitov, Daniel Borkmann, Martin Lau, Song Liu,
Yonghong Song, Andrii Nakryiko, Networking, bpf, open list,
Kaixu Xia
From: Joe Perches
> Sent: 06 November 2020 21:50
>
> On Fri, 2020-11-06 at 13:32 -0800, Andrii Nakryiko wrote:
> > On Thu, Nov 5, 2020 at 11:12 PM <xiakaixu1987@gmail.com> wrote:
> > > Fix following warning from coccinelle:
> > > ./tools/lib/bpf/libbpf.c:1478:43-48: WARNING: conversion to bool not needed here
> []
> > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> []
> > > @@ -1475,7 +1475,7 @@ static int set_kcfg_value_tri(struct extern_desc *ext, void *ext_val,
> > > ext->name, value);
> > > return -EINVAL;
> > > }
> > > - *(bool *)ext_val = value == 'y' ? true : false;
> > > + *(bool *)ext_val = value == 'y';
> >
> > I actually did this intentionally. x = y == z; pattern looked too
> > obscure to my taste, tbh.
>
> It's certainly a question of taste and obviously there is nothing
> wrong with yours.
>
> Maybe adding parentheses makes the below look less obscure to you?
>
> x = (y == z);
That just leads to people thinking conditionals need to be in parentheses
and then getting the priorities for ?: all wrong as in:
x = a + (b == c) ? d : e;
It would (probably) be better to make 'ext_val' be a union type
(probably a 'pointer to a union' rather than a union of pointers)
so that all the casts go away.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-02-23 1:50 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-20 9:10 [PATCH] libbpf: Remove unnecessary conversion to bool Jiapeng Chong
2021-02-23 1:49 ` Andrii Nakryiko
-- strict thread matches above, loose matches on Subject: below --
2020-11-06 7:12 xiakaixu1987
2020-11-06 21:32 ` Andrii Nakryiko
2020-11-06 21:50 ` Joe Perches
2020-11-06 22:19 ` Andrii Nakryiko
2020-11-07 10:46 ` David Laight
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.