[B.A.T.M.A.N.] [PATCH 0/3] batman-adv: Deletion of a few unnecessary checks

[B.A.T.M.A.N.] [PATCH 0/3] batman-adv: Deletion of a few unnecessary checks

[SF Markus Elfring]

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

[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]

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

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"

[Marek Lindner]

[-- 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 --]

[B.A.T.M.A.N.] [PATCH 2/3] batman-adv: Split a condition check

[SF Markus Elfring]

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

Re: [B.A.T.M.A.N.] [PATCH 2/3] batman-adv: Split a condition check

[Marek Lindner]

[-- 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 --]

[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]

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

Re: [B.A.T.M.A.N.] [PATCH 3/3] batman-adv: Less function calls in batadv_is_ap_isolated() after error detection

[Antonio Quartulli]

[-- 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 --]

Re: [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]

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

Re: [B.A.T.M.A.N.] [PATCH 3/3] batman-adv: Less function calls in batadv_is_ap_isolated() after error detection

[Sven Eckelmann]

[-- 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 --]

[B.A.T.M.A.N.] [PATCH] batman-adv: Less function calls in batadv_is_ap_isolated() after error detection

[SF Markus Elfring]

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

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Less function calls in batadv_is_ap_isolated() after error detection

[David Miller]

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.

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Less function calls in batadv_is_ap_isolated() after error detection

[Antonio Quartulli]

[-- 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 --]

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Less function calls in batadv_is_ap_isolated() after error detection

[Sven Eckelmann]

[-- 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 --]

Re: [B.A.T.M.A.N.] batman-adv: Less function calls in batadv_is_ap_isolated() after error detection

[Sven Eckelmann]

[-- 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 --]

[B.A.T.M.A.N.] [PATCH 0/2] batman-adv: Deletion of some unnecessary checks

[SF Markus Elfring]

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

[B.A.T.M.A.N.] [PATCH 1/2] batman-adv: Delete unnecessary checks before the function call "kfree_skb"

[SF Markus Elfring]

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

Re: [B.A.T.M.A.N.] [PATCH 1/2] batman-adv: Delete unnecessary checks before the function call "kfree_skb"

[Marek Lindner]

[-- 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 --]

[B.A.T.M.A.N.] [PATCH 2/2] batman-adv: Less checks in batadv_tvlv_unicast_send()

[SF Markus Elfring]

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

Re: [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: Less checks in batadv_tvlv_unicast_send()

[Marek Lindner]

[-- 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 --]