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?
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.
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.
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
