lkml.org 
[lkml]   [2012]   [Feb]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v5 1/5] netdev: ethernet dev_alloc_skb to netdev_alloc_skb
    From
    On Mon, Feb 6, 2012 at 10:15 PM, David Miller <davem@davemloft.net> wrote:
    > From: "Pradeep A. Dalvi" <netdev@pradeepdalvi.com>
    > Date: Sun,  5 Feb 2012 18:19:09 +0530
    >
    >> From: Pradeep A Dalvi <netdev@pradeepdalvi.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@pradeepdalvi.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.

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

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

    \
     
     \ /
      Last update: 2012-02-06 18:39    [W:0.025 / U:0.096 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site