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

[Chris Johns]

On 16/10/20 3:28 pm, Sebastian Huber wrote:
> On 16/10/2020 01:36, Chris Johns wrote:
>> On 16/10/20 4:23 am, Sebastian Huber wrote:
>>>
>>> rtems-fdt.c, rtems-fdt-shell.c and cpukit/include/rtems/rtems-fdt.h
>>> seem to be dead code. They implement a shell command `fdt` but that
>>> command is not part of the shell nor of any macro in
>>> cpukit/include/rtems/shellconfig.h.
>>
>> This comment is wrong and should not have been in the commit message and should
>> _not_ have been pushed. As I stated in my review it could be simple issue of not
>> being in the config file and documentation. I feel EB needs to be more
>> restrained in its view of what is used and not used in RTEMS and someone new the
>> community might be advised to take a simple approach with wording when posting
>> patches.
> 
> The code in question
> 
> * had warnings,

Yes these have appeared.

> * is not integrated in the shell configuration,

Yes this is what I pointed out is the issue in my review but then I saw it had
been pushed.

> * has no tests,

And all the other commands have tests?

> * no documentation, and

Yes this is something that needs to be fixed.

> * is in the architecture-independent area,

Yes. FDT does not need to be limited to just device set up and bootloaders. It
is used in others ways such as managing the address space of programmable logic
without impacting on software.

> so a comment that says this seems to be dead code is justified from my point of
> view.

Yes and a comment or question asking is more than fine. A statement in a commit
that is pushed while I am sleeping is not. If there is a situation like this
please raise a ticket and it can serve a discussion point and a reminder.
Commits messages not a place we review to see what needs to be sorted out.

>> Please revert the change and then wait for the process to complete on the devel
>> mailing list to complete. I will ack the patches when _I_ am OK with them.
> 
> Yes, sorry, the review time was too short.

Yes and thanks :)

Chris