[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <bc244970-498d-c6e7-7ed0-3c3d77777ccc@posteo.de>
Date: Fri, 22 Jun 2018 15:27:46 +0200
From: Michael Straube <michael.straube@...teo.de>
To: Dan Carpenter <dan.carpenter@...cle.com>
Cc: gregkh@...uxfoundation.org, devel@...verdev.osuosl.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: rtl8723bs: fix brace coding style issues
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
Powered by blists - more mailing lists