Hello Christian, On Sat, Feb 6, 2021 at 2:33 AM Christian Mauderer <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>> 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. 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. 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? 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 > > http://lists.rtems.org/mailman/listinfo/devel > > >
_______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
