On Tue, 12 Oct 2021 13:41:43 +0200 Rasmus Villemoes <[email protected]> wrote:
> On 12/10/2021 13.04, Marek Behún wrote: > > From: Marek Behún <[email protected]> > > > > Copy the value of the found variable into given buffer with > > strncpy(). > > > > Signed-off-by: Marek Behún <[email protected]> > > --- > > 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
