[PATCH v2] cmd: cat: add new command

Sun Jun 19 17:08:40 CEST 2022

On 6/19/22 16:23, Roger Knecht wrote:
> On Thursday, June 9th, 2022 at 22:27, Roger Knecht <rknecht at pm.me> wrote:
>> On Wednesday, June 8th, 2022 at 15:59, Tom Rini trini at konsulko.com wrote:
>>
>>> On Sat, Jun 04, 2022 at 11:19:15AM +0000, Roger Knecht wrote:
>>>
>>>> Add cat command to print file content to standard out
>>>>
>>>> Signed-off-by: Roger Knecht rknecht at pm.me
>>>> ---
>>>> v2:
>>>> - Moved cat from boot to shell commands
>>>> - Added MAINTAINERS entry
>>>> - Added comments
>>>> - Improved variable naming
>>>>
>>>> MAINTAINERS | 5 +++++
>>>> cmd/Kconfig | 6 ++++++
>>>> cmd/Makefile | 1 +
>>>> cmd/cat.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>> 4 files changed, 70 insertions(+)
>>>> create mode 100644 cmd/cat.c
>>>>
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index 56be0bfad0..7c5cd178d9 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -729,6 +729,11 @@ M: Simon Glass sjg at chromium.org
>>>> S: Maintained
>>>> F: tools/buildman/
>>>>
>>>> +CAT
>>>> +M: Roger Knecht rknecht at pm.me
>>>> +S: Maintained
>>>> +F: cmd/cat.c
>>>> +
>>>> CFI FLASH
>>>> M: Stefan Roese sr at denx.de
>>>> S: Maintained
>>>> diff --git a/cmd/Kconfig b/cmd/Kconfig
>>>> index 69c1814d24..8b531c7ebe 100644
>>>> --- a/cmd/Kconfig
>>>> +++ b/cmd/Kconfig
>>>> @@ -1492,6 +1492,12 @@ endmenu
>>>>
>>>> menu "Shell scripting commands"
>>>>
>>>> +config CMD_CAT
>>>> + bool "cat"
>>>> + default y
>>>
>>> New commands shouldn't be default enabled. I also don't see a test.
>>> Please add a test, and enable the command in sandbox so the test is run.
>>> Thanks!
>>
>>
>> Thanks for the review. Will be fixed in the next version.
>
> Hi Tom,
>
> I'm running into issues when writing a test for cat:

Dear Roger,

tests for console commands should be written in Python, see
https://u-boot.readthedocs.io/en/latest/develop/py_testing.html.

You will need a test fixture that generates the disk image with the file
that you want to load and display in the test function. Have a look at
test/py/tests/test_efi_bootmgr/ as an example.

We have different files that may be worth printing with 'cat':

* UTF-8 text files like uEnv.txt
* script files like boot.scr that can be executed with the 'source' command
* binary files

I suggest that the 'cat' command should support all of these. For binary
files you can use the print_hex_dump() function for console output.

Please, provide a man-page for the 'cat' command in doc/usage/cmd/.
You can use doc/usage/cmd/fatinfo.rst as a template.

Best regards

Heinrich

>
> 1. malloc() allocates memory outside the sandbox emulated DRAM which causes fs_read() to fail:
>
> relevant code: (paddr - allocated by malloc() - is not within the gd->ram_size)
> ```
> void *phys_to_virt(phys_addr_t paddr)
> {
> [...]
>
>      /* If the address is within emulated DRAM, calculate the value */
>      if (paddr < gd->ram_size)
>          return (void *)(gd->arch.ram_buf + paddr);
>
> [...]
>
>      printf("%s: Cannot map sandbox address %lx (SDRAM from 0 to %lx)\n",
>             __func__, (ulong)paddr, (ulong)gd->ram_size);
>      os_abort();
>
> [...]
>
> ```
> output:
> phys_to_virt: Cannot map sandbox address 140093b0 (SDRAM from 0 to 8000000)
>
> trace:
> #0  __GI_raise (sig=sig at entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
> #1  0x00007ffff74e87f1 in __GI_abort () at abort.c:79
> #2  0x000055555558b352 in os_abort () at arch/sandbox/cpu/os.c:948
> #3  0x000055555558c093 in phys_to_virt (paddr=paddr at entry=335582128) at arch/sandbox/cpu/cpu.c:118
> #4  0x000055555558c1c2 in map_physmem (paddr=paddr at entry=335582128, len=<optimized out>, len at entry=0, flags=flags at entry=0) at arch/sandbox/cpu/cpu.c:179
> #5  0x000055555564a5b6 in map_sysmem (len=0, paddr=335582128) at ./arch/sandbox/include/asm/io.h:36
> #6  _fs_read (filename=0x1400c5d0 "file", addr=addr at entry=335582128, offset=offset at entry=0, len=len at entry=0, do_lmb_check=do_lmb_check at entry=0, actread=actread at entry=0x7fffffffe740) at fs/fs.c:596
> #7  0x000055555564a68c in fs_read (filename=<optimized out>, addr=addr at entry=335582128, offset=offset at entry=0, len=len at entry=0, actread=actread at entry=0x7fffffffe740) at fs/fs.c:611
> #8  0x00005555555a2a77 in do_cat (cmdtp=<optimized out>, flag=<optimized out>, argc=<optimized out>, argv=0x1400c650) at cmd/cat.c:42
> #9  0x00005555555de75e in cmd_always_repeatable (cmdtp=<optimized out>, flag=<optimized out>, argc=<optimized out>, argv=<optimized out>, repeatable=<optimized out>) at common/command.c:544
> #10 0x00005555555ddb56 in cmd_call (cmdtp=cmdtp at entry=0x555555accbe8 <_u_boot_list_2_cmd_2_cat>, flag=flag at entry=0, argc=argc at entry=4, argv=argv at entry=0x1400c650, repeatable=repeatable at entry=0x7fffffffe794) at common/command.c:580
> #11 0x00005555555de87c in cmd_process (flag=<optimized out>, flag at entry=0, argc=4, argv=0x1400c650, repeatable=repeatable at entry=0x555555af64d4 <flag_repeat>, ticks=ticks at entry=0x0) at common/command.c:635
> #12 0x00005555555cb9fd in run_pipe_real (pi=pi at entry=0x1400c500) at common/cli_hush.c:1676
> #13 0x00005555555cbdc5 in run_list_real (pi=pi at entry=0x1400c500) at common/cli_hush.c:1873
> #14 0x00005555555cbedc in run_list (pi=0x1400c500) at common/cli_hush.c:2022
> #15 0x00005555555cc100 in parse_stream_outer (inp=inp at entry=0x7fffffffe950, flag=flag at entry=2) at common/cli_hush.c:3206
> #16 0x00005555555cc183 in parse_file_outer () at common/cli_hush.c:3289
> #17 0x00005555555dd90f in cli_loop () at common/cli.c:229
> #18 0x00005555555c9acc in main_loop () at common/main.c:69
> #19 0x00005555555ccfd4 in run_main_loop () at common/board_r.c:590
> #20 0x00005555555cd32d in initcall_run_list (init_sequence=0x555555ae9220 <init_sequence_r>) at include/initcall.h:46
> #21 board_init_r (new_gd=<optimized out>, dest_addr=<optimized out>) at common/board_r.c:824
> #22 0x0000555555589e34 in main (argc=<optimized out>, argv=0x7fffffffed58) at arch/sandbox/cpu/start.c:529
>
> Is this a bug in the malloc implementation or does the fs_read() buffer require a different allocation?
>
>
> 2. Testing the cat command requires a filesystem. How should the filesystem be provided for tests?
>
> Currently I test the command in the sandbox by:
> $ ./u-boot
> => host bind 0 disk.ext2
> => cat host 0 file
>
> Shipping a binary filesystem in the patch is not a great idea.
> Alternatively the filesystem could be created as part of the test.
> What is your recommendation?
>
> Thanks,
> Roger