lkml.org 
[lkml]   [2012]   [Jan]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [PATCH] Bluetooth: bfusb: Fixed some coding style issues
    From
    Date
    On Mon, 2012-01-16 at 01:17 +0530, mahendra singh meena wrote:
    > Fixed some coding style issues shown by scripts/checkpatch.pl
    []
    > diff --git a/drivers/bluetooth/bfusb.c b/drivers/bluetooth/bfusb.c
    []
    > @@ -127,11 +127,13 @@ static int bfusb_send_bulk(struct bfusb_data *data, struct sk_buff *skb)
    > {
    > struct bfusb_data_scb *scb = (void *) skb->cb;
    > struct urb *urb = bfusb_get_completed(data);
    > + struct urb *urb_chk = urb;
    > int err, pipe;
    >
    > BT_DBG("bfusb %p skb %p len %d", data, skb, skb->len);
    > + urb = usb_alloc_urb(0, GFP_ATOMIC);
    >
    > - if (!urb && !(urb = usb_alloc_urb(0, GFP_ATOMIC)))
    > + if (!urb_chk && !urb)

    Broken patch.

    You are now always calling usb_alloc_urb and
    overwriting the original urb that is returned
    from bfusb_get_completed().

    Not everything that checkpatch flags needs to be
    modified.

    The original is better than even a correct
    proposed patch because it's more readable and it
    uses fewer variables.

    If you _really_ want to avoid checkpatch warnings,
    then don't use the additional automatic.

    if (!urb) {
    urb = usb_alloc_urb(0, GFP_ATOMIC);
    if (!urb)
    return -ENOMEM;
    }

    > @@ -145,7 +147,7 @@ static int bfusb_send_bulk(struct bfusb_data *data, struct sk_buff *skb)
    >
    > err = usb_submit_urb(urb, GFP_ATOMIC);
    > if (err) {
    > - BT_ERR("%s bulk tx submit failed urb %p err %d",
    > + BT_ERR("%s bulk tx submit failed urb %p err %d",

    ? What does checkpatch complain about this?

    > data->hdev->name, urb, err);
    > skb_unlink(skb, &data->pending_q);
    > usb_free_urb(urb);
    > @@ -214,11 +216,13 @@ static int bfusb_rx_submit(struct bfusb_data *data, struct urb *urb)
    > {
    > struct bfusb_data_scb *scb;
    > struct sk_buff *skb;
    > + struct urb *urb_chk = urb;
    > int err, pipe, size = HCI_MAX_FRAME_SIZE + 32;
    >
    > BT_DBG("bfusb %p urb %p", data, urb);
    > + urb = usb_alloc_urb(0, GFP_ATOMIC);
    >
    > - if (!urb && !(urb = usb_alloc_urb(0, GFP_ATOMIC)))
    > + if (!urb && !urb_chk)
    > return -ENOMEM;

    Same broken change.

    > @@ -283,30 +288,37 @@ static inline int bfusb_recv_block(struct bfusb_data *data, int hdr, unsigned ch
    > switch (pkt_type) {
    > case HCI_EVENT_PKT:
    > if (len >= HCI_EVENT_HDR_SIZE) {
    > - struct hci_event_hdr *hdr = (struct hci_event_hdr *) buf;
    > + struct hci_event_hdr *hdr;
    > + hdr = (struct hci_event_hdr *) buf;

    It'd probably be better to change the type of "buf" to void *
    and avoid all the casts.

    > @@ -514,12 +528,14 @@ static int bfusb_send_frame(struct sk_buff *skb)
    > while (count) {
    > size = min_t(uint, count, BFUSB_MAX_BLOCK_SIZE);
    >
    > - buf[0] = 0xc1 | ((sent == 0) ? 0x04 : 0) | ((count == size) ? 0x08 : 0);
    > + buf[0] = 0xc1 | ((sent == 0) ? 0x04 : 0) |
    > + ((count == size) ? 0x08 : 0);

    When wrapping just for 80 columns, please try to use
    alignments that are a bit more readable and perhaps
    remove unnecessary parentheses like:

    buf[0] = 0xc1 |
    (sent == 0 ? 0x04 : 0) |
    (count == size ? 0x08 : 0);




    \
     
     \ /
      Last update: 2012-01-15 21:35    [W:0.026 / U:0.656 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site