On Thursday, June 9th, 2022 at 22:27, Roger Knecht <rkne...@pm.me> wrote:
> On Wednesday, June 8th, 2022 at 15:59, Tom Rini tr...@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 rkne...@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 s...@chromium.org
> > > S: Maintained
> > > F: tools/buildman/
> > >
> > > +CAT
> > > +M: Roger Knecht rkne...@pm.me
> > > +S: Maintained
> > > +F: cmd/cat.c
> > > +
> > > CFI FLASH
> > > M: Stefan Roese s...@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:
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@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@entry=335582128) at
arch/sandbox/cpu/cpu.c:118
#4 0x000055555558c1c2 in map_physmem (paddr=paddr@entry=335582128,
len=<optimized out>, len@entry=0, flags=flags@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@entry=335582128,
offset=offset@entry=0, len=len@entry=0, do_lmb_check=do_lmb_check@entry=0,
actread=actread@entry=0x7fffffffe740) at fs/fs.c:596
#7 0x000055555564a68c in fs_read (filename=<optimized out>,
addr=addr@entry=335582128, offset=offset@entry=0, len=len@entry=0,
actread=actread@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@entry=0x555555accbe8
<_u_boot_list_2_cmd_2_cat>, flag=flag@entry=0, argc=argc@entry=4,
argv=argv@entry=0x1400c650, repeatable=repeatable@entry=0x7fffffffe794) at
common/command.c:580
#11 0x00005555555de87c in cmd_process (flag=<optimized out>, flag@entry=0,
argc=4, argv=0x1400c650, repeatable=repeatable@entry=0x555555af64d4
<flag_repeat>, ticks=ticks@entry=0x0) at common/command.c:635
#12 0x00005555555cb9fd in run_pipe_real (pi=pi@entry=0x1400c500) at
common/cli_hush.c:1676
#13 0x00005555555cbdc5 in run_list_real (pi=pi@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@entry=0x7fffffffe950,
flag=flag@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
