Messages in this thread | | | Subject | Re: [PATCH 3v] char: hw_random: core.c: Changed from using strncat to strlcat | From | Joe Perches <> | Date | Thu, 16 Oct 2014 10:53:47 -0700 |
| |
On Thu, 2014-10-16 at 19:41 +0200, Rickard Strandqvist wrote: > 2014-10-16 19:25 GMT+02:00 Joe Perches <joe@perches.com>: > > On Thu, 2014-10-16 at 19:15 +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. > > [] > >> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c > > [] > >> @@ -289,16 +288,13 @@ 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++; > >> + strlcat(buf, "\n", PAGE_SIZE); > >> mutex_unlock(&rng_mutex); > >> > >> - return ret; > >> + return strlen(buf); > >> } > >> > >> static DEVICE_ATTR(rng_current, S_IRUGO | S_IWUSR, > > > > Rickard, can you please use some optimizations here > > (and elsewhere) so that strlcat doesn't always have > > to strlen the first string and the return doesn't > > have to do the strlen too? > > > > You could use a temporary for the returned length > > of the strlcat so that if it's shorter than > > the buffer, the next strlcat can start at the > > appropriate known position instead of having > > to do the initial strlen again and again. [] > But the others, I am very hesitant about. Because strlcat like > snprintf and strlcpy returns the length that would be copied rather > than what is actually copied. Hence such a code to be even more > complex than before.
None of the conversions you've done seem to use the return value so maybe there should be yet another function that doesn't return an overrun value.
| |