netdev - Re: [PATCH 2/3][NET] gen_estimator: list_empty() check in est

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID:  <20080120155544.0a071675@deepthought>
Date:	Sun, 20 Jan 2008 15:55:44 -0800
From:	Stephen Hemminger <shemminger@...ux-foundation.org>
To:	netdev@...r.kernel.org
Subject:  Re: [PATCH 2/3][NET] gen_estimator: list_empty() check in
 est_timer() fixed

On Mon, 21 Jan 2008 00:49:59 +0100
Jarek Poplawski <jarkao2@...il.com> wrote:

> This patch changes the method of checking for empty list in est_timer():
> list_empty() is not recommended for RCU protected lists. Now, it's simply
> a variable used for this.
> 
> Signed-off-by: Jarek Poplawski <jarkao2@...il.com>
> 
> ---
> 
> diff -Nurp 2.6.24-rc8-mm1-p1-/net/core/gen_estimator.c 2.6.24-rc8-mm1-p1+/net/core/gen_estimator.c
> --- 2.6.24-rc8-mm1-p1-/net/core/gen_estimator.c	2008-01-20 20:58:35.000000000 +0100
> +++ 2.6.24-rc8-mm1-p1+/net/core/gen_estimator.c	2008-01-20 21:07:42.000000000 +0100
> @@ -106,6 +106,7 @@ static void est_timer(unsigned long arg)
>  {
>  	int idx = (int)arg;
>  	struct gen_estimator *e;
> +	int list_not_empty = 0;

Using a negative name for what is a boolean value leads
to code that reads like a double negative sentence. Better to choose
a variable name that is direct, can't use list_empty because that
is a macro, so how about "estimator_found".

>  
>  	rcu_read_lock();
>  	list_for_each_entry_rcu(e, &elist[idx].list, list) {
> @@ -118,6 +119,9 @@ static void est_timer(unsigned long arg)
>  		if (e->bstats == NULL)
>  			goto skip;
>  
> +		if (list_not_empty == 0)
> +			list_not_empty = 1;
> +
>  		nbytes = e->bstats->bytes;
>  		npackets = e->bstats->packets;
>  		rate = (nbytes - e->last_bytes)<<(7 - idx);
> @@ -134,7 +138,7 @@ skip:
>  		spin_unlock(e->stats_lock);
>  	}
>  
> -	if (!list_empty(&elist[idx].list))
> +	if (list_not_empty)
>  		mod_timer(&elist[idx].timer, jiffies + ((HZ/4) << idx));
>  	rcu_read_unlock();
>  }
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Stephen Hemminger <stephen.hemminger@...tta.com>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html