b.a.t.m.a.n.lists.open-mesh.org archive mirror
 help / color / mirror / Atom feed
* [B.A.T.M.A.N.] [PATCH 0/3] batman-adv: Deletion of a few unnecessary checks
       [not found]                               ` <5317A59D.4@users.sourceforge.net>
@ 2015-11-03 20:45                                 ` SF Markus Elfring
  2015-11-03 20:52                                   ` [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: Delete an unnecessary check before the function call "batadv_softif_vlan_free_ref" SF Markus Elfring
                                                     ` (2 more replies)
  2015-11-15  8:40                                 ` [B.A.T.M.A.N.] [PATCH 0/2] batman-adv: Deletion of some unnecessary checks SF Markus Elfring
  1 sibling, 3 replies; 19+ messages in thread
From: SF Markus Elfring @ 2015-11-03 20:45 UTC (permalink / raw)
  To: Antonio Quartulli, David S. Miller, Marek Lindner,
	Simon Wunderlich, b.a.t.m.a.n, netdev
  Cc: Julia Lawall, kernel-janitors, LKML

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 3 Nov 2015 21:34:29 +0100

Further update suggestions were taken into account after a patch
was applied from static source code analysis.

Markus Elfring (3):
  Delete an unnecessary check before the function call "batadv_softif_vlan_free_ref"
  Split a condition check
  Less function calls in batadv_is_ap_isolated() after error detection

 net/batman-adv/translation-table.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

-- 
2.6.2


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: Delete an unnecessary check before the function call "batadv_softif_vlan_free_ref"
  2015-11-03 20:45                                 ` [B.A.T.M.A.N.] [PATCH 0/3] batman-adv: Deletion of a few unnecessary checks SF Markus Elfring
@ 2015-11-03 20:52                                   ` SF Markus Elfring
  2015-11-21 21:48                                     ` Marek Lindner
  2015-11-03 20:54                                   ` [B.A.T.M.A.N.] [PATCH 2/3] batman-adv: Split a condition check SF Markus Elfring
  2015-11-03 20:56                                   ` [B.A.T.M.A.N.] [PATCH 3/3] batman-adv: Less function calls in batadv_is_ap_isolated() after error detection SF Markus Elfring
  2 siblings, 1 reply; 19+ messages in thread
From: SF Markus Elfring @ 2015-11-03 20:52 UTC (permalink / raw)
  To: Antonio Quartulli, David S. Miller, Marek Lindner,
	Simon Wunderlich, b.a.t.m.a.n, netdev
  Cc: Julia Lawall, kernel-janitors, LKML

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 3 Nov 2015 19:20:34 +0100

The batadv_softif_vlan_free_ref() function tests whether its argument is NULL
and then returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 net/batman-adv/translation-table.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 4228b10..48315de 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -3336,8 +3336,7 @@ bool batadv_is_ap_isolated(struct batadv_priv *bat_priv, u8 *src, u8 *dst,
 	ret = true;
 
 out:
-	if (vlan)
-		batadv_softif_vlan_free_ref(vlan);
+	batadv_softif_vlan_free_ref(vlan);
 	if (tt_global_entry)
 		batadv_tt_global_entry_free_ref(tt_global_entry);
 	if (tt_local_entry)
-- 
2.6.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [B.A.T.M.A.N.] [PATCH 2/3] batman-adv: Split a condition check
  2015-11-03 20:45                                 ` [B.A.T.M.A.N.] [PATCH 0/3] batman-adv: Deletion of a few unnecessary checks SF Markus Elfring
  2015-11-03 20:52                                   ` [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: Delete an unnecessary check before the function call "batadv_softif_vlan_free_ref" SF Markus Elfring
@ 2015-11-03 20:54                                   ` SF Markus Elfring
  2015-11-21 21:51                                     ` Marek Lindner
  2015-11-03 20:56                                   ` [B.A.T.M.A.N.] [PATCH 3/3] batman-adv: Less function calls in batadv_is_ap_isolated() after error detection SF Markus Elfring
  2 siblings, 1 reply; 19+ messages in thread
From: SF Markus Elfring @ 2015-11-03 20:54 UTC (permalink / raw)
  To: Antonio Quartulli, David S. Miller, Marek Lindner,
	Simon Wunderlich, b.a.t.m.a.n, netdev
  Cc: Julia Lawall, kernel-janitors, LKML

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 3 Nov 2015 20:41:02 +0100

Let us split a check for a condition at the beginning of the
batadv_is_ap_isolated() function so that a direct return can be performed
in this function if the variable "vlan" contained a null pointer.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 net/batman-adv/translation-table.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 48315de..965a004 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -3319,7 +3319,10 @@ bool batadv_is_ap_isolated(struct batadv_priv *bat_priv, u8 *src, u8 *dst,
 	bool ret = false;
 
 	vlan = batadv_softif_vlan_get(bat_priv, vid);
-	if (!vlan || !atomic_read(&vlan->ap_isolation))
+	if (!vlan)
+		return false;
+
+	if (!atomic_read(&vlan->ap_isolation))
 		goto out;
 
 	tt_local_entry = batadv_tt_local_hash_find(bat_priv, dst, vid);
-- 
2.6.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [B.A.T.M.A.N.] [PATCH 3/3] batman-adv: Less function calls in batadv_is_ap_isolated() after error detection
  2015-11-03 20:45                                 ` [B.A.T.M.A.N.] [PATCH 0/3] batman-adv: Deletion of a few unnecessary checks SF Markus Elfring
  2015-11-03 20:52                                   ` [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: Delete an unnecessary check before the function call "batadv_softif_vlan_free_ref" SF Markus Elfring
  2015-11-03 20:54                                   ` [B.A.T.M.A.N.] [PATCH 2/3] batman-adv: Split a condition check SF Markus Elfring
@ 2015-11-03 20:56                                   ` SF Markus Elfring
  2015-11-20  8:47                                     ` Antonio Quartulli
  2 siblings, 1 reply; 19+ messages in thread
From: SF Markus Elfring @ 2015-11-03 20:56 UTC (permalink / raw)
  To: Antonio Quartulli, David S. Miller, Marek Lindner,
	Simon Wunderlich, b.a.t.m.a.n, netdev
  Cc: Julia Lawall, kernel-janitors, LKML

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 3 Nov 2015 21:10:51 +0100

The variables "tt_local_entry" and "tt_global_entry" were eventually checked
again despite of a corresponding null pointer test before.
Let us avoid this double check by reordering a function call sequence
and the better selection of jump targets.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 net/batman-adv/translation-table.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 965a004..3ac32d9 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -3323,27 +3323,24 @@ bool batadv_is_ap_isolated(struct batadv_priv *bat_priv, u8 *src, u8 *dst,
 		return false;
 
 	if (!atomic_read(&vlan->ap_isolation))
-		goto out;
+		goto vlan_free;
 
 	tt_local_entry = batadv_tt_local_hash_find(bat_priv, dst, vid);
 	if (!tt_local_entry)
-		goto out;
+		goto vlan_free;
 
 	tt_global_entry = batadv_tt_global_hash_find(bat_priv, src, vid);
 	if (!tt_global_entry)
-		goto out;
+		goto local_entry_free;
 
-	if (!_batadv_is_ap_isolated(tt_local_entry, tt_global_entry))
-		goto out;
-
-	ret = true;
+	if (_batadv_is_ap_isolated(tt_local_entry, tt_global_entry))
+		ret = true;
 
-out:
+	batadv_tt_global_entry_free_ref(tt_global_entry);
+local_entry_free:
+	batadv_tt_local_entry_free_ref(tt_local_entry);
+vlan_free:
 	batadv_softif_vlan_free_ref(vlan);
-	if (tt_global_entry)
-		batadv_tt_global_entry_free_ref(tt_global_entry);
-	if (tt_local_entry)
-		batadv_tt_local_entry_free_ref(tt_local_entry);
 	return ret;
 }
 
-- 
2.6.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [B.A.T.M.A.N.] [PATCH 0/2] batman-adv: Deletion of some unnecessary checks
       [not found]                               ` <5317A59D.4@users.sourceforge.net>
  2015-11-03 20:45                                 ` [B.A.T.M.A.N.] [PATCH 0/3] batman-adv: Deletion of a few unnecessary checks SF Markus Elfring
@ 2015-11-15  8:40                                 ` SF Markus Elfring
  2015-11-15  8:43                                   ` [B.A.T.M.A.N.] [PATCH 1/2] batman-adv: Delete unnecessary checks before the function call "kfree_skb" SF Markus Elfring
  2015-11-15  8:45                                   ` [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: Less checks in batadv_tvlv_unicast_send() SF Markus Elfring
  1 sibling, 2 replies; 19+ messages in thread
From: SF Markus Elfring @ 2015-11-15  8:40 UTC (permalink / raw)
  To: Antonio Quartulli, David S. Miller, Marek Lindner,
	Simon Wunderlich, b.a.t.m.a.n, netdev
  Cc: Julia Lawall, kernel-janitors, LKML

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 15 Nov 2015 09:34:12 +0100

Another update suggestion was taken into account after a patch was applied
from static source code analysis.

Markus Elfring (2):
  Delete unnecessary checks before the function call "kfree_skb"
  Less checks in batadv_tvlv_unicast_send()

 net/batman-adv/main.c           | 15 +++++----------
 net/batman-adv/network-coding.c |  4 +---
 net/batman-adv/send.c           |  3 +--
 3 files changed, 7 insertions(+), 15 deletions(-)

-- 
2.6.2

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [B.A.T.M.A.N.] [PATCH 1/2] batman-adv: Delete unnecessary checks before the function call "kfree_skb"
  2015-11-15  8:40                                 ` [B.A.T.M.A.N.] [PATCH 0/2] batman-adv: Deletion of some unnecessary checks SF Markus Elfring
@ 2015-11-15  8:43                                   ` SF Markus Elfring
  2015-11-21 21:45                                     ` Marek Lindner
  2015-11-15  8:45                                   ` [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: Less checks in batadv_tvlv_unicast_send() SF Markus Elfring
  1 sibling, 1 reply; 19+ messages in thread
From: SF Markus Elfring @ 2015-11-15  8:43 UTC (permalink / raw)
  To: Antonio Quartulli, David S. Miller, Marek Lindner,
	Simon Wunderlich, b.a.t.m.a.n, netdev
  Cc: Julia Lawall, kernel-janitors, LKML

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 15 Nov 2015 08:04:43 +0100

The kfree_skb() function tests whether its argument is NULL and then
returns immediately. Thus the test around the calls is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 net/batman-adv/main.c           | 2 +-
 net/batman-adv/network-coding.c | 4 +---
 net/batman-adv/send.c           | 3 +--
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index d7f17c1..9e9b8f6 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -1184,7 +1184,7 @@ void batadv_tvlv_unicast_send(struct batadv_priv *bat_priv, u8 *src,
 		ret = true;
 
 out:
-	if (skb && !ret)
+	if (!ret)
 		kfree_skb(skb);
 	if (orig_node)
 		batadv_orig_node_free_ref(orig_node);
diff --git a/net/batman-adv/network-coding.c b/net/batman-adv/network-coding.c
index f5276be..c98b0ab 100644
--- a/net/batman-adv/network-coding.c
+++ b/net/batman-adv/network-coding.c
@@ -244,9 +244,7 @@ static void batadv_nc_path_free_ref(struct batadv_nc_path *nc_path)
  */
 static void batadv_nc_packet_free(struct batadv_nc_packet *nc_packet)
 {
-	if (nc_packet->skb)
-		kfree_skb(nc_packet->skb);
-
+	kfree_skb(nc_packet->skb);
 	batadv_nc_path_free_ref(nc_packet->nc_path);
 	kfree(nc_packet);
 }
diff --git a/net/batman-adv/send.c b/net/batman-adv/send.c
index f664324..782fa33 100644
--- a/net/batman-adv/send.c
+++ b/net/batman-adv/send.c
@@ -407,8 +407,7 @@ void batadv_schedule_bat_ogm(struct batadv_hard_iface *hard_iface)
 
 static void batadv_forw_packet_free(struct batadv_forw_packet *forw_packet)
 {
-	if (forw_packet->skb)
-		kfree_skb(forw_packet->skb);
+	kfree_skb(forw_packet->skb);
 	if (forw_packet->if_incoming)
 		batadv_hardif_free_ref(forw_packet->if_incoming);
 	if (forw_packet->if_outgoing)
-- 
2.6.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: Less checks in batadv_tvlv_unicast_send()
  2015-11-15  8:40                                 ` [B.A.T.M.A.N.] [PATCH 0/2] batman-adv: Deletion of some unnecessary checks SF Markus Elfring
  2015-11-15  8:43                                   ` [B.A.T.M.A.N.] [PATCH 1/2] batman-adv: Delete unnecessary checks before the function call "kfree_skb" SF Markus Elfring
@ 2015-11-15  8:45                                   ` SF Markus Elfring
  2015-11-21 21:47                                     ` Marek Lindner
  1 sibling, 1 reply; 19+ messages in thread
From: SF Markus Elfring @ 2015-11-15  8:45 UTC (permalink / raw)
  To: Antonio Quartulli, David S. Miller, Marek Lindner,
	Simon Wunderlich, b.a.t.m.a.n, netdev
  Cc: Julia Lawall, kernel-janitors, LKML

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 15 Nov 2015 09:00:42 +0100

* Let us return directly if a call of the batadv_orig_hash_find() function
  returned a null pointer.

* Omit the initialisation for the variable "skb" at the beginning.

* Replace an assignment by a call of the kfree_skb() function
  and delete the affected variable "ret" then.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 net/batman-adv/main.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index 9e9b8f6..4eaa09a 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -1143,15 +1143,14 @@ void batadv_tvlv_unicast_send(struct batadv_priv *bat_priv, u8 *src,
 	struct batadv_unicast_tvlv_packet *unicast_tvlv_packet;
 	struct batadv_tvlv_hdr *tvlv_hdr;
 	struct batadv_orig_node *orig_node;
-	struct sk_buff *skb = NULL;
+	struct sk_buff *skb;
 	unsigned char *tvlv_buff;
 	unsigned int tvlv_len;
 	ssize_t hdr_len = sizeof(*unicast_tvlv_packet);
-	bool ret = false;
 
 	orig_node = batadv_orig_hash_find(bat_priv, dst);
 	if (!orig_node)
-		goto out;
+		return;
 
 	tvlv_len = sizeof(*tvlv_hdr) + tvlv_value_len;
 
@@ -1180,14 +1179,10 @@ void batadv_tvlv_unicast_send(struct batadv_priv *bat_priv, u8 *src,
 	tvlv_buff += sizeof(*tvlv_hdr);
 	memcpy(tvlv_buff, tvlv_value, tvlv_value_len);
 
-	if (batadv_send_skb_to_orig(skb, orig_node, NULL) != NET_XMIT_DROP)
-		ret = true;
-
-out:
-	if (!ret)
+	if (batadv_send_skb_to_orig(skb, orig_node, NULL) == NET_XMIT_DROP)
 		kfree_skb(skb);
-	if (orig_node)
-		batadv_orig_node_free_ref(orig_node);
+out:
+	batadv_orig_node_free_ref(orig_node);
 }
 
 /**
-- 
2.6.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH 3/3] batman-adv: Less function calls in batadv_is_ap_isolated() after error detection
  2015-11-03 20:56                                   ` [B.A.T.M.A.N.] [PATCH 3/3] batman-adv: Less function calls in batadv_is_ap_isolated() after error detection SF Markus Elfring
@ 2015-11-20  8:47                                     ` Antonio Quartulli
  2015-11-20 10:56                                       ` SF Markus Elfring
  0 siblings, 1 reply; 19+ messages in thread
From: Antonio Quartulli @ 2015-11-20  8:47 UTC (permalink / raw)
  To: elfring
  Cc: Marek Lindner, netdev,
	The list for a Better Approach To Mobile Ad-hoc Networking,
	kernel-janitors, LKML, Julia Lawall, David S. Miller

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

On 04/11/15 04:56, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 3 Nov 2015 21:10:51 +0100
> 
> The variables "tt_local_entry" and "tt_global_entry" were eventually checked
> again despite of a corresponding null pointer test before.
> Let us avoid this double check by reordering a function call sequence
> and the better selection of jump targets.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  net/batman-adv/translation-table.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
> index 965a004..3ac32d9 100644
> --- a/net/batman-adv/translation-table.c
> +++ b/net/batman-adv/translation-table.c
> @@ -3323,27 +3323,24 @@ bool batadv_is_ap_isolated(struct batadv_priv *bat_priv, u8 *src, u8 *dst,
>  		return false;
>  
>  	if (!atomic_read(&vlan->ap_isolation))
> -		goto out;
> +		goto vlan_free;
>  
>  	tt_local_entry = batadv_tt_local_hash_find(bat_priv, dst, vid);
>  	if (!tt_local_entry)
> -		goto out;
> +		goto vlan_free;
>  
>  	tt_global_entry = batadv_tt_global_hash_find(bat_priv, src, vid);
>  	if (!tt_global_entry)
> -		goto out;
> +		goto local_entry_free;
>  
> -	if (!_batadv_is_ap_isolated(tt_local_entry, tt_global_entry))
> -		goto out;
> -
> -	ret = true;
> +	if (_batadv_is_ap_isolated(tt_local_entry, tt_global_entry))
> +		ret = true;
>  
> -out:
> +	batadv_tt_global_entry_free_ref(tt_global_entry);
> +local_entry_free:
> +	batadv_tt_local_entry_free_ref(tt_local_entry);
> +vlan_free:
>  	batadv_softif_vlan_free_ref(vlan);
> -	if (tt_global_entry)
> -		batadv_tt_global_entry_free_ref(tt_global_entry);
> -	if (tt_local_entry)
> -		batadv_tt_local_entry_free_ref(tt_local_entry);
>  	return ret;

Markus,
if you really want to make this codestyle change, I'd suggest you to go
through the whole batman-adv code and apply the same change where
needed. It does not make sense to change the codestyle in one spot only.

On top of that, by going through the batman-adv code you might agree
that the current style is actually not a bad idea.


Cheers,

-- 
Antonio Quartulli


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH 3/3] batman-adv: Less function calls in batadv_is_ap_isolated() after error detection
  2015-11-20  8:47                                     ` Antonio Quartulli
@ 2015-11-20 10:56                                       ` SF Markus Elfring
  2016-03-10 19:30                                         ` Sven Eckelmann
  0 siblings, 1 reply; 19+ messages in thread
From: SF Markus Elfring @ 2015-11-20 10:56 UTC (permalink / raw)
  To: Antonio Quartulli
  Cc: Marek Lindner, netdev,
	The list for a Better Approach To Mobile Ad-hoc Networking,
	kernel-janitors, LKML, Julia Lawall, David S. Miller

>> -out:
>> +	batadv_tt_global_entry_free_ref(tt_global_entry);
>> +local_entry_free:
>> +	batadv_tt_local_entry_free_ref(tt_local_entry);
>> +vlan_free:
>>  	batadv_softif_vlan_free_ref(vlan);
>> -	if (tt_global_entry)
>> -		batadv_tt_global_entry_free_ref(tt_global_entry);
>> -	if (tt_local_entry)
>> -		batadv_tt_local_entry_free_ref(tt_local_entry);
>>  	return ret;

> if you really want to make this codestyle change, I'd suggest you to go
> through the whole batman-adv code and apply the same change where needed.

Thanks for your interest in similar source code changes.

I would prefer general acceptance for this specific update suggestion
before I might invest further software development efforts for the
affected network module.


> It does not make sense to change the codestyle in one spot only.

I agree in the way that I would be nice if more places can still be improved.


> On top of that, by going through the batman-adv code you might agree
> that the current style is actually not a bad idea.

I got the impression that the current Linux coding style convention
disagrees around the affected jump label selection to some degree,
doesn't it?

Regards,
Markus

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH 1/2] batman-adv: Delete unnecessary checks before the function call "kfree_skb"
  2015-11-15  8:43                                   ` [B.A.T.M.A.N.] [PATCH 1/2] batman-adv: Delete unnecessary checks before the function call "kfree_skb" SF Markus Elfring
@ 2015-11-21 21:45                                     ` Marek Lindner
  0 siblings, 0 replies; 19+ messages in thread
From: Marek Lindner @ 2015-11-21 21:45 UTC (permalink / raw)
  To: b.a.t.m.a.n
  Cc: netdev, kernel-janitors, LKML, David S. Miller, Julia Lawall,
	Antonio Quartulli, SF Markus Elfring

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

On Sunday, November 15, 2015 09:43:26 SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 15 Nov 2015 08:04:43 +0100
> 
> The kfree_skb() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the calls is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  net/batman-adv/main.c           | 2 +-
>  net/batman-adv/network-coding.c | 4 +---
>  net/batman-adv/send.c           | 3 +--
>  3 files changed, 3 insertions(+), 6 deletions(-)

Applied in revision 77d84a6.

Thanks,
Marek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: Less checks in batadv_tvlv_unicast_send()
  2015-11-15  8:45                                   ` [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: Less checks in batadv_tvlv_unicast_send() SF Markus Elfring
@ 2015-11-21 21:47                                     ` Marek Lindner
  0 siblings, 0 replies; 19+ messages in thread
From: Marek Lindner @ 2015-11-21 21:47 UTC (permalink / raw)
  To: b.a.t.m.a.n
  Cc: netdev, kernel-janitors, LKML, David S. Miller, Julia Lawall,
	Antonio Quartulli, SF Markus Elfring

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

On Sunday, November 15, 2015 09:45:51 SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 15 Nov 2015 09:00:42 +0100
> 
> * Let us return directly if a call of the batadv_orig_hash_find() function
>   returned a null pointer.
> 
> * Omit the initialisation for the variable "skb" at the beginning.
> 
> * Replace an assignment by a call of the kfree_skb() function
>   and delete the affected variable "ret" then.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  net/batman-adv/main.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)

Applied in revision 5a878b8.

Thanks,
Marek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: Delete an unnecessary check before the function call "batadv_softif_vlan_free_ref"
  2015-11-03 20:52                                   ` [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: Delete an unnecessary check before the function call "batadv_softif_vlan_free_ref" SF Markus Elfring
@ 2015-11-21 21:48                                     ` Marek Lindner
  0 siblings, 0 replies; 19+ messages in thread
From: Marek Lindner @ 2015-11-21 21:48 UTC (permalink / raw)
  To: b.a.t.m.a.n
  Cc: netdev, kernel-janitors, LKML, David S. Miller, Julia Lawall,
	Antonio Quartulli, SF Markus Elfring

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

On Tuesday, November 03, 2015 21:52:58 SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 3 Nov 2015 19:20:34 +0100
> 
> The batadv_softif_vlan_free_ref() function tests whether its argument is
> NULL and then returns immediately. Thus the test around the call is not
> needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  net/batman-adv/translation-table.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Applied in revision bbcbe0f.

Thanks,
Marek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH 2/3] batman-adv: Split a condition check
  2015-11-03 20:54                                   ` [B.A.T.M.A.N.] [PATCH 2/3] batman-adv: Split a condition check SF Markus Elfring
@ 2015-11-21 21:51                                     ` Marek Lindner
  0 siblings, 0 replies; 19+ messages in thread
From: Marek Lindner @ 2015-11-21 21:51 UTC (permalink / raw)
  To: b.a.t.m.a.n
  Cc: netdev, kernel-janitors, LKML, David S. Miller, Julia Lawall,
	Antonio Quartulli, SF Markus Elfring

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

On Tuesday, November 03, 2015 21:54:35 SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 3 Nov 2015 20:41:02 +0100
> 
> Let us split a check for a condition at the beginning of the
> batadv_is_ap_isolated() function so that a direct return can be performed
> in this function if the variable "vlan" contained a null pointer.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  net/batman-adv/translation-table.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Applied in revision b1199c6.

Thanks,
Marek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH 3/3] batman-adv: Less function calls in batadv_is_ap_isolated() after error detection
  2015-11-20 10:56                                       ` SF Markus Elfring
@ 2016-03-10 19:30                                         ` Sven Eckelmann
  2016-03-11 12:40                                           ` [B.A.T.M.A.N.] [PATCH] " SF Markus Elfring
  0 siblings, 1 reply; 19+ messages in thread
From: Sven Eckelmann @ 2016-03-10 19:30 UTC (permalink / raw)
  To: SF Markus Elfring, kernel-janitors
  Cc: Julia Lawall, b.a.t.m.a.n, Antonio Quartulli, Marek Lindner

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

On Friday 20 November 2015 11:56:39 SF Markus Elfring wrote:
> > On top of that, by going through the batman-adv code you might agree
> > that the current style is actually not a bad idea.
> 
> I got the impression that the current Linux coding style convention
> disagrees around the affected jump label selection to some degree,
> doesn't it?

Yes, see "Chapter 7: Centralized exiting of functions". But your patch
doesn't seem to apply anymore. Can you please resent it or mark it
correctly in patchwork [1].

There was also another suggestion in the past:

 * https://patchwork.open-mesh.org/patch/4081/ [2]

I have prepared a overview of functions and their goto's [3] to make
it easier to spot interesting places.

Kind regards,
	Sven

[1] https://patchwork.open-mesh.org/patch/4724/
    You can find an updated version at
    https://git.open-mesh.org/batman-adv.git/patch/1b79cb12821da928b4cf2d116469dfdcbe66d8cd
[2] Update version:
    https://git.open-mesh.org/batman-adv.git/commit/28578c9dd0e592f1c69c44ab7723021b1e8c5b28
[3] https://git.open-mesh.org/batman-adv.git/blob/783c827d01b3644ac09a0f32fe4a9dfa8d6debd3:/TODO

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [B.A.T.M.A.N.] [PATCH] batman-adv: Less function calls in batadv_is_ap_isolated() after error detection
  2016-03-10 19:30                                         ` Sven Eckelmann
@ 2016-03-11 12:40                                           ` SF Markus Elfring
  2016-03-14 19:25                                             ` David Miller
                                                               ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: SF Markus Elfring @ 2016-03-11 12:40 UTC (permalink / raw)
  To: b.a.t.m.a.n, netdev, Antonio Quartulli, David S. Miller,
	Marek Lindner, Simon Wunderlich, Sven Eckelmann
  Cc: Julia Lawall, kernel-janitors, linux-kernel

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 11 Mar 2016 13:10:20 +0100

The variables "tt_local_entry" and "tt_global_entry" were eventually
checked again despite of a corresponding null pointer test before.

* Avoid this double check by reordering a function call sequence
  and the better selection of jump targets.

* Omit the initialisation for these variables at the beginning then.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 net/batman-adv/translation-table.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 0b43e86..9c0193ee 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -3403,8 +3403,8 @@ void batadv_tt_local_commit_changes(struct batadv_priv *bat_priv)
 bool batadv_is_ap_isolated(struct batadv_priv *bat_priv, u8 *src, u8 *dst,
 			   unsigned short vid)
 {
-	struct batadv_tt_local_entry *tt_local_entry = NULL;
-	struct batadv_tt_global_entry *tt_global_entry = NULL;
+	struct batadv_tt_local_entry *tt_local_entry;
+	struct batadv_tt_global_entry *tt_global_entry;
 	struct batadv_softif_vlan *vlan;
 	bool ret = false;
 
@@ -3413,27 +3413,24 @@ bool batadv_is_ap_isolated(struct batadv_priv *bat_priv, u8 *src, u8 *dst,
 		return false;
 
 	if (!atomic_read(&vlan->ap_isolation))
-		goto out;
+		goto vlan_put;
 
 	tt_local_entry = batadv_tt_local_hash_find(bat_priv, dst, vid);
 	if (!tt_local_entry)
-		goto out;
+		goto vlan_put;
 
 	tt_global_entry = batadv_tt_global_hash_find(bat_priv, src, vid);
 	if (!tt_global_entry)
-		goto out;
-
-	if (!_batadv_is_ap_isolated(tt_local_entry, tt_global_entry))
-		goto out;
+		goto local_entry_put;
 
-	ret = true;
+	if (_batadv_is_ap_isolated(tt_local_entry, tt_global_entry))
+		ret = true;
 
-out:
+	batadv_tt_global_entry_put(tt_global_entry);
+local_entry_put:
+	batadv_tt_local_entry_put(tt_local_entry);
+vlan_put:
 	batadv_softif_vlan_put(vlan);
-	if (tt_global_entry)
-		batadv_tt_global_entry_put(tt_global_entry);
-	if (tt_local_entry)
-		batadv_tt_local_entry_put(tt_local_entry);
 	return ret;
 }
 
-- 
2.7.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Less function calls in batadv_is_ap_isolated() after error detection
  2016-03-11 12:40                                           ` [B.A.T.M.A.N.] [PATCH] " SF Markus Elfring
@ 2016-03-14 19:25                                             ` David Miller
  2016-03-15  0:14                                               ` Antonio Quartulli
  2016-05-31 17:57                                             ` Sven Eckelmann
  2016-10-18 12:44                                             ` [B.A.T.M.A.N.] " Sven Eckelmann
  2 siblings, 1 reply; 19+ messages in thread
From: David Miller @ 2016-03-14 19:25 UTC (permalink / raw)
  To: elfring
  Cc: kernel-janitors, mareklindner, netdev, b.a.t.m.a.n, a,
	linux-kernel, julia.lawall

From: SF Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 11 Mar 2016 13:40:56 +0100

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 11 Mar 2016 13:10:20 +0100
> 
> The variables "tt_local_entry" and "tt_global_entry" were eventually
> checked again despite of a corresponding null pointer test before.
> 
> * Avoid this double check by reordering a function call sequence
>   and the better selection of jump targets.
> 
> * Omit the initialisation for these variables at the beginning then.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

I am assuming Antonio will take this in via his tree.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Less function calls in batadv_is_ap_isolated() after error detection
  2016-03-14 19:25                                             ` David Miller
@ 2016-03-15  0:14                                               ` Antonio Quartulli
  0 siblings, 0 replies; 19+ messages in thread
From: Antonio Quartulli @ 2016-03-15  0:14 UTC (permalink / raw)
  To: David Miller
  Cc: mareklindner, netdev, b.a.t.m.a.n, kernel-janitors, linux-kernel,
	julia.lawall, elfring

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

On Mon, Mar 14, 2016 at 03:25:02PM -0400, David Miller wrote:
> From: SF Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 11 Mar 2016 13:40:56 +0100
> 
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Fri, 11 Mar 2016 13:10:20 +0100
> > 
> > The variables "tt_local_entry" and "tt_global_entry" were eventually
> > checked again despite of a corresponding null pointer test before.
> > 
> > * Avoid this double check by reordering a function call sequence
> >   and the better selection of jump targets.
> > 
> > * Omit the initialisation for these variables at the beginning then.
> > 
> > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> 
> I am assuming Antonio will take this in via his tree.
> 

Yeah, it will go through our tree. Still under review right now.

Cheers,

-- 
Antonio Quartulli

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Less function calls in batadv_is_ap_isolated() after error detection
  2016-03-11 12:40                                           ` [B.A.T.M.A.N.] [PATCH] " SF Markus Elfring
  2016-03-14 19:25                                             ` David Miller
@ 2016-05-31 17:57                                             ` Sven Eckelmann
  2016-10-18 12:44                                             ` [B.A.T.M.A.N.] " Sven Eckelmann
  2 siblings, 0 replies; 19+ messages in thread
From: Sven Eckelmann @ 2016-05-31 17:57 UTC (permalink / raw)
  To: b.a.t.m.a.n

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

On Friday 11 March 2016 13:40:56 SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 11 Mar 2016 13:10:20 +0100
> 
> The variables "tt_local_entry" and "tt_global_entry" were eventually
> checked again despite of a corresponding null pointer test before.
> 
> * Avoid this double check by reordering a function call sequence
>   and the better selection of jump targets.
> 
> * Omit the initialisation for these variables at the beginning then.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---

Reviewed-by: Sven Eckelmann <sven@narfation.org>

See 

 * https://www.kernel.org/doc/Documentation/SubmittingPatches
   Chapter 7: Centralized exiting of functions
 * https://plus.google.com/106378716002406849458/posts/dnanfhQ4mHQ
 * http://article.gmane.org/gmane.linux.documentation/28552


Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [B.A.T.M.A.N.] batman-adv: Less function calls in batadv_is_ap_isolated() after error detection
  2016-03-11 12:40                                           ` [B.A.T.M.A.N.] [PATCH] " SF Markus Elfring
  2016-03-14 19:25                                             ` David Miller
  2016-05-31 17:57                                             ` Sven Eckelmann
@ 2016-10-18 12:44                                             ` Sven Eckelmann
  2 siblings, 0 replies; 19+ messages in thread
From: Sven Eckelmann @ 2016-10-18 12:44 UTC (permalink / raw)
  To: SF Markus Elfring; +Cc: Julia Lawall, b.a.t.m.a.n, kernel-janitors

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

On Freitag, 11. März 2016 13:40:56 CEST SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 11 Mar 2016 13:10:20 +0100
> 
> The variables "tt_local_entry" and "tt_global_entry" were eventually
> checked again despite of a corresponding null pointer test before.
> 
> * Avoid this double check by reordering a function call sequence
>   and the better selection of jump targets.
> 
> * Omit the initialisation for these variables at the beginning then.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> Reviewed-by: Sven Eckelmann <sven@narfation.org>
> ---
>  net/batman-adv/translation-table.c | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)

Applied in 58d8ef2dd0c9f7274f0473a1b1d1ffdacdd914f6 [1].

Kind regards,
	Sven

[1] https://git.open-mesh.org/batman-adv.git/commit/58d8ef2dd0c9f7274f0473a1b1d1ffdacdd914f6

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2016-10-18 12:44 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <5307CAA2.8060406@users.sourceforge.net>
     [not found] ` <alpine.DEB.2.02.1402212321410.2043@localhost6.localdomain6>
     [not found]   ` <530A086E.8010901@users.sourceforge.net>
     [not found]     ` <alpine.DEB.2.02.1402231635510.1985@localhost6.localdomain6>
     [not found]       ` <530A72AA.3000601@users.sourceforge.net>
     [not found]         ` <alpine.DEB.2.02.1402240658210.2090@localhost6.localdomain6>
     [not found]           ` <530B5FB6.6010207@users.sourceforge.net>
     [not found]             ` <alpine.DEB.2.10.1402241710370.2074@hadrien>
     [not found]               ` <530C5E18.1020800@users.sourceforge.net>
     [not found]                 ` <alpine.DEB.2.10.1402251014170.2080@hadrien>
     [not found]                   ` <530CD2C4.4050903@users.sourceforge.net>
     [not found]                     ` <alpine.DEB.2.10.1402251840450.7035@hadrien>
     [not found]                       ` <530CF8FF.8080600@users.sourceforge.net>
     [not found]                         ` <alpine.DEB.2.02.1402252117150.2047@localhost6.localdomain6>
     [not found]                           ` <530DD06F.4090703@users.sourceforge.net>
     [not found]                             ` <alpine.DEB.2.02.1402262129250.2221@localhost6.localdomain6>
     [not found]                               ` <5317A59D.4@users.sourceforge.net>
2015-11-03 20:45                                 ` [B.A.T.M.A.N.] [PATCH 0/3] batman-adv: Deletion of a few unnecessary checks SF Markus Elfring
2015-11-03 20:52                                   ` [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: Delete an unnecessary check before the function call "batadv_softif_vlan_free_ref" SF Markus Elfring
2015-11-21 21:48                                     ` Marek Lindner
2015-11-03 20:54                                   ` [B.A.T.M.A.N.] [PATCH 2/3] batman-adv: Split a condition check SF Markus Elfring
2015-11-21 21:51                                     ` Marek Lindner
2015-11-03 20:56                                   ` [B.A.T.M.A.N.] [PATCH 3/3] batman-adv: Less function calls in batadv_is_ap_isolated() after error detection SF Markus Elfring
2015-11-20  8:47                                     ` Antonio Quartulli
2015-11-20 10:56                                       ` SF Markus Elfring
2016-03-10 19:30                                         ` Sven Eckelmann
2016-03-11 12:40                                           ` [B.A.T.M.A.N.] [PATCH] " SF Markus Elfring
2016-03-14 19:25                                             ` David Miller
2016-03-15  0:14                                               ` Antonio Quartulli
2016-05-31 17:57                                             ` Sven Eckelmann
2016-10-18 12:44                                             ` [B.A.T.M.A.N.] " Sven Eckelmann
2015-11-15  8:40                                 ` [B.A.T.M.A.N.] [PATCH 0/2] batman-adv: Deletion of some unnecessary checks SF Markus Elfring
2015-11-15  8:43                                   ` [B.A.T.M.A.N.] [PATCH 1/2] batman-adv: Delete unnecessary checks before the function call "kfree_skb" SF Markus Elfring
2015-11-21 21:45                                     ` Marek Lindner
2015-11-15  8:45                                   ` [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: Less checks in batadv_tvlv_unicast_send() SF Markus Elfring
2015-11-21 21:47                                     ` Marek Lindner

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).