On Fri, 2020-11-06 at 13:32 -0800, Andrii Nakryiko wrote:
> On Thu, Nov 5, 2020 at 11:12 PM <xiakaixu1987@xxxxxxxxx> 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:
