Message-ID: <1413480303.6702.7.camel@perches.com>
Subject: Re: [PATCH 3v] char: hw_random: core.c:  Changed from using strncat
 to strlcat
From: Joe Perches <joe@perches.com>
To: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
Cc: Matt Mackall <mpm@selenic.com>, Herbert Xu <herbert@gondor.apana.org.au>,
        Torsten Duwe <duwe@lst.de>, "Theodore Ts'o" <tytso@mit.edu>,
        Jason Cooper <jason@lakedaemon.net>, Amit Shah <amit.shah@redhat.com>,
        Stephen Boyd <sboyd@codeaurora.org>,
        Paul Gortmaker <paul.gortmaker@windriver.com>,
        Kees Cook <keescook@chromium.org>,
        Dan Carpenter <dan.carpenter@oracle.com>, linux-kernel@vger.kernel.org
Date: Thu, 16 Oct 2014 10:25:03 -0700
In-Reply-To: <1413479722-29247-1-git-send-email-rickard_strandqvist@spectrumdigital.se>
References: <1413479722-29247-1-git-send-email-rickard_strandqvist@spectrumdigital.se>
Content-Type: text/plain; charset="ISO-8859-1"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org

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.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/