[PATCH] rtems-fdt / shell - Fix string truncation warning

Chris Johns chrisj at rtems.org
Fri Oct 16 23:37:23 UTC 2020


On 17/10/20 3:16 am, Frank Kühndel wrote:
> Hello Chris,
> 
> On 10/15/20 10:29 PM, Chris Johns wrote:
>> On 16/10/20 7:24 am, Joel Sherrill wrote:
>>> On Thu, Oct 15, 2020 at 2:13 PM Chris Johns <chrisj at rtems.org
>>> <mailto:chrisj at rtems.org>> wrote:
>>>     On 15/10/20 10:27 pm, Frank Kuehndel wrote:
>>>     > From: Frank Kühndel <frank.kuehndel at embedded-brains.de
>>>     <mailto:frank.kuehndel at embedded-brains.de>>
>>>     >
>>>     > The compiler warning was:
>>>     >
>>>     > ../../../cpukit/libmisc/rtems-fdt/rtems-fdt.c:267:5: warning:
>>>     > 'strncpy' specified bound depends on the length of the source argument
>>>     >   267 |     strncpy(path, name, namelen);
>>>     >       |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>     >
>>>     > It turns out that the `strncpy()` nor the buffer `path` is needed when
>>>     > one uses `strncmp()` instead of `strcmp()`. This needs some change to
>>>     > the algorithm but has the advantage that `name` is never truncated
>>>     > to the size of the buffer `path`.
>>>     >
>>>     > Note:
>>>     >
>>>     > rtems-fdt.c, rtems-fdt-shell.c and cpukit/include/rtems/rtems-fdt.h
>>>     > seem to be dead code.
>>>
>>>     We cannot tell this is as a user may depend on it. With this command I know
>>>     users on Zynq platforms that use FDT to map PL logic to drivers. The command is
>>>     very much in use and is used as a PS to PL testing method.
> 
> This statement was not meant as an offense. 

Thank you. There was offence taken by the statement and the review process has
resolved its presence. I think we should avoid subjective statement in commit
message, they are fine in tickets and reviews where we can debt them.

> I spent a lot of time trying
> to figure out where and how this code was used - without result. With
> this statement, I hoped to trigger a reaction by someone indicating that
> he/she knows the code can be removed or that he/she knows the code is in
> use. I am sorry that my text was not appropriate.

This is understandable, the commands are hard in this respect. They are a grey
area and this is understood and accepted and maybe we could do better here. I am
happy to see a criteria established to remove commands so if one was proposed
with an audit of the command that meet it and those that do not then we can see
how we sit.

> I will send a patch v2 without this text on Monday.

Thanks. I have added onto my list adding the config setting for this command and
documentation. I should have done this when I added the command and I will sort
it out.

Chris


More information about the devel mailing list