linux-kernel - Re: [B.A.T.M.A.N.] [PATCH 3/3] batman-adv: Less function calls in batadv_is_ap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <564EDE1D.7070809@meshcoding.com>
Date:	Fri, 20 Nov 2015 16:47:25 +0800
From:	Antonio Quartulli <antonio@...hcoding.com>
To:	elfring@...rs.sourceforge.net
Cc:	The list for a Better Approach To Mobile Ad-hoc
	 Networking <b.a.t.m.a.n@...ts.open-mesh.org>,
	"David S. Miller" <davem@...emloft.net>,
	Marek Lindner <mareklindner@...mailbox.ch>,
	Simon Wunderlich <sw@...onwunderlich.de>,
	netdev@...r.kernel.org, Julia Lawall <julia.lawall@...6.fr>,
	kernel-janitors@...r.kernel.org,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [B.A.T.M.A.N.] [PATCH 3/3] batman-adv: Less function calls in
 batadv_is_ap_isolated() after error detection

On 04/11/15 04:56, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@...rs.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@...rs.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


Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)