[PATCH 08/10] env: Use strncpy() instead of ad-hoc code to copy variable value

[Rasmus Villemoes]

On 12/10/2021 13.04, Marek Behún wrote:
> From: Marek Behún <marek.behun at nic.cz>
> 
> Copy the value of the found variable into given buffer with strncpy().
> 
> Signed-off-by: Marek Behún <marek.behun at nic.cz>
> ---
>  cmd/nvedit.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/cmd/nvedit.c b/cmd/nvedit.c
> index 412918a000..51b9e4ffa4 100644
> --- a/cmd/nvedit.c
> +++ b/cmd/nvedit.c
> @@ -746,18 +746,13 @@ int env_get_f(const char *name, char *buf, unsigned len)
>  			continue;
>  
>  		/* found; copy out */
> -		for (n = 0; n < len; ++n, ++buf) {
> -			*buf = env[val++];
> -			if (*buf == '\0')
> -				return n;
> +		n = strncpy(buf, &env[val], len) - buf;

strncpy by definition returns the dst argument, so this is, apart from
the side effects done by strncpy, a complicated way of saying "n = 0;".

> +		if (len && n == len) {

which then makes this automatically false always.

I understand why you want to avoid an open-coded copying, but strncpy is
almost certainly the wrong tool for the job. Apart from not revealing
anything about the actual length of the source string, it also has the
well-known defect of zeroing the tail of the buffer in the (usual) case
where the source is shorter than the buffer.

n = strlcpy(buf, &env[val], len);
if (n >= len) ...

is probably closer to what you want. But only if you can trust &env[val]
to actually have a nul byte before going into lala land.

Rasmus