<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:small">Hello Christian,</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Feb 6, 2021 at 2:33 AM Christian Mauderer <<a href="mailto:oss@c-mauderer.de">oss@c-mauderer.de</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 05/02/2021 19:22, Gedare Bloom wrote:<br>
> <br>
> <br>
> On Fri, Feb 5, 2021 at 10:41 AM G S Niteesh Babu <<a href="mailto:niteesh.gs@gmail.com" target="_blank">niteesh.gs@gmail.com</a> <br>
> <mailto:<a href="mailto:niteesh.gs@gmail.com" target="_blank">niteesh.gs@gmail.com</a>>> wrote:<br>
> <br>
>     Changed rtems_ofw_get_prop to use memcpy instead of strncpy<br>
>     ---<br>
>       bsps/shared/ofw/ofw.c | 10 +++++++++-<br>
>       1 file changed, 9 insertions(+), 1 deletion(-)<br>
> <br>
>     diff --git a/bsps/shared/ofw/ofw.c b/bsps/shared/ofw/ofw.c<br>
>     index fa94bfbf05..9dec310247 100644<br>
>     --- a/bsps/shared/ofw/ofw.c<br>
>     +++ b/bsps/shared/ofw/ofw.c<br>
>     @@ -198,7 +198,15 @@ ssize_t rtems_ofw_get_prop(<br>
> <br>
>         if (prop == NULL && strcmp(propname, "name") == 0) {<br>
>           prop = fdt_get_name(fdtp, offset, &len);<br>
>     -    strncpy(buf, prop, bufsize);<br>
>     +<br>
>     +    bufsize = MIN(len, bufsize - 1);<br>
> <br>
> ok, reserving 1 byte for the \0. It could be worth adding a comment here <br>
> to that effect<br>
<br>
Is the content of that property really _allways_ a string? Isn't it <br>
possible to read some references or similar with it?<br></blockquote><div> </div><div><span class="gmail_default" style="font-size:small">Yes in this case it is always a string since we check if the propname == "name"</span></div><div><span class="gmail_default" style="font-size:small">In other cases we use bcopy. I wanted to get this to your attention but then I</span></div><div><span class="gmail_default" style="font-size:small">confused myself with endianness and thought it will still be an issue.</span></div><div><span class="gmail_default" style="font-size:small"><br></span></div><div><span class="gmail_default" style="font-size:small">And as per the device tree spec, the node name is 1-31 chars length, consists</span></div><div><span class="gmail_default" style="font-size:small">only of ASCII chars(Device tree spec table 2.1) and is null-terminated.</span></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
If it is always a string, I might have made a useless suggestion. In <br>
that case it might is more efficient and readable to just keep the <br>
strncpy. Depending on the use case, maybe using strlcpy instead of <br>
strncpy could be a good idea to guarantee the \0 termination.<br></blockquote><div> </div><div><div class="gmail_default" style="font-size:small">As per docs, strlcpy  is  not  present  in glibc and is not standardized by POSIX,</div><div class="gmail_default" style="font-size:small">so is it okay to use that?</div><div class="gmail_default" style="font-size:small">If not I can use strncpy with a check like this</div></div></div><blockquote style="margin:0 0 0 40px;border:none;padding:0px"><div class="gmail_quote"><div><div class="gmail_default" style="font-size:small">strncpy(buf, str, buflen - 1);</div></div></div></blockquote><div class="gmail_quote"><div><div class="gmail_default" style="font-size:small">           if (buflen > 0)<br>               buf[buflen - 1]= '\0';<br></div><div class="gmail_default" style="font-size:small">This ensures the buffer is always null terminated.</div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> <br>
>     +    memcpy(buf, prop, bufsize);<br>
>     +<br>
>     +    /* Null terminate the buffer */<br>
>     +    *((char *)buf + bufsize) = 0;<br>
> <br>
> that gets written here. looks fine to me.<br>
> <br>
>     +<br>
>     +    /* Return the length of the name including the null byte */<br>
>     +    /* This is the behaviour in libBSD ofw_fdt_getprop */<br>
>           return len + 1;<br>
> <br>
> shouldn't it be bufsize+1? if it got truncated by the MIN?<br>
> <br>
>         }<br>
> <br>
>     -- <br>
>     2.17.1<br>
> <br>
> <br>
> _______________________________________________<br>
> devel mailing list<br>
> <a href="mailto:devel@rtems.org" target="_blank">devel@rtems.org</a><br>
> <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
> <br>
</blockquote></div></div>