
On Tue, 12 Oct 2021 13:41:43 +0200 Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
On 12/10/2021 13.04, Marek Behún wrote:
From: Marek Behún marek.behun@nic.cz
Copy the value of the found variable into given buffer with strncpy().
Signed-off-by: Marek Behún marek.behun@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;".
You are right, this is a mistake.
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.
U-Boot's strncpy does not do that: * Note that unlike userspace strncpy, this does not %NUL-pad the buffer. * However, the result is not %NUL-terminated if the source exceeds * @count bytes.
But you are right that I should use strlcpy here.
Thanks.
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