BPF Archive on lore.kernel.org
 help / color / Atom feed
* [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	[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

* 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

* 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: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	[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

* [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	[flat|nested] 7+ messages in thread

end of thread, back to index

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

BPF Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/bpf/0 bpf/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 bpf bpf/ https://lore.kernel.org/bpf \
		bpf@vger.kernel.org
	public-inbox-index bpf

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.bpf


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git