From: Dean Jenkins <Dean_Jenkins@mentor.com>
To: Valentin Ilie <valentin.ilie@gmail.com>
Cc: marcel@holtmann.org, gustavo@padovan.org,
johan.hedberg@gmail.com, linux-bluetooth@vger.kernel.org
Subject: Re: [PATCH] Bluetooth: clean up
Date: Mon, 12 Aug 2013 10:16:13 +0100 [thread overview]
Message-ID: <5208A7DD.9070600@mentor.com> (raw)
In-Reply-To: <1376225793-22981-1-git-send-email-valentin.ilie@gmail.com>
On 11/08/13 13:56, Valentin Ilie wrote:
> Clean up bluetooth files
> - replace foo* bar with foo *bar
> - add space arround '='
> - remove trailing witespaces
> - remove assignment in if
> - fix coding style issues
I suggest you split the commits into 2:
a) commit with whitespace changes
b) commit with other coding style changes that cause C statements to be
rewritten.
This makes it easier for people to separate whitespace changes from
technical changes to the C code.
Also it would help people back-porting these commits to older kernels.
>
> Signed-off-by: Valentin Ilie <valentin.ilie@gmail.com>
> ---
> drivers/bluetooth/Kconfig | 12 +++++------
> drivers/bluetooth/dtl1_cs.c | 6 +++---
> drivers/bluetooth/hci_bcsp.c | 48 +++++++++++++++++++++--------------------
> drivers/bluetooth/hci_h5.c | 6 ++++--
> drivers/bluetooth/hci_ldisc.c | 15 +++++++------
> drivers/bluetooth/hci_vhci.c | 4 ++--
> 6 files changed, 49 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 11a6104..6421414 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -30,8 +30,8 @@ config BT_HCIUART
> help
> Bluetooth HCI UART driver.
> This driver is required if you want to use Bluetooth devices with
> - serial port interface. You will also need this driver if you have
> - UART based Bluetooth PCMCIA and CF devices like Xircom Credit Card
> + serial port interface. You will also need this driver if you have
> + UART based Bluetooth PCMCIA and CF devices like Xircom Credit Card
> adapter and BrainBoxes Bluetooth PC Card.
>
> Say Y here to compile support for Bluetooth UART devices into the
> @@ -41,9 +41,9 @@ config BT_HCIUART_H4
> bool "UART (H4) protocol support"
> depends on BT_HCIUART
> help
> - UART (H4) is serial protocol for communication between Bluetooth
> - device and host. This protocol is required for most Bluetooth devices
> - with UART interface, including PCMCIA and CF cards.
> + UART (H4) is serial protocol for communication between Bluetooth
> + device and host. This protocol is required for most Bluetooth devices
> + with UART interface, including PCMCIA and CF cards.
>
> Say Y here to compile support for HCI UART (H4) protocol.
>
> @@ -52,7 +52,7 @@ config BT_HCIUART_BCSP
> depends on BT_HCIUART
> select BITREVERSE
> help
> - BCSP (BlueCore Serial Protocol) is serial protocol for communication
> + BCSP (BlueCore Serial Protocol) is serial protocol for communication
> between Bluetooth device and host. This protocol is required for non
> USB Bluetooth devices based on CSR BlueCore chip, including PCMCIA and
> CF cards.
> diff --git a/drivers/bluetooth/dtl1_cs.c b/drivers/bluetooth/dtl1_cs.c
> index 33f3a69..3b89c0d 100644
> --- a/drivers/bluetooth/dtl1_cs.c
> +++ b/drivers/bluetooth/dtl1_cs.c
> @@ -153,7 +153,8 @@ static void dtl1_write_wakeup(dtl1_info_t *info)
> if (!pcmcia_dev_present(info->p_dev))
> return;
>
> - if (!(skb = skb_dequeue(&(info->txq))))
> + skb = skb_dequeue(&(info->txq));
> + if (!skb)
> break;
>
> /* Send frame */
> @@ -181,9 +182,8 @@ static void dtl1_control(dtl1_info_t *info, struct sk_buff *skb)
> int i;
>
> printk(KERN_INFO "Bluetooth: Nokia control data =");
> - for (i = 0; i < skb->len; i++) {
> + for (i = 0; i < skb->len; i++)
> printk(" %02x", skb->data[i]);
> - }
> printk("\n");
>
> /* transition to active state */
> diff --git a/drivers/bluetooth/hci_bcsp.c b/drivers/bluetooth/hci_bcsp.c
> index 57e502e..fbd54e8 100644
> --- a/drivers/bluetooth/hci_bcsp.c
> +++ b/drivers/bluetooth/hci_bcsp.c
> @@ -105,11 +105,11 @@ static const u16 crc_table[] = {
> #define BCSP_CRC_INIT(x) x = 0xffff
>
> /*
> - Update crc with next data byte
> + Update crc with next data byte
>
> - Implementation note
> - The data byte is treated as two nibbles. The crc is generated
> - in reverse, i.e., bits are fed into the register from the top.
> + Implementation note
> + The data byte is treated as two nibbles. The crc is generated
> + in reverse, i.e., bits are fed into the register from the top.
> */
> static void bcsp_crc_update(u16 *crc, u8 d)
> {
> @@ -287,11 +287,12 @@ static struct sk_buff *bcsp_dequeue(struct hci_uart *hu)
> struct bcsp_struct *bcsp = hu->priv;
> unsigned long flags;
> struct sk_buff *skb;
> -
> +
> /* First of all, check for unreliable messages in the queue,
> since they have priority */
>
> - if ((skb = skb_dequeue(&bcsp->unrel)) != NULL) {
> + skb = skb_dequeue(&bcsp->unrel);
> + if (skb != NULL) {
> struct sk_buff *nskb = bcsp_prepare_pkt(bcsp, skb->data, skb->len, bt_cb(skb)->pkt_type);
> if (nskb) {
> kfree_skb(skb);
> @@ -308,7 +309,8 @@ static struct sk_buff *bcsp_dequeue(struct hci_uart *hu)
>
> spin_lock_irqsave_nested(&bcsp->unack.lock, flags, SINGLE_DEPTH_NESTING);
>
> - if (bcsp->unack.qlen < BCSP_TXWINSIZE && (skb = skb_dequeue(&bcsp->rel)) != NULL) {
> + skb = skb_dequeue(&bcsp->rel);
> + if (bcsp->unack.qlen < BCSP_TXWINSIZE && skb != NULL) {
THIS CHANGE IS INCORRECT.
The call to skb_dequeue() depends on bcsp->unack.qlen < BCSP_TXWINSIZE
being true. In other words the 2nd condition of && is not executed when
the 1st condition is false. Therefore 2 nested if statements are needed.
> struct sk_buff *nskb = bcsp_prepare_pkt(bcsp, skb->data, skb->len, bt_cb(skb)->pkt_type);
> if (nskb) {
> __skb_queue_tail(&bcsp->unack, skb);
> @@ -433,7 +435,7 @@ static inline void bcsp_unslip_one_byte(struct bcsp_struct *bcsp, unsigned char
> break;
> default:
> memcpy(skb_put(bcsp->rx_skb, 1), &byte, 1);
> - if ((bcsp->rx_skb-> data[0] & 0x40) != 0 &&
> + if ((bcsp->rx_skb->data[0] & 0x40) != 0 &&
> bcsp->rx_state != BCSP_W4_CRC)
> bcsp_crc_update(&bcsp->message_crc, byte);
> bcsp->rx_count--;
> @@ -444,24 +446,24 @@ static inline void bcsp_unslip_one_byte(struct bcsp_struct *bcsp, unsigned char
> switch (byte) {
> case 0xdc:
> memcpy(skb_put(bcsp->rx_skb, 1), &c0, 1);
> - if ((bcsp->rx_skb-> data[0] & 0x40) != 0 &&
> + if ((bcsp->rx_skb->data[0] & 0x40) != 0 &&
> bcsp->rx_state != BCSP_W4_CRC)
> - bcsp_crc_update(&bcsp-> message_crc, 0xc0);
> + bcsp_crc_update(&bcsp->message_crc, 0xc0);
> bcsp->rx_esc_state = BCSP_ESCSTATE_NOESC;
> bcsp->rx_count--;
> break;
>
> case 0xdd:
> memcpy(skb_put(bcsp->rx_skb, 1), &db, 1);
> - if ((bcsp->rx_skb-> data[0] & 0x40) != 0 &&
> - bcsp->rx_state != BCSP_W4_CRC)
> - bcsp_crc_update(&bcsp-> message_crc, 0xdb);
> + if ((bcsp->rx_skb->data[0] & 0x40) != 0 &&
> + bcsp->rx_state != BCSP_W4_CRC)
> + bcsp_crc_update(&bcsp->message_crc, 0xdb);
> bcsp->rx_esc_state = BCSP_ESCSTATE_NOESC;
> bcsp->rx_count--;
> break;
>
> default:
> - BT_ERR ("Invalid byte %02x after esc byte", byte);
> + BT_ERR("Invalid byte %02x after esc byte", byte);
> kfree_skb(bcsp->rx_skb);
> bcsp->rx_skb = NULL;
> bcsp->rx_state = BCSP_W4_PKT_DELIMITER;
> @@ -524,9 +526,9 @@ static void bcsp_complete_rx_pkt(struct hci_uart *hu)
>
> hci_recv_frame(bcsp->rx_skb);
> } else {
> - BT_ERR ("Packet for unknown channel (%u %s)",
> + BT_ERR("Packet for unknown channel (%u %s)",
> bcsp->rx_skb->data[1] & 0x0f,
> - bcsp->rx_skb->data[0] & 0x80 ?
> + bcsp->rx_skb->data[0] & 0x80 ?
> "reliable" : "unreliable");
> kfree_skb(bcsp->rx_skb);
> }
> @@ -554,7 +556,7 @@ static int bcsp_recv(struct hci_uart *hu, void *data, int count)
> struct bcsp_struct *bcsp = hu->priv;
> unsigned char *ptr;
>
> - BT_DBG("hu %p count %d rx_state %d rx_count %ld",
> + BT_DBG("hu %p count %d rx_state %d rx_count %ld",
> hu, count, bcsp->rx_state, bcsp->rx_count);
>
> ptr = data;
> @@ -574,7 +576,7 @@ static int bcsp_recv(struct hci_uart *hu, void *data, int count)
>
> switch (bcsp->rx_state) {
> case BCSP_W4_BCSP_HDR:
> - if ((0xff & (u8) ~ (bcsp->rx_skb->data[0] + bcsp->rx_skb->data[1] +
> + if ((0xff & (u8) ~(bcsp->rx_skb->data[0] + bcsp->rx_skb->data[1] +
> bcsp->rx_skb->data[2])) != bcsp->rx_skb->data[3]) {
> BT_ERR("Error in BCSP hdr checksum");
> kfree_skb(bcsp->rx_skb);
> @@ -583,8 +585,8 @@ static int bcsp_recv(struct hci_uart *hu, void *data, int count)
> continue;
> }
> if (bcsp->rx_skb->data[0] & 0x80 /* reliable pkt */
> - && (bcsp->rx_skb->data[0] & 0x07) != bcsp->rxseq_txack) {
> - BT_ERR ("Out-of-order packet arrived, got %u expected %u",
> + && (bcsp->rx_skb->data[0] & 0x07) != bcsp->rxseq_txack) {
> + BT_ERR("Out-of-order packet arrived, got %u expected %u",
> bcsp->rx_skb->data[0] & 0x07, bcsp->rxseq_txack);
>
> kfree_skb(bcsp->rx_skb);
> @@ -593,7 +595,7 @@ static int bcsp_recv(struct hci_uart *hu, void *data, int count)
> continue;
> }
> bcsp->rx_state = BCSP_W4_DATA;
> - bcsp->rx_count = (bcsp->rx_skb->data[1] >> 4) +
> + bcsp->rx_count = (bcsp->rx_skb->data[1] >> 4) +
> (bcsp->rx_skb->data[2] << 4); /* May be 0 */
> continue;
>
> @@ -607,7 +609,7 @@ static int bcsp_recv(struct hci_uart *hu, void *data, int count)
>
> case BCSP_W4_CRC:
> if (bitrev16(bcsp->message_crc) != bscp_get_crc(bcsp)) {
> - BT_ERR ("Checksum failed: computed %04x received %04x",
> + BT_ERR("Checksum failed: computed %04x received %04x",
> bitrev16(bcsp->message_crc),
> bscp_get_crc(bcsp));
>
> @@ -645,7 +647,7 @@ static int bcsp_recv(struct hci_uart *hu, void *data, int count)
> BCSP_CRC_INIT(bcsp->message_crc);
>
> /* Do not increment ptr or decrement count
> - * Allocate packet. Max len of a BCSP pkt=
> + * Allocate packet. Max len of a BCSP pkt=
> * 0xFFF (payload) +4 (header) +2 (crc) */
>
> bcsp->rx_skb = bt_skb_alloc(0x1005, GFP_ATOMIC);
> diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
> index b6154d5..67a0e6c 100644
> --- a/drivers/bluetooth/hci_h5.c
> +++ b/drivers/bluetooth/hci_h5.c
> @@ -673,7 +673,8 @@ static struct sk_buff *h5_dequeue(struct hci_uart *hu)
> return h5_prepare_pkt(hu, HCI_3WIRE_LINK_PKT, wakeup_req, 2);
> }
>
> - if ((skb = skb_dequeue(&h5->unrel)) != NULL) {
> + skb = skb_dequeue(&h5->unrel);
> + if (skb != NULL) {
> nskb = h5_prepare_pkt(hu, bt_cb(skb)->pkt_type,
> skb->data, skb->len);
> if (nskb) {
> @@ -690,7 +691,8 @@ static struct sk_buff *h5_dequeue(struct hci_uart *hu)
> if (h5->unack.qlen >= h5->tx_win)
> goto unlock;
>
> - if ((skb = skb_dequeue(&h5->rel)) != NULL) {
> + skb = skb_dequeue(&h5->rel);
> + if (skb != NULL) {
> nskb = h5_prepare_pkt(hu, bt_cb(skb)->pkt_type,
> skb->data, skb->len);
> if (nskb) {
> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> index bc68a44..d424e7d 100644
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -236,7 +236,7 @@ static int hci_uart_close(struct hci_dev *hdev)
> /* Send frames from HCI layer */
> static int hci_uart_send_frame(struct sk_buff *skb)
> {
> - struct hci_dev* hdev = (struct hci_dev *) skb->dev;
> + struct hci_dev *hdev = (struct hci_dev *) skb->dev;
> struct hci_uart *hu;
>
> if (!hdev) {
> @@ -279,7 +279,8 @@ static int hci_uart_tty_open(struct tty_struct *tty)
> if (tty->ops->write == NULL)
> return -EOPNOTSUPP;
>
> - if (!(hu = kzalloc(sizeof(struct hci_uart), GFP_KERNEL))) {
> + hu = kzalloc(sizeof(struct hci_uart), GFP_KERNEL);
> + if (!hu) {
> BT_ERR("Can't allocate control structure");
> return -ENFILE;
> }
> @@ -483,7 +484,7 @@ static int hci_uart_set_proto(struct hci_uart *hu, int id)
> *
> * Return Value: Command dependent
> */
> -static int hci_uart_tty_ioctl(struct tty_struct *tty, struct file * file,
> +static int hci_uart_tty_ioctl(struct tty_struct *tty, struct file *file,
> unsigned int cmd, unsigned long arg)
> {
> struct hci_uart *hu = (void *)tty->disc_data;
> @@ -564,7 +565,7 @@ static int __init hci_uart_init(void)
>
> /* Register the tty discipline */
>
> - memset(&hci_uart_ldisc, 0, sizeof (hci_uart_ldisc));
> + memset(&hci_uart_ldisc, 0, sizeof(hci_uart_ldisc));
> hci_uart_ldisc.magic = TTY_LDISC_MAGIC;
> hci_uart_ldisc.name = "n_hci";
> hci_uart_ldisc.open = hci_uart_tty_open;
> @@ -577,7 +578,8 @@ static int __init hci_uart_init(void)
> hci_uart_ldisc.write_wakeup = hci_uart_tty_wakeup;
> hci_uart_ldisc.owner = THIS_MODULE;
>
> - if ((err = tty_register_ldisc(N_HCI, &hci_uart_ldisc))) {
> + err = tty_register_ldisc(N_HCI, &hci_uart_ldisc);
> + if (err) {
> BT_ERR("HCI line discipline registration failed. (%d)", err);
> return err;
> }
> @@ -622,7 +624,8 @@ static void __exit hci_uart_exit(void)
> #endif
>
> /* Release tty registration of line discipline */
> - if ((err = tty_unregister_ldisc(N_HCI)))
> + err = tty_unregister_ldisc(N_HCI);
> + if (err)
> BT_ERR("Can't unregister HCI line discipline (%d)", err);
> }
>
> diff --git a/drivers/bluetooth/hci_vhci.c b/drivers/bluetooth/hci_vhci.c
> index d8b7aed..4b499f2 100644
> --- a/drivers/bluetooth/hci_vhci.c
> +++ b/drivers/bluetooth/hci_vhci.c
> @@ -82,7 +82,7 @@ static int vhci_flush(struct hci_dev *hdev)
>
> static int vhci_send_frame(struct sk_buff *skb)
> {
> - struct hci_dev* hdev = (struct hci_dev *) skb->dev;
> + struct hci_dev *hdev = (struct hci_dev *) skb->dev;
> struct vhci_data *data;
>
> if (!hdev) {
> @@ -281,7 +281,7 @@ static const struct file_operations vhci_fops = {
> .llseek = no_llseek,
> };
>
> -static struct miscdevice vhci_miscdev= {
> +static struct miscdevice vhci_miscdev = {
> .name = "vhci",
> .fops = &vhci_fops,
> .minor = MISC_DYNAMIC_MINOR,
Dean Jenkins
Mentor Graphics
next prev parent reply other threads:[~2013-08-12 9:16 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-11 12:56 [PATCH] Bluetooth: clean up Valentin Ilie
2013-08-12 9:16 ` Dean Jenkins [this message]
2013-08-12 15:46 ` [PATCH v2] Bluetooth: Remove assignment in if Valentin Ilie
2013-08-19 8:41 ` Valentin Ilie
2013-08-20 9:25 ` Gustavo Padovan
2014-03-05 18:58 ` Johan Hedberg
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5208A7DD.9070600@mentor.com \
--to=dean_jenkins@mentor.com \
--cc=gustavo@padovan.org \
--cc=johan.hedberg@gmail.com \
--cc=linux-bluetooth@vger.kernel.org \
--cc=marcel@holtmann.org \
--cc=valentin.ilie@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).