Re: [PATCH v5 1/5] netdev: ethernet dev_alloc_skb to netdev_alloc_skb

On Mon, Feb 6, 2012 at 10:15 PM, David Miller [off-list ref] wrote:

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

quoted

From: Pradeep A Dalvi <redacted>

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

Applied, but I had to fix several things up:

quoted

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

quoted

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

quoted

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

quoted

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

quoted

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

Same problem.

Thanks a lot! I will make note of these points and shall not repeat again.

Regards,
Pradeep A. Dalvi