[B.A.T.M.A.N.] [PATCH] batman-adv: do not silently ignore wrong condition

[B.A.T.M.A.N.] [PATCH] batman-adv: do not silently ignore wrong condition

[Antonio Quartulli]

Only one neigh_node per orig_node should match a given
neighbor address, therefore, if more than one matching
neigh_node is found, a WARNING has to be triggered to let
the user know that something is wrong in the originator
state instead of silently skipping the error.

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 bat_iv_ogm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c
index 38183dc..9fbf588 100644
--- a/bat_iv_ogm.c
+++ b/bat_iv_ogm.c
@@ -669,7 +669,7 @@ batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv,
 		if (batadv_compare_eth(neigh_addr, ethhdr->h_source) &&
 		    tmp_neigh_node->if_incoming == if_incoming &&
 		    atomic_inc_not_zero(&tmp_neigh_node->refcount)) {
-			if (neigh_node)
+			if (WARN(neigh_node, "too many matching neigh_nodes")
 				batadv_neigh_node_free_ref(neigh_node);
 			neigh_node = tmp_neigh_node;
 			continue;
-- 
1.8.1.5

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: do not silently ignore wrong condition

[Marek Lindner]

On Saturday, March 30, 2013 22:49:27 Antonio Quartulli wrote:
> @@ -669,7 +669,7 @@ batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv,
>                 if (batadv_compare_eth(neigh_addr, ethhdr->h_source) &&
>                     tmp_neigh_node->if_incoming == if_incoming &&
>                     atomic_inc_not_zero(&tmp_neigh_node->refcount)) {
> -                       if (neigh_node)
> +                       if (WARN(neigh_node, "too many matching
> neigh_nodes") batadv_neigh_node_free_ref(neigh_node); neigh_node =
> tmp_neigh_node;
>                         continue;

I wasn't aware one could throw the warning right in the if-statement but for 
the sake of readability, please don't do that.

Cheers,
Marek

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: do not silently ignore wrong condition

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 1311 bytes --]

On Sat, Mar 30, 2013 at 11:01:29PM +0800, Marek Lindner wrote:
> On Saturday, March 30, 2013 22:49:27 Antonio Quartulli wrote:
> > @@ -669,7 +669,7 @@ batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv,
> >                 if (batadv_compare_eth(neigh_addr, ethhdr->h_source) &&
> >                     tmp_neigh_node->if_incoming == if_incoming &&
> >                     atomic_inc_not_zero(&tmp_neigh_node->refcount)) {
> > -                       if (neigh_node)
> > +                       if (WARN(neigh_node, "too many matching
> > neigh_nodes") batadv_neigh_node_free_ref(neigh_node); neigh_node =
> > tmp_neigh_node;
> >                         continue;
> 
> I wasn't aware one could throw the warning right in the if-statement but for 
> the sake of readability, please don't do that.

We already do that in hard-interface.c:92 and WARN() has been designed to return
the value of the condition passed as argument
(http://lingrok.org/xref/linux-net-next/include/asm-generic/bug.h#94).

There are many places in the kernel were this is done the same way.
Actually this "connects" the warning to the action taken into the if block:

if "we are in trouble" do this to repair.

Cheers,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

[B.A.T.M.A.N.] [PATCHv2] batman-adv: do not silently ignore wrong condition

[Antonio Quartulli]

Only one neigh_node per orig_node should match a given
neighbor address, therefore, if more than one matching
neigh_node is found, a WARNING has to be triggered to let
the user know that something is wrong in the originator
state instead of silently skipping the error.

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---

v2:
- fixed closing parenthesis

 bat_iv_ogm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c
index 38183dc..bd50e0d 100644
--- a/bat_iv_ogm.c
+++ b/bat_iv_ogm.c
@@ -669,7 +669,7 @@ batadv_iv_ogm_orig_update(struct batadv_priv *bat_priv,
 		if (batadv_compare_eth(neigh_addr, ethhdr->h_source) &&
 		    tmp_neigh_node->if_incoming == if_incoming &&
 		    atomic_inc_not_zero(&tmp_neigh_node->refcount)) {
-			if (neigh_node)
+			if (WARN(neigh_node, "too many matching neigh_nodes"))
 				batadv_neigh_node_free_ref(neigh_node);
 			neigh_node = tmp_neigh_node;
 			continue;
-- 
1.8.1.5

Re: [B.A.T.M.A.N.] [PATCHv2] batman-adv: do not silently ignore wrong condition

[Marek Lindner]

On Sunday, March 31, 2013 00:22:00 Antonio Quartulli wrote:
> Only one neigh_node per orig_node should match a given
> neighbor address, therefore, if more than one matching
> neigh_node is found, a WARNING has to be triggered to let
> the user know that something is wrong in the originator
> state instead of silently skipping the error.
> 
> Signed-off-by: Antonio Quartulli <ordex@autistici.org>
> ---
> 
> v2:
> - fixed closing parenthesis
> 
>  bat_iv_ogm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied in revision c969b9e.

Thanks,
Marek