On Tue, Sep 12, 2017 at 2:40 AM, Liam Ryan <[email protected]> wrote:
> Fix checkpath-reported unbalanced braces in the following areas
>
> 221: FILE: drivers/staging/rtl8712/hal_init.c:221:
> 392: FILE: drivers/staging/rtl8712/os_intfs.c:392:
> 363: FILE: drivers/staging/rtl8712/rtl8712_cmd.c:363:
> 889: FILE: drivers/staging/rtl8712/rtl8712_recv.c:889:
> 902: FILE: drivers/staging/rtl8712/rtl871x_cmd.c:902:
> 84: FILE: drivers/staging/rtl8712/rtl871x_ioctl_set.c:84:
> 580: FILE: drivers/staging/rtl8712/rtl871x_mlme.c:580:
> 593: FILE: drivers/staging/rtl8712/usb_intf.c:593:
>
> Signed-off-by: Liam Ryan <[email protected]>
> ---
> This is my first patch and I have several doubts about style choices
>
> At line 216 of hal_init.c should opening brace follow comment instead?
>
> At line 577 of rtl871x_mlme.c should I bring logic to one line instead of
> opening the brace on the continued line?
>
> At line 353 of rtl8712_cmd.c the if/else is technically only 1 line each.
> Should the braces still have been added per checkpath for readability?
>
> drivers/staging/rtl8712/hal_init.c | 4 ++--
> drivers/staging/rtl8712/os_intfs.c | 4 ++--
> drivers/staging/rtl8712/rtl8712_cmd.c | 4 ++--
> drivers/staging/rtl8712/rtl8712_recv.c | 4 ++--
> drivers/staging/rtl8712/rtl871x_cmd.c | 3 ++-
> drivers/staging/rtl8712/rtl871x_ioctl_set.c | 4 ++--
> drivers/staging/rtl8712/rtl871x_mlme.c | 4 ++--
> drivers/staging/rtl8712/usb_intf.c | 3 ++-
> 8 files changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/staging/rtl8712/hal_init.c
> b/drivers/staging/rtl8712/hal_init.c
> index c83d7eb..de832b0b5 100644
> --- a/drivers/staging/rtl8712/hal_init.c
> +++ b/drivers/staging/rtl8712/hal_init.c
> @@ -216,9 +216,9 @@ static u8 rtl8712_dl_fw(struct _adapter *padapter)
> emem_sz = fwhdr.img_SRAM_size;
> do {
> memset(ptx_desc, 0, TXDESC_SIZE);
> - if (emem_sz > MAX_DUMP_FWSZ) /* max=48k */
> + if (emem_sz > MAX_DUMP_FWSZ) { /* max=48k */
I would not have the comment there in the first place. It doesn't add
any new information and just messes up the style.
> dump_emem_sz = MAX_DUMP_FWSZ;
> - else {
> + } else {
> dump_emem_sz = emem_sz;
> ptx_desc->txdw0 |= cpu_to_le32(BIT(28));
> }
> diff --git a/drivers/staging/rtl8712/os_intfs.c
> b/drivers/staging/rtl8712/os_intfs.c
> index e698f6e..990a343 100644
> --- a/drivers/staging/rtl8712/os_intfs.c
> +++ b/drivers/staging/rtl8712/os_intfs.c
> @@ -385,11 +385,11 @@ static int netdev_open(struct net_device *pnetdev)
> padapter->bup = true;
> if (rtl871x_hal_init(padapter) != _SUCCESS)
> goto netdev_open_error;
> - if (!r8712_initmac)
> + if (!r8712_initmac) {
> /* Use the mac address stored in the Efuse */
> memcpy(pnetdev->dev_addr,
> padapter->eeprompriv.mac_addr, ETH_ALEN);
> - else {
> + } else {
> /* We have to inform f/w to use user-supplied MAC
> * address.
> */
> diff --git a/drivers/staging/rtl8712/rtl8712_cmd.c
> b/drivers/staging/rtl8712/rtl8712_cmd.c
> index 0104ace..3c88994 100644
> --- a/drivers/staging/rtl8712/rtl8712_cmd.c
> +++ b/drivers/staging/rtl8712/rtl8712_cmd.c
> @@ -356,11 +356,11 @@ int r8712_cmd_thread(void *context)
> if ((wr_sz % 64) == 0)
> blnPending = 1;
> }
> - if (blnPending) /* 32 bytes for TX Desc - 8 offset */
> + if (blnPending) { /* 32 bytes for TX Desc - 8 offset
> */
> pdesc->txdw0 |= cpu_to_le32(((TXDESC_SIZE +
> OFFSET_SZ + 8) << OFFSET_SHT)
> &
> 0x00ff0000);
> - else {
> + } else {
> pdesc->txdw0 |= cpu_to_le32(((TXDESC_SIZE +
> OFFSET_SZ) <<
> OFFSET_SHT) &
I would add the braces. I think once you have multiple lines, even
though it's just one statement, braces are in order, so as to avoid
any confusion.
> diff --git a/drivers/staging/rtl8712/rtl8712_recv.c
> b/drivers/staging/rtl8712/rtl8712_recv.c
> index ea3eb94..0978727 100644
> --- a/drivers/staging/rtl8712/rtl8712_recv.c
> +++ b/drivers/staging/rtl8712/rtl8712_recv.c
> @@ -883,10 +883,10 @@ static void query_rx_phy_status(struct _adapter
> *padapter,
> * from 0~100. It is assigned to the BSS List in
> * GetValueFromBeaconOrProbeRsp().
> */
> - if (bcck_rate)
> + if (bcck_rate) {
> prframe->u.hdr.attrib.signal_strength =
> (u8)r8712_signal_scale_mapping(pwdb_all);
> - else {
> + } else {
> if (rf_rx_num != 0)
> prframe->u.hdr.attrib.signal_strength =
> (u8)(r8712_signal_scale_mapping(total_rssi /=
> diff --git a/drivers/staging/rtl8712/rtl871x_cmd.c
> b/drivers/staging/rtl8712/rtl871x_cmd.c
> index 04638f1..a424f447 100644
> --- a/drivers/staging/rtl8712/rtl871x_cmd.c
> +++ b/drivers/staging/rtl8712/rtl871x_cmd.c
> @@ -899,9 +899,10 @@ void r8712_createbss_cmd_callback(struct _adapter
> *padapter,
> if (!pwlan)
> goto createbss_cmd_fail;
> pwlan->last_scanned = jiffies;
> - } else
> + } else {
> list_add_tail(&(pwlan->list),
> &pmlmepriv->scanned_queue.queue);
> + }
> pnetwork->Length = r8712_get_wlan_bssid_ex_sz(pnetwork);
> memcpy(&(pwlan->network), pnetwork, pnetwork->Length);
> pwlan->fixed = true;
> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_set.c
> b/drivers/staging/rtl8712/rtl871x_ioctl_set.c
> index 01a1504..8a5ced4 100644
> --- a/drivers/staging/rtl8712/rtl871x_ioctl_set.c
> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_set.c
> @@ -78,10 +78,10 @@ static u8 do_join(struct _adapter *padapter)
> int ret;
>
> ret = r8712_select_and_join_from_scan(pmlmepriv);
> - if (ret == _SUCCESS)
> + if (ret == _SUCCESS) {
> mod_timer(&pmlmepriv->assoc_timer,
> jiffies +
> msecs_to_jiffies(MAX_JOIN_TIMEOUT));
> - else {
> + } else {
> if (check_fwstate(pmlmepriv, WIFI_ADHOC_STATE)) {
> /* submit r8712_createbss_cmd to change to an
> * ADHOC_MASTER pmlmepriv->lock has been
> diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c
> b/drivers/staging/rtl8712/rtl871x_mlme.c
> index bf1ac22..111c809 100644
> --- a/drivers/staging/rtl8712/rtl871x_mlme.c
> +++ b/drivers/staging/rtl8712/rtl871x_mlme.c
> @@ -574,10 +574,10 @@ void r8712_surveydone_event_callback(struct _adapter
> *adapter, u8 *pbuf)
> set_fwstate(pmlmepriv, _FW_UNDER_LINKING);
>
> if (r8712_select_and_join_from_scan(pmlmepriv)
> - == _SUCCESS)
> + == _SUCCESS) {
I think this merits another patch to get rid of the rather generic
SUCCESS, and translate it into a normal kernel return value, so that
we can just say !r8712_select_and_join_from_scan(). Although I think
the function name is also a possible target for a rename. I'd leave it
like it is in this patch.
> mod_timer(&pmlmepriv->assoc_timer,
> jiffies +
>
> msecs_to_jiffies(MAX_JOIN_TIMEOUT));
> - else {
> + } else {
> struct wlan_bssid_ex *pdev_network =
>
> &(adapter->registrypriv.dev_network);
> u8 *pibss =
> diff --git a/drivers/staging/rtl8712/usb_intf.c
> b/drivers/staging/rtl8712/usb_intf.c
> index b3e266b..85eaddd 100644
> --- a/drivers/staging/rtl8712/usb_intf.c
> +++ b/drivers/staging/rtl8712/usb_intf.c
> @@ -590,9 +590,10 @@ static int r871xu_drv_init(struct usb_interface
> *pusb_intf,
> mac[0] &= 0xFE;
> dev_info(&udev->dev,
> "r8712u: MAC Address from user = %pM\n", mac);
> - } else
> + } else {
> dev_info(&udev->dev,
> "r8712u: MAC Address from efuse = %pM\n",
> mac);
> + }
> ether_addr_copy(pnetdev->dev_addr, mac);
> }
> /* step 6. Load the firmware asynchronously */
> --
> 2.7.4
>
> _______________________________________________
> devel mailing list
> [email protected]
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
