On Sat, Feb 6, 2021 at 1:59 AM Christian Mauderer <o...@c-mauderer.de> wrote:
> On 06/02/2021 06:02, Niteesh G. S. wrote: > > Hello Christian, > > > > On Sat, Feb 6, 2021 at 2:33 AM Christian Mauderer <o...@c-mauderer.de > > <mailto:o...@c-mauderer.de>> wrote: > > > > On 05/02/2021 19:22, Gedare Bloom wrote: > > > > > > > > > On Fri, Feb 5, 2021 at 10:41 AM G S Niteesh Babu > > <niteesh...@gmail.com <mailto:niteesh...@gmail.com> > > > <mailto:niteesh...@gmail.com <mailto:niteesh...@gmail.com>>> > wrote: > > > > > > Changed rtems_ofw_get_prop to use memcpy instead of strncpy > > > --- > > > bsps/shared/ofw/ofw.c | 10 +++++++++- > > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > > > diff --git a/bsps/shared/ofw/ofw.c b/bsps/shared/ofw/ofw.c > > > index fa94bfbf05..9dec310247 100644 > > > --- a/bsps/shared/ofw/ofw.c > > > +++ b/bsps/shared/ofw/ofw.c > > > @@ -198,7 +198,15 @@ ssize_t rtems_ofw_get_prop( > > > > > > if (prop == NULL && strcmp(propname, "name") == 0) { > > > prop = fdt_get_name(fdtp, offset, &len); > > > - strncpy(buf, prop, bufsize); > > > + > > > + bufsize = MIN(len, bufsize - 1); > > > > > > ok, reserving 1 byte for the \0. It could be worth adding a > > comment here > > > to that effect > > > > Is the content of that property really _allways_ a string? Isn't it > > possible to read some references or similar with it? > > > > Yes in this case it is always a string since we check if the propname == > > "name" > > In other cases we use bcopy. I wanted to get this to your attention but > > then I > > confused myself with endianness and thought it will still be an issue. > > Sorry, I think that confusion has been caused by me. If the spec tells > that "name" is always a string and can never be something else, keeping > str*cpy is OK from my point of view. What do you think Gedare? > > Yes > > > > And as per the device tree spec, the node name is 1-31 chars length, > > consists > > only of ASCII chars(Device tree spec table 2.1) and is null-terminated. > > In that case enforcing null-termination might still be a good idea in > case the buffer passed to this function is smaller or in case someone > passes an invalid device tree. > > +1 And add a comment like you said about the spec. > > > > If it is always a string, I might have made a useless suggestion. In > > that case it might is more efficient and readable to just keep the > > strncpy. Depending on the use case, maybe using strlcpy instead of > > strncpy could be a good idea to guarantee the \0 termination. > > > > As per docs, strlcpy is not present in glibc and is not standardized > > by POSIX, > > so is it okay to use that? > > As far as I know, newlib (the C library we use) has a strlcpy. It is > already used in some rtems code (for example > cpukit/score/src/threadname.c) so I don't see a problem using it. > > We assume it exists inside of rtems. > > If not I can use strncpy with a check like this > > > > strncpy(buf, str, buflen - 1); > > > > if (buflen > 0) > > buf[buflen - 1]= '\0'; > > This ensures the buffer is always null terminated. > > > > > > > > + memcpy(buf, prop, bufsize); > > > + > > > + /* Null terminate the buffer */ > > > + *((char *)buf + bufsize) = 0; > > > > > > that gets written here. looks fine to me. > > > > > > + > > > + /* Return the length of the name including the null byte > */ > > > + /* This is the behaviour in libBSD ofw_fdt_getprop */ > > > return len + 1; > > > > > > shouldn't it be bufsize+1? if it got truncated by the MIN? > > > > > > } > > > > > > -- > > > 2.17.1 > > > > > > > > > _______________________________________________ > > > devel mailing list > > > devel@rtems.org <mailto:devel@rtems.org> > > > http://lists.rtems.org/mailman/listinfo/devel > > <http://lists.rtems.org/mailman/listinfo/devel> > > > > > >
_______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel