* [PATCH] staging: rtl8723bs: Move closing braces of 'if' statements to end of line
@ 2018-02-17 17:19 Dafna Hirschfeld
2018-02-17 17:27 ` [Outreachy kernel] " Julia Lawall
0 siblings, 1 reply; 4+ messages in thread
From: Dafna Hirschfeld @ 2018-02-17 17:19 UTC (permalink / raw)
To: gregkh; +Cc: outreachy-kernel
Move all closing braces to the end of the line.
Remove braces in single 'if' statement.
Break 'if' condition to avoid 80 characters line.
Issues found with checkpatch.pl
Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
drivers/staging/rtl8723bs/os_dep/xmit_linux.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/rtl8723bs/os_dep/xmit_linux.c b/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
index f29e110..4861c80 100644
--- a/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
+++ b/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
@@ -64,9 +64,7 @@ int rtw_os_xmit_resource_alloc(struct adapter *padapter, struct xmit_buf *pxmitb
if (alloc_sz > 0) {
pxmitbuf->pallocated_buf = rtw_zmalloc(alloc_sz);
if (pxmitbuf->pallocated_buf == NULL)
- {
return _FAIL;
- }
pxmitbuf->pbuf = (u8 *)N_BYTE_ALIGMENT((SIZE_PTR)(pxmitbuf->pallocated_buf), XMITBUF_ALIGN_SZ);
}
@@ -90,8 +88,8 @@ void rtw_os_pkt_complete(struct adapter *padapter, _pkt *pkt)
queue = skb_get_queue_mapping(pkt);
if (padapter->registrypriv.wifi_spec) {
if (__netif_subqueue_stopped(padapter->pnetdev, queue) &&
- (pxmitpriv->hwxmits[queue].accnt < WMM_XMIT_THRESHOLD))
- {
+ (pxmitpriv->hwxmits[queue].accnt <
+ WMM_XMIT_THRESHOLD)) {
netif_wake_subqueue(padapter->pnetdev, queue);
}
} else {
@@ -177,8 +175,7 @@ static int rtw_mlcst2unicst(struct adapter *padapter, struct sk_buff *skb)
for (i = 0; i < chk_alive_num; i++) {
psta = rtw_get_stainfo_by_offset(pstapriv, chk_alive_list[i]);
- if (!(psta->state & _FW_LINKED))
- {
+ if (!(psta->state & _FW_LINKED)) {
DBG_COUNTER(padapter->tx_logs.os_tx_m2u_ignore_fw_linked);
continue;
}
@@ -187,8 +184,7 @@ static int rtw_mlcst2unicst(struct adapter *padapter, struct sk_buff *skb)
if (!memcmp(psta->hwaddr, &skb->data[6], 6)
|| !memcmp(psta->hwaddr, null_addr, 6)
|| !memcmp(psta->hwaddr, bc_addr, 6)
- )
- {
+ ) {
DBG_COUNTER(padapter->tx_logs.os_tx_m2u_ignore_self);
continue;
}
@@ -249,8 +245,7 @@ int _rtw_xmit_entry(_pkt *pkt, _nic_hdl pnetdev)
#endif
)
&& (padapter->registrypriv.wifi_spec == 0)
- )
- {
+ ) {
if (pxmitpriv->free_xmitframe_cnt > (NR_XMITFRAME/4)) {
res = rtw_mlcst2unicst(padapter, pkt);
if (res == true) {
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: rtl8723bs: Move closing braces of 'if' statements to end of line
2018-02-17 17:19 [PATCH] staging: rtl8723bs: Move closing braces of 'if' statements to end of line Dafna Hirschfeld
@ 2018-02-17 17:27 ` Julia Lawall
2018-02-17 19:40 ` Dafna Hirschfeld
0 siblings, 1 reply; 4+ messages in thread
From: Julia Lawall @ 2018-02-17 17:27 UTC (permalink / raw)
To: Dafna Hirschfeld; +Cc: gregkh, outreachy-kernel
On Sat, 17 Feb 2018, Dafna Hirschfeld wrote:
> Move all closing braces to the end of the line.
> Remove braces in single 'if' statement.
It's not really a single if statement, but rather an if with a single
statement in the then branch (or in both branches, as appropriate).
> Break 'if' condition to avoid 80 characters line.
> Issues found with checkpatch.pl
This goes overboard on shortening the commit message line length. Maybe
you can configure your editor to do the appropriate line break, or at
least show the column number.
>
> Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
> ---
> drivers/staging/rtl8723bs/os_dep/xmit_linux.c | 15 +++++----------
> 1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/os_dep/xmit_linux.c b/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
> index f29e110..4861c80 100644
> --- a/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
> +++ b/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
> @@ -64,9 +64,7 @@ int rtw_os_xmit_resource_alloc(struct adapter *padapter, struct xmit_buf *pxmitb
> if (alloc_sz > 0) {
> pxmitbuf->pallocated_buf = rtw_zmalloc(alloc_sz);
> if (pxmitbuf->pallocated_buf == NULL)
> - {
> return _FAIL;
> - }
OK
>
> pxmitbuf->pbuf = (u8 *)N_BYTE_ALIGMENT((SIZE_PTR)(pxmitbuf->pallocated_buf), XMITBUF_ALIGN_SZ);
> }
> @@ -90,8 +88,8 @@ void rtw_os_pkt_complete(struct adapter *padapter, _pkt *pkt)
> queue = skb_get_queue_mapping(pkt);
> if (padapter->registrypriv.wifi_spec) {
> if (__netif_subqueue_stopped(padapter->pnetdev, queue) &&
> - (pxmitpriv->hwxmits[queue].accnt < WMM_XMIT_THRESHOLD))
> - {
> + (pxmitpriv->hwxmits[queue].accnt <
> + WMM_XMIT_THRESHOLD)) {
This does not look so nice. What happens if you move the condition to the
left, such that it starts just to the right of the ( on the preceeding
line. If needed, you can also drop the parentheses.
> netif_wake_subqueue(padapter->pnetdev, queue);
> }
> } else {
> @@ -177,8 +175,7 @@ static int rtw_mlcst2unicst(struct adapter *padapter, struct sk_buff *skb)
>
> for (i = 0; i < chk_alive_num; i++) {
> psta = rtw_get_stainfo_by_offset(pstapriv, chk_alive_list[i]);
> - if (!(psta->state & _FW_LINKED))
> - {
> + if (!(psta->state & _FW_LINKED)) {
OK
> DBG_COUNTER(padapter->tx_logs.os_tx_m2u_ignore_fw_linked);
> continue;
> }
> @@ -187,8 +184,7 @@ static int rtw_mlcst2unicst(struct adapter *padapter, struct sk_buff *skb)
> if (!memcmp(psta->hwaddr, &skb->data[6], 6)
> || !memcmp(psta->hwaddr, null_addr, 6)
> || !memcmp(psta->hwaddr, bc_addr, 6)
> - )
> - {
> + ) {
The close parenthesis on the line with the { is not nice. Move the whole
thing back to just after the last memcmp call.
> DBG_COUNTER(padapter->tx_logs.os_tx_m2u_ignore_self);
> continue;
> }
> @@ -249,8 +245,7 @@ int _rtw_xmit_entry(_pkt *pkt, _nic_hdl pnetdev)
> #endif
> )
> && (padapter->registrypriv.wifi_spec == 0)
> - )
> - {
> + ) {
Same here.
julia
> if (pxmitpriv->free_xmitframe_cnt > (NR_XMITFRAME/4)) {
> res = rtw_mlcst2unicst(padapter, pkt);
> if (res == true) {
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20180217171924.GA4155%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: rtl8723bs: Move closing braces of 'if' statements to end of line
2018-02-17 17:27 ` [Outreachy kernel] " Julia Lawall
@ 2018-02-17 19:40 ` Dafna Hirschfeld
2018-02-17 19:43 ` Julia Lawall
0 siblings, 1 reply; 4+ messages in thread
From: Dafna Hirschfeld @ 2018-02-17 19:40 UTC (permalink / raw)
To: Julia Lawall; +Cc: outreachy-kernel, gregkh
On Sat, Feb 17, 2018 at 06:27:34PM +0100, Julia Lawall wrote:
>
>
> On Sat, 17 Feb 2018, Dafna Hirschfeld wrote:
>
> > Move all closing braces to the end of the line.
> > Remove braces in single 'if' statement.
>
> It's not really a single if statement, but rather an if with a single
> statement in the then branch (or in both branches, as appropriate).
>
> > Break 'if' condition to avoid 80 characters line.
> > Issues found with checkpatch.pl
>
> This goes overboard on shortening the commit message line length. Maybe
> you can configure your editor to do the appropriate line break, or at
> least show the column number.
>
> >
> > Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
> > ---
> > drivers/staging/rtl8723bs/os_dep/xmit_linux.c | 15 +++++----------
> > 1 file changed, 5 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8723bs/os_dep/xmit_linux.c b/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
> > index f29e110..4861c80 100644
> > --- a/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
> > +++ b/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
> > @@ -64,9 +64,7 @@ int rtw_os_xmit_resource_alloc(struct adapter *padapter, struct xmit_buf *pxmitb
> > if (alloc_sz > 0) {
> > pxmitbuf->pallocated_buf = rtw_zmalloc(alloc_sz);
> > if (pxmitbuf->pallocated_buf == NULL)
> > - {
> > return _FAIL;
> > - }
>
> OK
>
> >
> > pxmitbuf->pbuf = (u8 *)N_BYTE_ALIGMENT((SIZE_PTR)(pxmitbuf->pallocated_buf), XMITBUF_ALIGN_SZ);
> > }
> > @@ -90,8 +88,8 @@ void rtw_os_pkt_complete(struct adapter *padapter, _pkt *pkt)
> > queue = skb_get_queue_mapping(pkt);
> > if (padapter->registrypriv.wifi_spec) {
> > if (__netif_subqueue_stopped(padapter->pnetdev, queue) &&
> > - (pxmitpriv->hwxmits[queue].accnt < WMM_XMIT_THRESHOLD))
> > - {
> > + (pxmitpriv->hwxmits[queue].accnt <
> > + WMM_XMIT_THRESHOLD)) {
>
> This does not look so nice. What happens if you move the condition to the
> left, such that it starts just to the right of the ( on the preceeding
> line. If needed, you can also drop the parentheses.
>
> > netif_wake_subqueue(padapter->pnetdev, queue);
> > }
> > } else {
> > @@ -177,8 +175,7 @@ static int rtw_mlcst2unicst(struct adapter *padapter, struct sk_buff *skb)
> >
> > for (i = 0; i < chk_alive_num; i++) {
> > psta = rtw_get_stainfo_by_offset(pstapriv, chk_alive_list[i]);
> > - if (!(psta->state & _FW_LINKED))
> > - {
> > + if (!(psta->state & _FW_LINKED)) {
>
> OK
>
> > DBG_COUNTER(padapter->tx_logs.os_tx_m2u_ignore_fw_linked);
> > continue;
> > }
> > @@ -187,8 +184,7 @@ static int rtw_mlcst2unicst(struct adapter *padapter, struct sk_buff *skb)
> > if (!memcmp(psta->hwaddr, &skb->data[6], 6)
> > || !memcmp(psta->hwaddr, null_addr, 6)
> > || !memcmp(psta->hwaddr, bc_addr, 6)
> > - )
> > - {
> > + ) {
>
> The close parenthesis on the line with the { is not nice. Move the whole
> thing back to just after the last memcmp call.
Hi, Solving the issue here as you asked raises a "CHECK: Logical continuations should be on the previous line"
It apperntly relates to the '||' being in the start of each line instead of the end.
Moving the '||' to the end solves all issues. Should I add it to the patch ?
>
> > DBG_COUNTER(padapter->tx_logs.os_tx_m2u_ignore_self);
> > continue;
> > }
> > @@ -249,8 +245,7 @@ int _rtw_xmit_entry(_pkt *pkt, _nic_hdl pnetdev)
> > #endif
> > )
> > && (padapter->registrypriv.wifi_spec == 0)
> > - )
> > - {
> > + ) {
>
> Same here.
See the longer context of this code https://elixir.bootlin.com/linux/v4.16-rc1/source/drivers/staging/rtl8723bs/os_dep/xmit_linux.c#L248
Here is the same thing as in previous case, the '||' are in the start. It's also not possible to move them to the end
of previous line because it is withing an #if/#endif clause.
>
> julia
>
> > if (pxmitpriv->free_xmitframe_cnt > (NR_XMITFRAME/4)) {
> > res = rtw_mlcst2unicst(padapter, pkt);
> > if (res == true) {
> > --
> > 2.7.4
> >
> > --
> > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20180217171924.GA4155%40gmail.com.
> > For more options, visit https://groups.google.com/d/optout.
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: rtl8723bs: Move closing braces of 'if' statements to end of line
2018-02-17 19:40 ` Dafna Hirschfeld
@ 2018-02-17 19:43 ` Julia Lawall
0 siblings, 0 replies; 4+ messages in thread
From: Julia Lawall @ 2018-02-17 19:43 UTC (permalink / raw)
To: Dafna Hirschfeld; +Cc: outreachy-kernel, gregkh
On Sat, 17 Feb 2018, Dafna Hirschfeld wrote:
> On Sat, Feb 17, 2018 at 06:27:34PM +0100, Julia Lawall wrote:
> >
> >
> > On Sat, 17 Feb 2018, Dafna Hirschfeld wrote:
> >
> > > Move all closing braces to the end of the line.
> > > Remove braces in single 'if' statement.
> >
> > It's not really a single if statement, but rather an if with a single
> > statement in the then branch (or in both branches, as appropriate).
> >
> > > Break 'if' condition to avoid 80 characters line.
> > > Issues found with checkpatch.pl
> >
> > This goes overboard on shortening the commit message line length. Maybe
> > you can configure your editor to do the appropriate line break, or at
> > least show the column number.
> >
> > >
> > > Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
> > > ---
> > > drivers/staging/rtl8723bs/os_dep/xmit_linux.c | 15 +++++----------
> > > 1 file changed, 5 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/staging/rtl8723bs/os_dep/xmit_linux.c b/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
> > > index f29e110..4861c80 100644
> > > --- a/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
> > > +++ b/drivers/staging/rtl8723bs/os_dep/xmit_linux.c
> > > @@ -64,9 +64,7 @@ int rtw_os_xmit_resource_alloc(struct adapter *padapter, struct xmit_buf *pxmitb
> > > if (alloc_sz > 0) {
> > > pxmitbuf->pallocated_buf = rtw_zmalloc(alloc_sz);
> > > if (pxmitbuf->pallocated_buf == NULL)
> > > - {
> > > return _FAIL;
> > > - }
> >
> > OK
> >
> > >
> > > pxmitbuf->pbuf = (u8 *)N_BYTE_ALIGMENT((SIZE_PTR)(pxmitbuf->pallocated_buf), XMITBUF_ALIGN_SZ);
> > > }
> > > @@ -90,8 +88,8 @@ void rtw_os_pkt_complete(struct adapter *padapter, _pkt *pkt)
> > > queue = skb_get_queue_mapping(pkt);
> > > if (padapter->registrypriv.wifi_spec) {
> > > if (__netif_subqueue_stopped(padapter->pnetdev, queue) &&
> > > - (pxmitpriv->hwxmits[queue].accnt < WMM_XMIT_THRESHOLD))
> > > - {
> > > + (pxmitpriv->hwxmits[queue].accnt <
> > > + WMM_XMIT_THRESHOLD)) {
> >
> > This does not look so nice. What happens if you move the condition to the
> > left, such that it starts just to the right of the ( on the preceeding
> > line. If needed, you can also drop the parentheses.
> >
> > > netif_wake_subqueue(padapter->pnetdev, queue);
> > > }
> > > } else {
> > > @@ -177,8 +175,7 @@ static int rtw_mlcst2unicst(struct adapter *padapter, struct sk_buff *skb)
> > >
> > > for (i = 0; i < chk_alive_num; i++) {
> > > psta = rtw_get_stainfo_by_offset(pstapriv, chk_alive_list[i]);
> > > - if (!(psta->state & _FW_LINKED))
> > > - {
> > > + if (!(psta->state & _FW_LINKED)) {
> >
> > OK
> >
> > > DBG_COUNTER(padapter->tx_logs.os_tx_m2u_ignore_fw_linked);
> > > continue;
> > > }
> > > @@ -187,8 +184,7 @@ static int rtw_mlcst2unicst(struct adapter *padapter, struct sk_buff *skb)
> > > if (!memcmp(psta->hwaddr, &skb->data[6], 6)
> > > || !memcmp(psta->hwaddr, null_addr, 6)
> > > || !memcmp(psta->hwaddr, bc_addr, 6)
> > > - )
> > > - {
> > > + ) {
> >
> > The close parenthesis on the line with the { is not nice. Move the whole
> > thing back to just after the last memcmp call.
> Hi, Solving the issue here as you asked raises a "CHECK: Logical continuations should be on the previous line"
> It apperntly relates to the '||' being in the start of each line instead of the end.
> Moving the '||' to the end solves all issues. Should I add it to the patch ?
It could be reasonable. The subject of the patch could become "clean up
conditionals", to be a bit more general.
> >
> > > DBG_COUNTER(padapter->tx_logs.os_tx_m2u_ignore_self);
> > > continue;
> > > }
> > > @@ -249,8 +245,7 @@ int _rtw_xmit_entry(_pkt *pkt, _nic_hdl pnetdev)
> > > #endif
> > > )
> > > && (padapter->registrypriv.wifi_spec == 0)
> > > - )
> > > - {
> > > + ) {
> >
> > Same here.
> See the longer context of this code https://elixir.bootlin.com/linux/v4.16-rc1/source/drivers/staging/rtl8723bs/os_dep/xmit_linux.c#L248
> Here is the same thing as in previous case, the '||' are in the start. It's also not possible to move them to the end
> of previous line because it is withing an #if/#endif clause.
If it's impossible, it's impossible. Checkpatch just makes suggestions.
It doesn't always see enough of the context.
julia
> >
> > julia
> >
> > > if (pxmitpriv->free_xmitframe_cnt > (NR_XMITFRAME/4)) {
> > > res = rtw_mlcst2unicst(padapter, pkt);
> > > if (res == true) {
> > > --
> > > 2.7.4
> > >
> > > --
> > > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20180217171924.GA4155%40gmail.com.
> > > For more options, visit https://groups.google.com/d/optout.
> > >
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-02-17 19:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-17 17:19 [PATCH] staging: rtl8723bs: Move closing braces of 'if' statements to end of line Dafna Hirschfeld
2018-02-17 17:27 ` [Outreachy kernel] " Julia Lawall
2018-02-17 19:40 ` Dafna Hirschfeld
2018-02-17 19:43 ` Julia Lawall
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.