* [PATCH] staging: rtl8723bs: fix brace coding style issues
@ 2018-06-21 18:21 Michael Straube
2018-06-22 10:28 ` Dan Carpenter
0 siblings, 1 reply; 7+ messages in thread
From: Michael Straube @ 2018-06-21 18:21 UTC (permalink / raw)
To: gregkh; +Cc: devel, linux-kernel, Michael Straube
Remove braces from single line if statements.
Also fix a comparsion to NULL in one of the conditions.
Issues found by checkpatch.
Signed-off-by: Michael Straube <michael.straube@posteo.de>
---
drivers/staging/rtl8723bs/core/rtw_debug.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/rtl8723bs/core/rtw_debug.c b/drivers/staging/rtl8723bs/core/rtw_debug.c
index f852fde47350..2244ed72ab9c 100644
--- a/drivers/staging/rtl8723bs/core/rtw_debug.c
+++ b/drivers/staging/rtl8723bs/core/rtw_debug.c
@@ -618,9 +618,8 @@ ssize_t proc_set_wait_hiq_empty(struct file *file, const char __user *buffer, si
if (count < 1)
return -EFAULT;
- if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp))) {
+ if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp)))
sscanf(tmp, "%u", &g_wait_hiq_empty);
- }
return count;
}
@@ -1425,9 +1424,8 @@ int proc_get_btcoex_info(struct seq_file *m, void *v)
padapter = (struct adapter *)rtw_netdev_priv(dev);
pbuf = rtw_zmalloc(bufsize);
- if (NULL == pbuf) {
+ if (!pbuf)
return -ENOMEM;
- }
rtw_btcoex_DisplayBtCoexInfo(padapter, pbuf, bufsize);
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] staging: rtl8723bs: fix brace coding style issues
2018-06-21 18:21 [PATCH] staging: rtl8723bs: fix brace coding style issues Michael Straube
@ 2018-06-22 10:28 ` Dan Carpenter
2018-06-22 13:27 ` Michael Straube
2018-06-25 9:47 ` Andy Shevchenko
0 siblings, 2 replies; 7+ messages in thread
From: Dan Carpenter @ 2018-06-22 10:28 UTC (permalink / raw)
To: Michael Straube; +Cc: gregkh, devel, linux-kernel
On Thu, Jun 21, 2018 at 08:21:55PM +0200, Michael Straube wrote:
> Remove braces from single line if statements.
> Also fix a comparsion to NULL in one of the conditions.
> Issues found by checkpatch.
>
> Signed-off-by: Michael Straube <michael.straube@posteo.de>
> ---
> drivers/staging/rtl8723bs/core/rtw_debug.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_debug.c b/drivers/staging/rtl8723bs/core/rtw_debug.c
> index f852fde47350..2244ed72ab9c 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_debug.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_debug.c
> @@ -618,9 +618,8 @@ ssize_t proc_set_wait_hiq_empty(struct file *file, const char __user *buffer, si
> if (count < 1)
> return -EFAULT;
>
> - if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp))) {
> + if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp)))
> sscanf(tmp, "%u", &g_wait_hiq_empty);
> - }
The original code is kind of bad. The NULL check isn't required.
The sscanf call should have error checking. The error code is wrong if
the copy from user fails. The tmp buffer isn't NUL terminated.
if (copy_from_user(tmp, buffer, sizeof(tmp)))
return -EFAULT;
tmp[sizeof(tmp) - 1] = '\0';
if (sscanf(tmp, "%u", &g_wait_hiq_empty) != 1)
return -EINVAL;
return count;
regards,
dan carpenter
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] staging: rtl8723bs: fix brace coding style issues
2018-06-22 10:28 ` Dan Carpenter
@ 2018-06-22 13:27 ` Michael Straube
2018-06-23 8:40 ` Dan Carpenter
2018-06-25 9:47 ` Andy Shevchenko
1 sibling, 1 reply; 7+ messages in thread
From: Michael Straube @ 2018-06-22 13:27 UTC (permalink / raw)
To: Dan Carpenter; +Cc: gregkh, devel, linux-kernel
On 06/22/18 12:28, Dan Carpenter wrote:
>> if (count < 1)
>> return -EFAULT;
>>
>> - if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp))) {
>> + if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp)))
>> sscanf(tmp, "%u", &g_wait_hiq_empty);
>> - }
>
>
> The original code is kind of bad. The NULL check isn't required.
Just for clarification, NULL check refers to checking if buffer != NULL in the
if condition?
if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp)))
~~~~~~
> The sscanf call should have error checking. The error code is wrong if
> the copy from user fails. The tmp buffer isn't NUL terminated.
>
> if (copy_from_user(tmp, buffer, sizeof(tmp)))
> return -EFAULT;
> tmp[sizeof(tmp) - 1] = '\0';
>
> if (sscanf(tmp, "%u", &g_wait_hiq_empty) != 1)
> return -EINVAL;
>
> return count;
>
> regards,
> dan carpenter
>
Regards,
Michael
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] staging: rtl8723bs: fix brace coding style issues
2018-06-22 13:27 ` Michael Straube
@ 2018-06-23 8:40 ` Dan Carpenter
0 siblings, 0 replies; 7+ messages in thread
From: Dan Carpenter @ 2018-06-23 8:40 UTC (permalink / raw)
To: Michael Straube; +Cc: gregkh, devel, linux-kernel
On Fri, Jun 22, 2018 at 03:27:46PM +0200, Michael Straube wrote:
> On 06/22/18 12:28, Dan Carpenter wrote:
> > > if (count < 1)
> > > return -EFAULT;
> > > - if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp))) {
> > > + if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp)))
> > > sscanf(tmp, "%u", &g_wait_hiq_empty);
> > > - }
> >
> >
> > The original code is kind of bad. The NULL check isn't required.
>
> Just for clarification, NULL check refers to checking if buffer != NULL in the
> if condition?
>
> if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp)))
> ~~~~~~
Yes. If buffer is NULL that's just another invalid pointer which the
user cannot access, it's not special, and the copy will fail.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: rtl8723bs: fix brace coding style issues
2018-06-22 10:28 ` Dan Carpenter
2018-06-22 13:27 ` Michael Straube
@ 2018-06-25 9:47 ` Andy Shevchenko
2018-06-25 12:24 ` Dan Carpenter
1 sibling, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2018-06-25 9:47 UTC (permalink / raw)
To: Dan Carpenter
Cc: Michael Straube, Greg Kroah-Hartman, devel, Linux Kernel Mailing List
On Fri, Jun 22, 2018 at 1:28 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> On Thu, Jun 21, 2018 at 08:21:55PM +0200, Michael Straube wrote:
>> Remove braces from single line if statements.
>> Also fix a comparsion to NULL in one of the conditions.
>> Issues found by checkpatch.
>>
>> Signed-off-by: Michael Straube <michael.straube@posteo.de>
>> ---
>> drivers/staging/rtl8723bs/core/rtw_debug.c | 6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8723bs/core/rtw_debug.c b/drivers/staging/rtl8723bs/core/rtw_debug.c
>> index f852fde47350..2244ed72ab9c 100644
>> --- a/drivers/staging/rtl8723bs/core/rtw_debug.c
>> +++ b/drivers/staging/rtl8723bs/core/rtw_debug.c
>> @@ -618,9 +618,8 @@ ssize_t proc_set_wait_hiq_empty(struct file *file, const char __user *buffer, si
>> if (count < 1)
>> return -EFAULT;
>>
>> - if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp))) {
>> + if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp)))
>> sscanf(tmp, "%u", &g_wait_hiq_empty);
>> - }
>
>
> The original code is kind of bad. The NULL check isn't required.
> The sscanf call should have error checking. The error code is wrong if
> the copy from user fails. The tmp buffer isn't NUL terminated.
>
> if (copy_from_user(tmp, buffer, sizeof(tmp)))
> return -EFAULT;
> tmp[sizeof(tmp) - 1] = '\0';
>
> if (sscanf(tmp, "%u", &g_wait_hiq_empty) != 1)
> return -EINVAL;
>
> return count;
Shouldn't this be
kstrtouint_from_user()
instead of all those lines?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] staging: rtl8723bs: fix brace coding style issues
2018-06-25 9:47 ` Andy Shevchenko
@ 2018-06-25 12:24 ` Dan Carpenter
2018-06-26 19:09 ` Andy Shevchenko
0 siblings, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2018-06-25 12:24 UTC (permalink / raw)
To: Andy Shevchenko
Cc: devel, Greg Kroah-Hartman, Michael Straube, Linux Kernel Mailing List
On Mon, Jun 25, 2018 at 12:47:44PM +0300, Andy Shevchenko wrote:
> On Fri, Jun 22, 2018 at 1:28 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > On Thu, Jun 21, 2018 at 08:21:55PM +0200, Michael Straube wrote:
> >> Remove braces from single line if statements.
> >> Also fix a comparsion to NULL in one of the conditions.
> >> Issues found by checkpatch.
> >>
> >> Signed-off-by: Michael Straube <michael.straube@posteo.de>
> >> ---
> >> drivers/staging/rtl8723bs/core/rtw_debug.c | 6 ++----
> >> 1 file changed, 2 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/staging/rtl8723bs/core/rtw_debug.c b/drivers/staging/rtl8723bs/core/rtw_debug.c
> >> index f852fde47350..2244ed72ab9c 100644
> >> --- a/drivers/staging/rtl8723bs/core/rtw_debug.c
> >> +++ b/drivers/staging/rtl8723bs/core/rtw_debug.c
> >> @@ -618,9 +618,8 @@ ssize_t proc_set_wait_hiq_empty(struct file *file, const char __user *buffer, si
> >> if (count < 1)
> >> return -EFAULT;
> >>
> >> - if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp))) {
> >> + if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp)))
> >> sscanf(tmp, "%u", &g_wait_hiq_empty);
> >> - }
> >
> >
> > The original code is kind of bad. The NULL check isn't required.
> > The sscanf call should have error checking. The error code is wrong if
> > the copy from user fails. The tmp buffer isn't NUL terminated.
> >
> > if (copy_from_user(tmp, buffer, sizeof(tmp)))
> > return -EFAULT;
> > tmp[sizeof(tmp) - 1] = '\0';
> >
> > if (sscanf(tmp, "%u", &g_wait_hiq_empty) != 1)
> > return -EINVAL;
> >
> > return count;
>
> Shouldn't this be
>
> kstrtouint_from_user()
>
> instead of all those lines?
I wasn't sure kstrtoint() was the exact same as sscanf()... If so then
sure.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] staging: rtl8723bs: fix brace coding style issues
2018-06-25 12:24 ` Dan Carpenter
@ 2018-06-26 19:09 ` Andy Shevchenko
0 siblings, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2018-06-26 19:09 UTC (permalink / raw)
To: Dan Carpenter
Cc: devel, Greg Kroah-Hartman, Michael Straube, Linux Kernel Mailing List
On Mon, Jun 25, 2018 at 3:24 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> On Mon, Jun 25, 2018 at 12:47:44PM +0300, Andy Shevchenko wrote:
>> On Fri, Jun 22, 2018 at 1:28 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>> > On Thu, Jun 21, 2018 at 08:21:55PM +0200, Michael Straube wrote:
>> >> Remove braces from single line if statements.
>> >> Also fix a comparsion to NULL in one of the conditions.
>> >> Issues found by checkpatch.
>> >> --- a/drivers/staging/rtl8723bs/core/rtw_debug.c
>> >> +++ b/drivers/staging/rtl8723bs/core/rtw_debug.c
>> >> @@ -618,9 +618,8 @@ ssize_t proc_set_wait_hiq_empty(struct file *file, const char __user *buffer, si
>> >> if (count < 1)
>> >> return -EFAULT;
>> >>
>> >> - if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp))) {
>> >> + if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp)))
>> >> sscanf(tmp, "%u", &g_wait_hiq_empty);
>> >> - }
>> >
>> >
>> > The original code is kind of bad. The NULL check isn't required.
>> > The sscanf call should have error checking. The error code is wrong if
>> > the copy from user fails. The tmp buffer isn't NUL terminated.
>> >
>> > if (copy_from_user(tmp, buffer, sizeof(tmp)))
>> > return -EFAULT;
>> > tmp[sizeof(tmp) - 1] = '\0';
>> >
>> > if (sscanf(tmp, "%u", &g_wait_hiq_empty) != 1)
>> > return -EINVAL;
>> >
>> > return count;
>>
>> Shouldn't this be
>>
>> kstrtouint_from_user()
>>
>> instead of all those lines?
>
> I wasn't sure kstrtoint() was the exact same as sscanf()... If so then
> sure.
In this case it's very close to what sscanf() is doing here including
all user buffer handing.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-06-26 19:09 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-21 18:21 [PATCH] staging: rtl8723bs: fix brace coding style issues Michael Straube
2018-06-22 10:28 ` Dan Carpenter
2018-06-22 13:27 ` Michael Straube
2018-06-23 8:40 ` Dan Carpenter
2018-06-25 9:47 ` Andy Shevchenko
2018-06-25 12:24 ` Dan Carpenter
2018-06-26 19:09 ` Andy Shevchenko
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).