From: Joe Perches <joe@perches.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>, xiakaixu1987@gmail.com
Cc: Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>, Martin Lau <kafai@fb.com>,
Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
Andrii Nakryiko <andrii@kernel.org>,
Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
open list <linux-kernel@vger.kernel.org>,
Kaixu Xia <kaixuxia@tencent.com>
Subject: Re: [PATCH] libbpf: Remove unnecessary conversion to bool
Date: Fri, 06 Nov 2020 13:50:08 -0800 [thread overview]
Message-ID: <d1cefb17a0a915fdabe7a80d14895ff3d85970c1.camel@perches.com> (raw)
In-Reply-To: <CAEf4BzZQ6=-h3g1duXFwDLr92z7nE6ajv8Rz_Zv=qx=-F3sRVA@mail.gmail.com>
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:
next prev parent reply other threads:[~2020-11-06 21:50 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-06 7:12 xiakaixu1987
2020-11-06 21:32 ` Andrii Nakryiko
2020-11-06 21:50 ` Joe Perches [this message]
2020-11-06 22:19 ` Andrii Nakryiko
2020-11-07 10:46 ` David Laight
2021-02-20 9:10 Jiapeng Chong
2021-02-23 1:49 ` Andrii Nakryiko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d1cefb17a0a915fdabe7a80d14895ff3d85970c1.camel@perches.com \
--to=joe@perches.com \
--cc=andrii.nakryiko@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kafai@fb.com \
--cc=kaixuxia@tencent.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=songliubraving@fb.com \
--cc=xiakaixu1987@gmail.com \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®