Re: [PATCH] staging: rtl8723bs: Add missing blank lines
Em qua, 22 de mai de 2019 06:41, Dan Carpenter
escreveu:
>
> On Tue, May 21, 2019 at 09:46:55PM -0300, Fabio Lima wrote:
> > This patch resolves the following warning from checkpatch.pl
> > WARNING: Missing a blank line after declarations
> >
> > Signed-off-by: Fabio Lima
> > ---
> > drivers/staging/rtl8723bs/core/rtw_debug.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/staging/rtl8723bs/core/rtw_debug.c
> > b/drivers/staging/rtl8723bs/core/rtw_debug.c
> > index 9f8446ccf..853362381 100644
> > --- a/drivers/staging/rtl8723bs/core/rtw_debug.c
> > +++ b/drivers/staging/rtl8723bs/core/rtw_debug.c
> > @@ -382,6 +382,7 @@ ssize_t proc_set_roam_tgt_addr(struct file *file, const
> > char __user *buffer, siz
> > if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp))) {
> >
> > int num = sscanf(tmp, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx", addr,
> > addr+1, addr+2, addr+3, addr+4, addr+5);
> > +
> > if (num == 6)
> > memcpy(adapter->mlmepriv.roam_tgt_addr, addr,
> > ETH_ALEN);
> >
>
> I'm sorry but this function is really such nonsense. Can you send a
> patch to re-write it instead?
>
> drivers/staging/rtl8723bs/core/rtw_debug.c
>371 ssize_t proc_set_roam_tgt_addr(struct file *file, const char __user
> *buffer, size_t count, loff_t *pos, void *data)
>372 {
>373 struct net_device *dev = data;
>374 struct adapter *adapter = (struct adapter
> *)rtw_netdev_priv(dev);
>375
>376 char tmp[32];
>377 u8 addr[ETH_ALEN];
>378
>379 if (count < 1)
>
> This check is silly. I guess the safest thing is to change it to:
> if (count < sizeof(tmp))
>
>380 return -EFAULT;
>
> It should be return -EINVAL;
>
>381
>382 if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp))) {
>
> Remove the check for if the user passes a NULL buffer, because that's
> already handled in copy_from_user(). Return -EFAULT if copy_from_user()
> fails.
>
> if (copy_from_user(tmp, buffer, sizeof(tmp)))
> return -EFAULT;
>
>
>383
>
> Extra blank line.
>
>384 int num = sscanf(tmp,
> "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx", addr, addr+1, addr+2, addr+3, addr+4,
> addr+5);
>
> You will need to move the num declaration to the start of the function.
>
>385 if (num == 6)
>386 memcpy(adapter->mlmepriv.roam_tgt_addr, addr,
> ETH_ALEN);
>
> If num != 6 then return -EINVAL;
>
>387
>388 DBG_871X("set roam_tgt_addr to "MAC_FMT"\n",
> MAC_ARG(adapter->mlmepriv.roam_tgt_addr));
>389 }
>390
>391 return count;
>392 }
>
> regards,
> dan carpenter
Thanks for your feedback.
This is my first patch and I will send the second patch with
modifications that you suggest.
Fabio Lima
Re: [PATCH] staging: rtl8723bs: Add missing blank lines
On Tue, May 21, 2019 at 09:46:55PM -0300, Fabio Lima wrote:
> This patch resolves the following warning from checkpatch.pl
> WARNING: Missing a blank line after declarations
>
> Signed-off-by: Fabio Lima
> ---
> drivers/staging/rtl8723bs/core/rtw_debug.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_debug.c
> b/drivers/staging/rtl8723bs/core/rtw_debug.c
> index 9f8446ccf..853362381 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_debug.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_debug.c
> @@ -382,6 +382,7 @@ ssize_t proc_set_roam_tgt_addr(struct file *file, const
> char __user *buffer, siz
> if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp))) {
>
> int num = sscanf(tmp, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx", addr,
> addr+1, addr+2, addr+3, addr+4, addr+5);
> +
> if (num == 6)
> memcpy(adapter->mlmepriv.roam_tgt_addr, addr, ETH_ALEN);
>
I'm sorry but this function is really such nonsense. Can you send a
patch to re-write it instead?
drivers/staging/rtl8723bs/core/rtw_debug.c
371 ssize_t proc_set_roam_tgt_addr(struct file *file, const char __user
*buffer, size_t count, loff_t *pos, void *data)
372 {
373 struct net_device *dev = data;
374 struct adapter *adapter = (struct adapter
*)rtw_netdev_priv(dev);
375
376 char tmp[32];
377 u8 addr[ETH_ALEN];
378
379 if (count < 1)
This check is silly. I guess the safest thing is to change it to:
if (count < sizeof(tmp))
380 return -EFAULT;
It should be return -EINVAL;
381
382 if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp))) {
Remove the check for if the user passes a NULL buffer, because that's
already handled in copy_from_user(). Return -EFAULT if copy_from_user()
fails.
if (copy_from_user(tmp, buffer, sizeof(tmp)))
return -EFAULT;
383
Extra blank line.
384 int num = sscanf(tmp, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
addr, addr+1, addr+2, addr+3, addr+4, addr+5);
You will need to move the num declaration to the start of the function.
385 if (num == 6)
386 memcpy(adapter->mlmepriv.roam_tgt_addr, addr,
ETH_ALEN);
If num != 6 then return -EINVAL;
387
388 DBG_871X("set roam_tgt_addr to "MAC_FMT"\n",
MAC_ARG(adapter->mlmepriv.roam_tgt_addr));
389 }
390
391 return count;
392 }
regards,
dan carpenter
Re: [PATCH] staging: rtl8723bs: Add missing blank lines
On 2019-05-21, at 21:46:55 -0300, Fabio Lima wrote:
> This patch resolves the following warning from checkpatch.pl
> WARNING: Missing a blank line after declarations
>
> Signed-off-by: Fabio Lima
> ---
> drivers/staging/rtl8723bs/core/rtw_debug.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_debug.c
> b/drivers/staging/rtl8723bs/core/rtw_debug.c
> index 9f8446ccf..853362381 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_debug.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_debug.c
> @@ -382,6 +382,7 @@ ssize_t proc_set_roam_tgt_addr(struct file *file, const
> char __user *buffer, siz
> if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp))) {
>
> int num = sscanf(tmp, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx", addr,
> addr+1, addr+2, addr+3, addr+4, addr+5);
> +
> if (num == 6)
> memcpy(adapter->mlmepriv.roam_tgt_addr, addr, ETH_ALEN);
>
> @@ -1348,6 +1349,7 @@ int proc_get_btcoex_dbg(struct seq_file *m, void *v)
> struct net_device *dev = m->private;
> struct adapter *padapter;
> char buf[512] = {0};
> +
> padapter = (struct adapter *)rtw_netdev_priv(dev);
>
> rtw_btcoex_GetDBG(padapter, buf, 512);
> --
> 2.11.0
Looks good to me.
J.
signature.asc
Description: PGP signature
[PATCH] staging: rtl8723bs: Add missing blank lines
This patch resolves the following warning from checkpatch.pl
WARNING: Missing a blank line after declarations
Signed-off-by: Fabio Lima
---
drivers/staging/rtl8723bs/core/rtw_debug.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/rtl8723bs/core/rtw_debug.c
b/drivers/staging/rtl8723bs/core/rtw_debug.c
index 9f8446ccf..853362381 100644
--- a/drivers/staging/rtl8723bs/core/rtw_debug.c
+++ b/drivers/staging/rtl8723bs/core/rtw_debug.c
@@ -382,6 +382,7 @@ ssize_t proc_set_roam_tgt_addr(struct file *file, const
char __user *buffer, siz
if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp))) {
int num = sscanf(tmp, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx", addr,
addr+1, addr+2, addr+3, addr+4, addr+5);
+
if (num == 6)
memcpy(adapter->mlmepriv.roam_tgt_addr, addr, ETH_ALEN);
@@ -1348,6 +1349,7 @@ int proc_get_btcoex_dbg(struct seq_file *m, void *v)
struct net_device *dev = m->private;
struct adapter *padapter;
char buf[512] = {0};
+
padapter = (struct adapter *)rtw_netdev_priv(dev);
rtw_btcoex_GetDBG(padapter, buf, 512);
--
2.11.0
