netdev - Re: [PATCH] tcp: fix comment for tp->highest

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: <alpine.DEB.2.00.1202280053320.17924@melkinpaasi.cs.helsinki.fi>
Date:	Tue, 28 Feb 2012 00:56:41 +0200 (EET)
From:	"Ilpo Järvinen" <ilpo.jarvinen@...sinki.fi>
To:	Neal Cardwell <ncardwell@...gle.com>
cc:	David Miller <davem@...emloft.net>,
	Netdev <netdev@...r.kernel.org>,
	Nandita Dukkipati <nanditad@...gle.com>,
	Yuchung Cheng <ycheng@...gle.com>,
	Tom Herbert <therbert@...gle.com>
Subject: Re: [PATCH] tcp: fix comment for tp->highest_sack

On Mon, 27 Feb 2012, Neal Cardwell wrote:

> There was an off-by-one error in the comments describing the
> highest_sack field in struct tcp_sock. The comments previously claimed
> that it was the "start sequence of the highest skb with SACKed
> bit". This commit fixes the comments to note that it is the "start
> sequence of the skb just *after* the highest skb with SACKed bit".
> 
> Signed-off-by: Neal Cardwell <ncardwell@...gle.com>
> ---
>  include/linux/tcp.h |    3 ++-
>  include/net/tcp.h   |    5 +++--
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index 46a85c9..3c7ffdb 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -412,7 +412,8 @@ struct tcp_sock {
>  
>  	struct tcp_sack_block recv_sack_cache[4];
>  
> -	struct sk_buff *highest_sack;   /* highest skb with SACK received
> +	struct sk_buff *highest_sack;   /* skb just after the highest
> +					 * skb with SACKed bit set
>  					 * (validity guaranteed only if
>  					 * sacked_out > 0)
>  					 */
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 42c29bf..2d80c29 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -1364,8 +1364,9 @@ static inline void tcp_push_pending_frames(struct sock *sk)
>  	}
>  }
>  
> -/* Start sequence of the highest skb with SACKed bit, valid only if
> - * sacked > 0 or when the caller has ensured validity by itself.
> +/* Start sequence of the skb just after the highest skb with SACKed
> + * bit, valid only if sacked_out > 0 or when the caller has ensured
> + * validity by itself.
>   */
>  static inline u32 tcp_highest_sack_seq(struct tcp_sock *tp)
>  {

Right, commenting code in the first place is evil as we see. ...I remember 
changing this at some point, perhaps already before the submission but 
never managed to detect the errors in those non-compile tested
comments. :-)

Acked-by: Ilpo Järvinen <ilpo.jarvinen@...sinki.fi>

-- 
 i.