The buf is used to hold the list of hwrng devices registered.
The old code ensures we don't walk off the end of buf as we
fill it, but it's unnecessarily complicated and thus difficult
to maintain. Simplify it by using strlcat.
We also ensure the string within buf is NULL terminated
so the final strlen is ok.
Signed-off-by: Rickard Strandqvist <[email protected]>
---
drivers/char/hw_random/core.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index aa30a25..25599ea 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -281,7 +281,7 @@ static ssize_t hwrng_attr_available_show(struct device *dev,
char *buf)
{
int err;
- ssize_t ret = 0;
+ ssize_t ret;
struct hwrng *rng;
err = mutex_lock_interruptible(&rng_mutex);
@@ -289,13 +289,10 @@ static ssize_t hwrng_attr_available_show(struct device *dev,
return -ERESTARTSYS;
buf[0] = '\0';
list_for_each_entry(rng, &rng_list, list) {
- strncat(buf, rng->name, PAGE_SIZE - ret - 1);
- ret += strlen(rng->name);
- strncat(buf, " ", PAGE_SIZE - ret - 1);
- ret++;
+ strlcat(buf, rng->name, PAGE_SIZE);
+ strlcat(buf, " ", PAGE_SIZE);
}
- strncat(buf, "\n", PAGE_SIZE - ret - 1);
- ret++;
+ ret = strlcat(buf, "\n", PAGE_SIZE);
mutex_unlock(&rng_mutex);
return ret;
--
1.7.10.4
On Thu, Oct 16, 2014 at 08:15:55PM +0200, Rickard Strandqvist wrote:
> The buf is used to hold the list of hwrng devices registered.
> The old code ensures we don't walk off the end of buf as we
> fill it, but it's unnecessarily complicated and thus difficult
> to maintain. Simplify it by using strlcat.
> We also ensure the string within buf is NULL terminated
> so the final strlen is ok.
This text is no longer necessary.
>
> Signed-off-by: Rickard Strandqvist <[email protected]>
Please add my:
Reviewed-by: Jason Cooper <[email protected]>
to the next version of this patch. It makes the maintainers job a *lot*
easier not having to sift through old threads collecting up all the
tags.
thx,
Jason.
> ---
> drivers/char/hw_random/core.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index aa30a25..25599ea 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -281,7 +281,7 @@ static ssize_t hwrng_attr_available_show(struct device *dev,
> char *buf)
> {
> int err;
> - ssize_t ret = 0;
> + ssize_t ret;
> struct hwrng *rng;
>
> err = mutex_lock_interruptible(&rng_mutex);
> @@ -289,13 +289,10 @@ static ssize_t hwrng_attr_available_show(struct device *dev,
> return -ERESTARTSYS;
> buf[0] = '\0';
> list_for_each_entry(rng, &rng_list, list) {
> - strncat(buf, rng->name, PAGE_SIZE - ret - 1);
> - ret += strlen(rng->name);
> - strncat(buf, " ", PAGE_SIZE - ret - 1);
> - ret++;
> + strlcat(buf, rng->name, PAGE_SIZE);
> + strlcat(buf, " ", PAGE_SIZE);
> }
> - strncat(buf, "\n", PAGE_SIZE - ret - 1);
> - ret++;
> + ret = strlcat(buf, "\n", PAGE_SIZE);
> mutex_unlock(&rng_mutex);
>
> return ret;
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
2014-10-16 20:25 GMT+02:00 Jason Cooper <[email protected]>:
> On Thu, Oct 16, 2014 at 08:15:55PM +0200, Rickard Strandqvist wrote:
>> The buf is used to hold the list of hwrng devices registered.
>> The old code ensures we don't walk off the end of buf as we
>> fill it, but it's unnecessarily complicated and thus difficult
>> to maintain. Simplify it by using strlcat.
>
>> We also ensure the string within buf is NULL terminated
>> so the final strlen is ok.
>
> This text is no longer necessary.
>
>>
>> Signed-off-by: Rickard Strandqvist <[email protected]>
>
> Please add my:
>
> Reviewed-by: Jason Cooper <[email protected]>
>
> to the next version of this patch. It makes the maintainers job a *lot*
> easier not having to sift through old threads collecting up all the
> tags.
>
> thx,
>
> Jason.
>
>> ---
>> drivers/char/hw_random/core.c | 11 ++++-------
>> 1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
>> index aa30a25..25599ea 100644
>> --- a/drivers/char/hw_random/core.c
>> +++ b/drivers/char/hw_random/core.c
>> @@ -281,7 +281,7 @@ static ssize_t hwrng_attr_available_show(struct device *dev,
>> char *buf)
>> {
>> int err;
>> - ssize_t ret = 0;
>> + ssize_t ret;
>> struct hwrng *rng;
>>
>> err = mutex_lock_interruptible(&rng_mutex);
>> @@ -289,13 +289,10 @@ static ssize_t hwrng_attr_available_show(struct device *dev,
>> return -ERESTARTSYS;
>> buf[0] = '\0';
>> list_for_each_entry(rng, &rng_list, list) {
>> - strncat(buf, rng->name, PAGE_SIZE - ret - 1);
>> - ret += strlen(rng->name);
>> - strncat(buf, " ", PAGE_SIZE - ret - 1);
>> - ret++;
>> + strlcat(buf, rng->name, PAGE_SIZE);
>> + strlcat(buf, " ", PAGE_SIZE);
>> }
>> - strncat(buf, "\n", PAGE_SIZE - ret - 1);
>> - ret++;
>> + ret = strlcat(buf, "\n", PAGE_SIZE);
>> mutex_unlock(&rng_mutex);
>>
>> return ret;
>> --
>> 1.7.10.4
>>
Hello again
I was not sure how I would do with others Reviewed-by:
But then I know it, did not seem obvious to do, because I actually
change some code in between.
And I also think about my change, which is actually wrong with using
the return value from strlcat the same reason as I mentioned before,
it can return strlen + 1
So sorry Joe, I do not think we even can do that :-(
So back to my previous patch then, also I update the text again.
Kind regards
Rickard Strandqvist