* [PATCH] net/ipv4: fib_trie: Avoid cryptic ternary expressions
@ 2019-06-18 21:14 Matthias Kaehlcke
2019-06-18 21:45 ` Doug Anderson
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Matthias Kaehlcke @ 2019-06-18 21:14 UTC (permalink / raw)
To: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Alexander Duyck
Cc: netdev, linux-kernel, Douglas Anderson, Matthias Kaehlcke
empty_child_inc/dec() use the ternary operator for conditional
operations. The conditions involve the post/pre in/decrement
operator and the operation is only performed when the condition
is *not* true. This is hard to parse for humans, use a regular
'if' construct instead and perform the in/decrement separately.
This also fixes two warnings that are emitted about the value
of the ternary expression being unused, when building the kernel
with clang + "kbuild: Remove unnecessary -Wno-unused-value"
(https://lore.kernel.org/patchwork/patch/1089869/):
CC net/ipv4/fib_trie.o
net/ipv4/fib_trie.c:351:2: error: expression result unused [-Werror,-Wunused-value]
++tn_info(n)->empty_children ? : ++tn_info(n)->full_children;
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
I have no good understanding of the fib_trie code, but the
disentangled code looks wrong, and it should be equivalent to the
cryptic version, unless I messed it up. In empty_child_inc()
'full_children' is only incremented when 'empty_children' is -1. I
suspect a bug in the cryptic code, but am surprised why it hasn't
blown up yet. Or is it intended behavior that is just
super-counterintuitive?
For now I'm leaving it at disentangling the cryptic expressions,
if there is a bug we can discuss what action to take.
---
net/ipv4/fib_trie.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 868c74771fa9..cf7788e336b7 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -338,12 +338,18 @@ static struct tnode *tnode_alloc(int bits)
static inline void empty_child_inc(struct key_vector *n)
{
- ++tn_info(n)->empty_children ? : ++tn_info(n)->full_children;
+ tn_info(n)->empty_children++;
+
+ if (!tn_info(n)->empty_children)
+ tn_info(n)->full_children++;
}
static inline void empty_child_dec(struct key_vector *n)
{
- tn_info(n)->empty_children-- ? : tn_info(n)->full_children--;
+ if (!tn_info(n)->empty_children)
+ tn_info(n)->full_children--;
+
+ tn_info(n)->empty_children--;
}
static struct key_vector *leaf_new(t_key key, struct fib_alias *fa)
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] net/ipv4: fib_trie: Avoid cryptic ternary expressions
2019-06-18 21:14 [PATCH] net/ipv4: fib_trie: Avoid cryptic ternary expressions Matthias Kaehlcke
@ 2019-06-18 21:45 ` Doug Anderson
2019-06-18 22:57 ` Matthias Kaehlcke
2019-06-18 23:04 ` Nathan Chancellor
2019-06-19 21:29 ` David Miller
2 siblings, 1 reply; 11+ messages in thread
From: Doug Anderson @ 2019-06-18 21:45 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Alexander Duyck, netdev, LKML
Hi,
On Tue, Jun 18, 2019 at 2:14 PM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> empty_child_inc/dec() use the ternary operator for conditional
> operations. The conditions involve the post/pre in/decrement
> operator and the operation is only performed when the condition
> is *not* true. This is hard to parse for humans, use a regular
> 'if' construct instead and perform the in/decrement separately.
>
> This also fixes two warnings that are emitted about the value
> of the ternary expression being unused, when building the kernel
> with clang + "kbuild: Remove unnecessary -Wno-unused-value"
> (https://lore.kernel.org/patchwork/patch/1089869/):
>
> CC net/ipv4/fib_trie.o
> net/ipv4/fib_trie.c:351:2: error: expression result unused [-Werror,-Wunused-value]
> ++tn_info(n)->empty_children ? : ++tn_info(n)->full_children;
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> I have no good understanding of the fib_trie code, but the
> disentangled code looks wrong, and it should be equivalent to the
> cryptic version, unless I messed it up. In empty_child_inc()
> 'full_children' is only incremented when 'empty_children' is -1. I
> suspect a bug in the cryptic code, but am surprised why it hasn't
> blown up yet. Or is it intended behavior that is just
> super-counterintuitive?
>
> For now I'm leaving it at disentangling the cryptic expressions,
> if there is a bug we can discuss what action to take.
> ---
> net/ipv4/fib_trie.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
I have no knowledge of this code either but Matthias's patch looks
sane to me and I agree with the disentangling before making functional
changes.
My own personal belief is that this is pointing out a bug somewhere.
Since "empty_children" ends up being an unsigned type it doesn't feel
like it was by-design that -1 is ever a value that should be in there.
In any case:
Reviewed-by: Douglas Anderson <dianders@chromium.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net/ipv4: fib_trie: Avoid cryptic ternary expressions
2019-06-18 21:45 ` Doug Anderson
@ 2019-06-18 22:57 ` Matthias Kaehlcke
0 siblings, 0 replies; 11+ messages in thread
From: Matthias Kaehlcke @ 2019-06-18 22:57 UTC (permalink / raw)
To: Doug Anderson
Cc: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Alexander Duyck, netdev, LKML, Greg Kroah-Hartman, Sasha Levin
On Tue, Jun 18, 2019 at 02:45:34PM -0700, Doug Anderson wrote:
> Hi,
>
> On Tue, Jun 18, 2019 at 2:14 PM Matthias Kaehlcke <mka@chromium.org> wrote:
> >
> > empty_child_inc/dec() use the ternary operator for conditional
> > operations. The conditions involve the post/pre in/decrement
> > operator and the operation is only performed when the condition
> > is *not* true. This is hard to parse for humans, use a regular
> > 'if' construct instead and perform the in/decrement separately.
> >
> > This also fixes two warnings that are emitted about the value
> > of the ternary expression being unused, when building the kernel
> > with clang + "kbuild: Remove unnecessary -Wno-unused-value"
> > (https://lore.kernel.org/patchwork/patch/1089869/):
> >
> > CC net/ipv4/fib_trie.o
> > net/ipv4/fib_trie.c:351:2: error: expression result unused [-Werror,-Wunused-value]
> > ++tn_info(n)->empty_children ? : ++tn_info(n)->full_children;
> >
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> > I have no good understanding of the fib_trie code, but the
> > disentangled code looks wrong, and it should be equivalent to the
> > cryptic version, unless I messed it up. In empty_child_inc()
> > 'full_children' is only incremented when 'empty_children' is -1. I
> > suspect a bug in the cryptic code, but am surprised why it hasn't
> > blown up yet. Or is it intended behavior that is just
> > super-counterintuitive?
> >
> > For now I'm leaving it at disentangling the cryptic expressions,
> > if there is a bug we can discuss what action to take.
> > ---
> > net/ipv4/fib_trie.c | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
>
> I have no knowledge of this code either but Matthias's patch looks
> sane to me and I agree with the disentangling before making functional
> changes.
I in terms of the -stable process it might make sense to either
disentangle & fix in a single step, or first fix the cryptic code
(shudder!) and then disentangle it. I guess if we make it a series
disentangle & fix could be separate steps.
I'm open to whatever maintainers & stable folks prefer.
> My own personal belief is that this is pointing out a bug somewhere.
> Since "empty_children" ends up being an unsigned type it doesn't feel
> like it was by-design that -1 is ever a value that should be in there.
good point that 'empty_children' is unsigned, that indeed reinforces
the bug theory.
> In any case:
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
Thanks!
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net/ipv4: fib_trie: Avoid cryptic ternary expressions
2019-06-18 21:14 [PATCH] net/ipv4: fib_trie: Avoid cryptic ternary expressions Matthias Kaehlcke
2019-06-18 21:45 ` Doug Anderson
@ 2019-06-18 23:04 ` Nathan Chancellor
2019-06-18 23:21 ` Matthias Kaehlcke
2019-06-18 23:23 ` Nick Desaulniers
2019-06-19 21:29 ` David Miller
2 siblings, 2 replies; 11+ messages in thread
From: Nathan Chancellor @ 2019-06-18 23:04 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Alexander Duyck, netdev, linux-kernel, Douglas Anderson,
Nick Desaulniers, Nathan Huckleberry, clang-built-linux
On Tue, Jun 18, 2019 at 02:14:40PM -0700, Matthias Kaehlcke wrote:
> empty_child_inc/dec() use the ternary operator for conditional
> operations. The conditions involve the post/pre in/decrement
> operator and the operation is only performed when the condition
> is *not* true. This is hard to parse for humans, use a regular
> 'if' construct instead and perform the in/decrement separately.
>
> This also fixes two warnings that are emitted about the value
> of the ternary expression being unused, when building the kernel
> with clang + "kbuild: Remove unnecessary -Wno-unused-value"
> (https://lore.kernel.org/patchwork/patch/1089869/):
>
> CC net/ipv4/fib_trie.o
> net/ipv4/fib_trie.c:351:2: error: expression result unused [-Werror,-Wunused-value]
> ++tn_info(n)->empty_children ? : ++tn_info(n)->full_children;
>
As an FYI, this is also being fixed in clang:
https://bugs.llvm.org/show_bug.cgi?id=42239
https://reviews.llvm.org/D63369
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> I have no good understanding of the fib_trie code, but the
> disentangled code looks wrong, and it should be equivalent to the
> cryptic version, unless I messed it up. In empty_child_inc()
> 'full_children' is only incremented when 'empty_children' is -1. I
> suspect a bug in the cryptic code, but am surprised why it hasn't
> blown up yet. Or is it intended behavior that is just
> super-counterintuitive?
>
> For now I'm leaving it at disentangling the cryptic expressions,
> if there is a bug we can discuss what action to take.
> ---
> net/ipv4/fib_trie.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
> index 868c74771fa9..cf7788e336b7 100644
> --- a/net/ipv4/fib_trie.c
> +++ b/net/ipv4/fib_trie.c
> @@ -338,12 +338,18 @@ static struct tnode *tnode_alloc(int bits)
>
> static inline void empty_child_inc(struct key_vector *n)
> {
> - ++tn_info(n)->empty_children ? : ++tn_info(n)->full_children;
> + tn_info(n)->empty_children++;
> +
> + if (!tn_info(n)->empty_children)
> + tn_info(n)->full_children++;
> }
>
> static inline void empty_child_dec(struct key_vector *n)
> {
> - tn_info(n)->empty_children-- ? : tn_info(n)->full_children--;
> + if (!tn_info(n)->empty_children)
> + tn_info(n)->full_children--;
> +
> + tn_info(n)->empty_children--;
> }
>
> static struct key_vector *leaf_new(t_key key, struct fib_alias *fa)
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net/ipv4: fib_trie: Avoid cryptic ternary expressions
2019-06-18 23:04 ` Nathan Chancellor
@ 2019-06-18 23:21 ` Matthias Kaehlcke
2019-06-19 2:00 ` Alexander Duyck
2019-06-18 23:23 ` Nick Desaulniers
1 sibling, 1 reply; 11+ messages in thread
From: Matthias Kaehlcke @ 2019-06-18 23:21 UTC (permalink / raw)
To: Nathan Chancellor
Cc: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev,
linux-kernel, Douglas Anderson, Nick Desaulniers,
Nathan Huckleberry, clang-built-linux
On Tue, Jun 18, 2019 at 04:04:20PM -0700, Nathan Chancellor wrote:
> On Tue, Jun 18, 2019 at 02:14:40PM -0700, Matthias Kaehlcke wrote:
> > empty_child_inc/dec() use the ternary operator for conditional
> > operations. The conditions involve the post/pre in/decrement
> > operator and the operation is only performed when the condition
> > is *not* true. This is hard to parse for humans, use a regular
> > 'if' construct instead and perform the in/decrement separately.
> >
> > This also fixes two warnings that are emitted about the value
> > of the ternary expression being unused, when building the kernel
> > with clang + "kbuild: Remove unnecessary -Wno-unused-value"
> > (https://lore.kernel.org/patchwork/patch/1089869/):
> >
> > CC net/ipv4/fib_trie.o
> > net/ipv4/fib_trie.c:351:2: error: expression result unused [-Werror,-Wunused-value]
> > ++tn_info(n)->empty_children ? : ++tn_info(n)->full_children;
> >
>
> As an FYI, this is also being fixed in clang:
>
> https://bugs.llvm.org/show_bug.cgi?id=42239
>
> https://reviews.llvm.org/D63369
Great, thanks!
In this case it was actually useful to get the warning, even though it
didn't point out the actual bug. I think in general it would be
preferable to avoid such constructs, even when they are correct. But
then again, it's the reviewers/maintainers task to avoid unnecessarily
cryptic code from slipping in, and this just happens to be one instance
where the compiler could have helped.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net/ipv4: fib_trie: Avoid cryptic ternary expressions
2019-06-18 23:04 ` Nathan Chancellor
2019-06-18 23:21 ` Matthias Kaehlcke
@ 2019-06-18 23:23 ` Nick Desaulniers
2019-06-19 9:36 ` Joe Perches
1 sibling, 1 reply; 11+ messages in thread
From: Nick Desaulniers @ 2019-06-18 23:23 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Alexander Duyck, netdev, LKML, Douglas Anderson,
Nathan Huckleberry, clang-built-linux, Nathan Chancellor
On Tue, Jun 18, 2019 at 4:04 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> On Tue, Jun 18, 2019 at 02:14:40PM -0700, Matthias Kaehlcke wrote:
> > empty_child_inc/dec() use the ternary operator for conditional
> > operations. The conditions involve the post/pre in/decrement
> > operator and the operation is only performed when the condition
> > is *not* true. This is hard to parse for humans, use a regular
> > 'if' construct instead and perform the in/decrement separately.
> >
> > This also fixes two warnings that are emitted about the value
> > of the ternary expression being unused, when building the kernel
> > with clang + "kbuild: Remove unnecessary -Wno-unused-value"
> > (https://lore.kernel.org/patchwork/patch/1089869/):
> >
> > CC net/ipv4/fib_trie.o
> > net/ipv4/fib_trie.c:351:2: error: expression result unused [-Werror,-Wunused-value]
> > ++tn_info(n)->empty_children ? : ++tn_info(n)->full_children;
> >
>
> As an FYI, this is also being fixed in clang:
>
> https://bugs.llvm.org/show_bug.cgi?id=42239
>
> https://reviews.llvm.org/D63369
While I'm glad we found and fixed a bug in Clang; I'm still for fixing
the underlying code from a readability perspective. If it takes
longer than a few seconds to understand a statement to the non-author,
the code as written may be too clever.
As a side note, I'm going to try to see if MAINTAINERS and
scripts/get_maintainers.pl supports regexes on the commit messages in
order to cc our mailing list (clang-built-linux
<clang-built-linux@googlegroups.com>) automatically; but in the
meantime please try to remember to cc the list manually. I usually
find it quickly from the bookmarked https://clangbuiltlinux.github.io/
which lists it there) (but I should still automate this).
>
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> > I have no good understanding of the fib_trie code, but the
> > disentangled code looks wrong, and it should be equivalent to the
> > cryptic version, unless I messed it up. In empty_child_inc()
> > 'full_children' is only incremented when 'empty_children' is -1. I
> > suspect a bug in the cryptic code, but am surprised why it hasn't
> > blown up yet. Or is it intended behavior that is just
> > super-counterintuitive?
> >
> > For now I'm leaving it at disentangling the cryptic expressions,
> > if there is a bug we can discuss what action to take.
> > ---
> > net/ipv4/fib_trie.c | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
> > index 868c74771fa9..cf7788e336b7 100644
> > --- a/net/ipv4/fib_trie.c
> > +++ b/net/ipv4/fib_trie.c
> > @@ -338,12 +338,18 @@ static struct tnode *tnode_alloc(int bits)
> >
> > static inline void empty_child_inc(struct key_vector *n)
> > {
> > - ++tn_info(n)->empty_children ? : ++tn_info(n)->full_children;
> > + tn_info(n)->empty_children++;
> > +
> > + if (!tn_info(n)->empty_children)
> > + tn_info(n)->full_children++;
> > }
> >
> > static inline void empty_child_dec(struct key_vector *n)
> > {
> > - tn_info(n)->empty_children-- ? : tn_info(n)->full_children--;
> > + if (!tn_info(n)->empty_children)
> > + tn_info(n)->full_children--;
> > +
> > + tn_info(n)->empty_children--;
This change looks correct to me, so thanks for the patch and:
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
(Bikeshed; I prefer pre-inc/dec to post-inc/dec unless you really care
about the previous value. Should be irrelevant with a sufficiently
smart compiler though. ;) )
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net/ipv4: fib_trie: Avoid cryptic ternary expressions
2019-06-18 23:21 ` Matthias Kaehlcke
@ 2019-06-19 2:00 ` Alexander Duyck
0 siblings, 0 replies; 11+ messages in thread
From: Alexander Duyck @ 2019-06-19 2:00 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: Nathan Chancellor, David S . Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, Netdev, LKML, Douglas Anderson,
Nick Desaulniers, Nathan Huckleberry, clang-built-linux
On Tue, Jun 18, 2019 at 4:22 PM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> On Tue, Jun 18, 2019 at 04:04:20PM -0700, Nathan Chancellor wrote:
> > On Tue, Jun 18, 2019 at 02:14:40PM -0700, Matthias Kaehlcke wrote:
> > > empty_child_inc/dec() use the ternary operator for conditional
> > > operations. The conditions involve the post/pre in/decrement
> > > operator and the operation is only performed when the condition
> > > is *not* true. This is hard to parse for humans, use a regular
> > > 'if' construct instead and perform the in/decrement separately.
> > >
> > > This also fixes two warnings that are emitted about the value
> > > of the ternary expression being unused, when building the kernel
> > > with clang + "kbuild: Remove unnecessary -Wno-unused-value"
> > > (https://lore.kernel.org/patchwork/patch/1089869/):
> > >
> > > CC net/ipv4/fib_trie.o
> > > net/ipv4/fib_trie.c:351:2: error: expression result unused [-Werror,-Wunused-value]
> > > ++tn_info(n)->empty_children ? : ++tn_info(n)->full_children;
> > >
> >
> > As an FYI, this is also being fixed in clang:
> >
> > https://bugs.llvm.org/show_bug.cgi?id=42239
> >
> > https://reviews.llvm.org/D63369
>
> Great, thanks!
>
> In this case it was actually useful to get the warning, even though it
> didn't point out the actual bug. I think in general it would be
> preferable to avoid such constructs, even when they are correct. But
> then again, it's the reviewers/maintainers task to avoid unnecessarily
> cryptic code from slipping in, and this just happens to be one instance
> where the compiler could have helped.
So it took me a bit to remember/understand it as well since I haven't
touched the code in over 4 years, however part of that is because the
comment for this code is actually buried down in put_child.
Essentially this is just meant to be an add w/ carry and a sub w/
borrow to address a potential overflow if bits == KEYLENGTH.
If you want you can add:
Fixes: 95f60ea3e99a ("fib_trie: Add collapse() and should_collapse() to resize")
Acked-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net/ipv4: fib_trie: Avoid cryptic ternary expressions
2019-06-18 23:23 ` Nick Desaulniers
@ 2019-06-19 9:36 ` Joe Perches
2019-06-19 17:41 ` Nick Desaulniers
0 siblings, 1 reply; 11+ messages in thread
From: Joe Perches @ 2019-06-19 9:36 UTC (permalink / raw)
To: Nick Desaulniers, Matthias Kaehlcke
Cc: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Alexander Duyck, netdev, LKML, Douglas Anderson,
Nathan Huckleberry, clang-built-linux, Nathan Chancellor
On Tue, 2019-06-18 at 16:23 -0700, Nick Desaulniers wrote:
> As a side note, I'm going to try to see if MAINTAINERS and
> scripts/get_maintainers.pl supports regexes on the commit messages in
> order to cc our mailing list
Neither. Why should either?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net/ipv4: fib_trie: Avoid cryptic ternary expressions
2019-06-19 9:36 ` Joe Perches
@ 2019-06-19 17:41 ` Nick Desaulniers
2019-06-19 18:10 ` Joe Perches
0 siblings, 1 reply; 11+ messages in thread
From: Nick Desaulniers @ 2019-06-19 17:41 UTC (permalink / raw)
To: Joe Perches, Linus Torvalds
Cc: Matthias Kaehlcke, David S . Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, Alexander Duyck, netdev, LKML,
Douglas Anderson, Nathan Huckleberry, clang-built-linux,
Nathan Chancellor
On Wed, Jun 19, 2019 at 2:36 AM Joe Perches <joe@perches.com> wrote:
>
> On Tue, 2019-06-18 at 16:23 -0700, Nick Desaulniers wrote:
> > As a side note, I'm going to try to see if MAINTAINERS and
> > scripts/get_maintainers.pl supports regexes on the commit messages in
> > order to cc our mailing list
>
> Neither. Why should either?
Looks like `K:` is exactly what I'm looking for. Joe, how does:
https://github.com/ClangBuiltLinux/linux/commit/a0a64b8d65c4e7e033f49e48cc610d6e4002927e
look? Is there a maintainer for MAINTAINERS or do I just send the
patch to Linus?
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net/ipv4: fib_trie: Avoid cryptic ternary expressions
2019-06-19 17:41 ` Nick Desaulniers
@ 2019-06-19 18:10 ` Joe Perches
0 siblings, 0 replies; 11+ messages in thread
From: Joe Perches @ 2019-06-19 18:10 UTC (permalink / raw)
To: Nick Desaulniers, Linus Torvalds
Cc: Matthias Kaehlcke, David S . Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, Alexander Duyck, netdev, LKML,
Douglas Anderson, Nathan Huckleberry, clang-built-linux,
Nathan Chancellor
On Wed, 2019-06-19 at 10:41 -0700, Nick Desaulniers wrote:
> On Wed, Jun 19, 2019 at 2:36 AM Joe Perches <joe@perches.com> wrote:
> > On Tue, 2019-06-18 at 16:23 -0700, Nick Desaulniers wrote:
> > > As a side note, I'm going to try to see if MAINTAINERS and
> > > scripts/get_maintainers.pl supports regexes on the commit messages in
> > > order to cc our mailing list
> >
> > Neither. Why should either?
>
> Looks like `K:` is exactly what I'm looking for.
Using K: for commit message content isn't the intended
use, but if it works for you, fine by me.
> Joe, how does:
> https://github.com/ClangBuiltLinux/linux/commit/a0a64b8d65c4e7e033f49e48cc610d6e4002927e
> look?
You might consider using
K: \b(?i:clang|llvm)\b
to get case insensitive matches.
> Is there a maintainer for MAINTAINERS or do I just send the
> patch to Linus?
Generally MAINTAINER patches go via Andrew Morton or
indirectly along with other changes via a pull request
to Linus.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] net/ipv4: fib_trie: Avoid cryptic ternary expressions
2019-06-18 21:14 [PATCH] net/ipv4: fib_trie: Avoid cryptic ternary expressions Matthias Kaehlcke
2019-06-18 21:45 ` Doug Anderson
2019-06-18 23:04 ` Nathan Chancellor
@ 2019-06-19 21:29 ` David Miller
2 siblings, 0 replies; 11+ messages in thread
From: David Miller @ 2019-06-19 21:29 UTC (permalink / raw)
To: mka; +Cc: kuznet, yoshfuji, alexander.h.duyck, netdev, linux-kernel, dianders
From: Matthias Kaehlcke <mka@chromium.org>
Date: Tue, 18 Jun 2019 14:14:40 -0700
> empty_child_inc/dec() use the ternary operator for conditional
> operations. The conditions involve the post/pre in/decrement
> operator and the operation is only performed when the condition
> is *not* true. This is hard to parse for humans, use a regular
> 'if' construct instead and perform the in/decrement separately.
>
> This also fixes two warnings that are emitted about the value
> of the ternary expression being unused, when building the kernel
> with clang + "kbuild: Remove unnecessary -Wno-unused-value"
> (https://lore.kernel.org/patchwork/patch/1089869/):
>
> CC net/ipv4/fib_trie.o
> net/ipv4/fib_trie.c:351:2: error: expression result unused [-Werror,-Wunused-value]
> ++tn_info(n)->empty_children ? : ++tn_info(n)->full_children;
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Applied to net-next, thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-06-19 21:30 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-18 21:14 [PATCH] net/ipv4: fib_trie: Avoid cryptic ternary expressions Matthias Kaehlcke
2019-06-18 21:45 ` Doug Anderson
2019-06-18 22:57 ` Matthias Kaehlcke
2019-06-18 23:04 ` Nathan Chancellor
2019-06-18 23:21 ` Matthias Kaehlcke
2019-06-19 2:00 ` Alexander Duyck
2019-06-18 23:23 ` Nick Desaulniers
2019-06-19 9:36 ` Joe Perches
2019-06-19 17:41 ` Nick Desaulniers
2019-06-19 18:10 ` Joe Perches
2019-06-19 21:29 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).