linux-kernel - Re: [PATCH v5 1/5] netdev: ethernet dev_alloc_skb to netdev_alloc

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: <20120206.114505.1068389709235471266.davem@davemloft.net>
Date:	Mon, 06 Feb 2012 11:45:05 -0500 (EST)
From:	David Miller <davem@...emloft.net>
To:	netdev@...deepdalvi.com
Cc:	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 1/5] netdev: ethernet dev_alloc_skb to
 netdev_alloc_skb

From: "Pradeep A. Dalvi" <netdev@...deepdalvi.com>
Date: Sun,  5 Feb 2012 18:19:09 +0530

> From: Pradeep A Dalvi <netdev@...deepdalvi.com>
> 
> Replaced deprecating dev_alloc_skb with netdev_alloc_skb in drivers/net/ethernet
>   - Removed extra skb->dev = dev after netdev_alloc_skb
> 
> Signed-off-by: Pradeep A Dalvi <netdev@...deepdalvi.com>

Applied, but I had to fix several things up:

> -			if (pkt_len < rx_copybreak && (skb = dev_alloc_skb(pkt_len + 2)) != NULL) {
> +			if (pkt_len < rx_copybreak &&
> +					(skb = netdev_alloc_skb(dev, pkt_len + 2)) != NULL) {

This is not the correct way to format a multi-line conditional.

All subsequent lines must start at the first column after the initial line's
openning parenthesis:

	if (format_it &&
	    like_this)

	if (not &&
		like_this)

To be honest I have no idea what posses people to tab things out in such
an incredibly ugly fashion in the first place.

> -		if (!(lp->rx_skbuff[i] = dev_alloc_skb(lp->rx_buff_len))) {
> +		lp->rx_skbuff[i] = netdev_alloc_skb(dev, lp->rx_buff_len);
> +		if (!lp->rx_skbuff[i]) {

You properly leave this test alone and keep it as "!foo" yet:

> -			if(!(new_skb = dev_alloc_skb(lp->rx_buff_len))){
> +			new_skb = netdev_alloc_skb(dev, lp->rx_buff_len);
> +			if (new_skb == NULL) {

You change this one to the undesirable "== NULL" test, don't do that.
"!foo" is the canonical and most efficient NULL pointer test.

> -      skb = dev_alloc_skb(pkt_len+2);
> +	skb = netdev_alloc_skb(dev, pkt_len + 2);

Do not change the indentation in one place when the entire rest of the source
file uses something else, fixing that would a seperate change from what you're
doing.

> -	    skb = dev_alloc_skb(RX_BUFLEN + 2);
> +		skb = netdev_alloc_skb(RX_BUFLEN + 2);

Same problem.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/