[PATCH v4] cmd: cat: add new command

Sun Aug 21 15:35:17 CEST 2022

------- Original Message -------
On Sunday, August 21st, 2022 at 07:35, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> 
> 
> On 8/19/22 10:35, Roger Knecht wrote:
> 
> > Add cat command to print file content to standard out
> > 
> > Signed-off-by: Roger Knecht rknecht at pm.me
> > ---
> > v4:
> > - Return only values from enum command_ret_t in do_cat()
> > - Use calloc() instead of malloc() for zero initialized memory
> > - Make use of CONFIG_SYS_LONGHELP
> > - Improved error messages
> > 
> > v3:
> > - Disable 'cat' by default (CONFIG_CMD_CAT=n)
> > - Enable 'cat' in sandbox and sandbox64 defconfig
> > - Use map_to_sysmem() to fix "phys_to_virt: Cannot map sandbox address"
> > - Use puts() instead of a loop
> > - Added python test
> > - Addd usage documentation
> > 
> > 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 | 88 ++++++++++++++++++++++++++++++
> > configs/sandbox64_defconfig | 1 +
> > configs/sandbox_defconfig | 1 +
> > doc/usage/cmd/cat.rst | 49 +++++++++++++++++
> > test/py/tests/test_cat/conftest.py | 33 +++++++++++
> > test/py/tests/test_cat/test_cat.py | 22 ++++++++
> > 9 files changed, 206 insertions(+)
> > create mode 100644 cmd/cat.c
> > create mode 100644 doc/usage/cmd/cat.rst
> > create mode 100644 test/py/tests/test_cat/conftest.py
> > create mode 100644 test/py/tests/test_cat/test_cat.py
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 5857fbf398..2864f84274 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -765,6 +765,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 211ebe9c87..ce7e876475 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -1531,6 +1531,12 @@ endmenu
> > 
> > menu "Shell scripting commands"
> > 
> > +config CMD_CAT
> > + bool "cat"
> > + default n
> > + help
> > + Print file to standard output
> > +
> > config CMD_ECHO
> > bool "echo"
> > default y
> > diff --git a/cmd/Makefile b/cmd/Makefile
> > index 6e87522b62..1d2590e958 100644
> > --- a/cmd/Makefile
> > +++ b/cmd/Makefile
> > @@ -38,6 +38,7 @@ obj-$(CONFIG_CMD_BOOTZ) += bootz.o
> > obj-$(CONFIG_CMD_BOOTI) += booti.o
> > obj-$(CONFIG_CMD_BTRFS) += btrfs.o
> > obj-$(CONFIG_CMD_BUTTON) += button.o
> > +obj-$(CONFIG_CMD_CAT) += cat.o
> > obj-$(CONFIG_CMD_CACHE) += cache.o
> > obj-$(CONFIG_CMD_CBFS) += cbfs.o
> > obj-$(CONFIG_CMD_CLK) += clk.o
> > diff --git a/cmd/cat.c b/cmd/cat.c
> > new file mode 100644
> > index 0000000000..c2c17b7abc
> > --- /dev/null
> > +++ b/cmd/cat.c
> > @@ -0,0 +1,88 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2022
> > + * Roger Knecht rknecht at pm.de
> > + */
> > +
> > +#include <common.h>
> > +#include <command.h>
> > +#include <fs.h>
> > +#include <malloc.h>
> > +#include <mapmem.h>
> > +
> > +static int do_cat(struct cmd_tbl *cmdtp, int flag, int argc,
> > + char *const argv[])
> > +{
> > + char *ifname;
> > + char *dev;
> > + char *file;
> > + char *buffer;
> > + phys_addr_t buffer_sysmem_addr;
> > + loff_t file_size;
> > +
> > + if (argc < 4)
> > + return CMD_RET_USAGE;
> > +
> > + ifname = argv[1];
> > + dev = argv[2];
> > + file = argv[3];
> > +
> > + // check file exists
> > + if (fs_set_blk_dev(ifname, dev, FS_TYPE_ANY))
> > + return CMD_RET_FAILURE;
> > +
> > + if (!fs_exists(file)) {
> > + log_err("File does not exist: ifname=%s dev=%s file=%s\n", ifname, dev, file);
> > + return CMD_RET_FAILURE;
> > + }
> > +
> > + // get file size
> > + if (fs_set_blk_dev(ifname, dev, FS_TYPE_ANY))
> > + return CMD_RET_FAILURE;
> > +
> > + if (fs_size(file, &file_size)) {
> > + log_err("Cannot read file size: ifname=%s dev=%s file=%s\n", ifname, dev, file);
> > + return CMD_RET_FAILURE;
> > + }
> > +
> > + // allocate memory for file content
> > + buffer = calloc(sizeof(char), file_size + 1);
> > + if (!buffer) {
> > + log_err("Out of memory\n");
> > + return CMD_RET_FAILURE;
> > + }
> > +
> > + // map pointer to system memory
> > + buffer_sysmem_addr = map_to_sysmem(buffer);
> > +
> > + // read file to memory
> > + if (fs_set_blk_dev(ifname, dev, FS_TYPE_ANY))
> > + return CMD_RET_FAILURE;
> > +
> > + if (fs_read(file, buffer_sysmem_addr, 0, 0, &file_size)) {
> > + log_err("Cannot read file: ifname=%s dev=%s file=%s\n", ifname, dev, file);
> > + return CMD_RET_FAILURE;
> > + }
> > +
> > + // unmap system memory
> > + unmap_sysmem(buffer);
> > +
> > + // print file content
> > + buffer[file_size] = '\0';
> > + puts(buffer);
> > +
> > + free(buffer);
> > +
> > + return 0;
> > +}
> > +
> > +#ifdef CONFIG_SYS_LONGHELP
> > +static char cat_help_text[] =
> > + "<interface> <dev[:part]> <file>\n"
> > + " - Print file from 'dev' on 'interface' to standard output\n";
> > +#endif
> > +
> > +U_BOOT_CMD(cat, 4, 1, do_cat,
> > + "Print file to standard output",
> > + cat_help_text
> > +);
> > diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
> > index 6553568e76..b2c9f19f11 100644
> > --- a/configs/sandbox64_defconfig
> > +++ b/configs/sandbox64_defconfig
> > @@ -22,6 +22,7 @@ CONFIG_CONSOLE_RECORD=y
> > CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000
> > CONFIG_PRE_CONSOLE_BUFFER=y
> > CONFIG_DISPLAY_BOARDINFO_LATE=y
> > +CONFIG_CMD_CAT=y
> > CONFIG_CMD_CPU=y
> > CONFIG_CMD_LICENSE=y
> > CONFIG_CMD_BOOTZ=y
> > diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> > index eba7bcbb48..2136c76fe3 100644
> > --- a/configs/sandbox_defconfig
> > +++ b/configs/sandbox_defconfig
> > @@ -36,6 +36,7 @@ CONFIG_LOG_DEFAULT_LEVEL=6
> > CONFIG_DISPLAY_BOARDINFO_LATE=y
> > CONFIG_STACKPROTECTOR=y
> > CONFIG_ANDROID_AB=y
> > +CONFIG_CMD_CAT=y
> > CONFIG_CMD_CPU=y
> > CONFIG_CMD_LICENSE=y
> > CONFIG_CMD_BOOTM_PRE_LOAD=y
> > diff --git a/doc/usage/cmd/cat.rst b/doc/usage/cmd/cat.rst
> > new file mode 100644
> > index 0000000000..5ef4731fe3
> > --- /dev/null
> > +++ b/doc/usage/cmd/cat.rst
> > @@ -0,0 +1,49 @@
> > +.. SPDX-License-Identifier: GPL-2.0+:
> > +
> > +cat command
> > +===============
> > +
> > +Synopsis
> > +--------
> > +
> > +::
> > +
> > + cat <interface> <dev[:part]> <file>
> > +
> > +Description
> > +-----------
> > +
> > +The cat command prints the file content to standard out.
> > +
> > +interface
> > + interface for accessing the block device (mmc, sata, scsi, usb, ....)
> > +
> > +dev
> > + device number
> > +
> > +part
> > + partition number, defaults to 1
> > +
> > +file
> > + path to file
> > +
> > +Example
> > +-------
> > +
> > +Here is the output for a example text file:
> > +
> > +::
> > +
> > + => cat mmc 0:1 hello
> > + hello world
> > + =>
> > +
> > +Configuration
> > +-------------
> > +
> > +The cat command is only available if CONFIG_CMD_CAT=y.
> > +
> > +Return value
> > +------------
> > +
> > +The return value $? is set to 0 (true) if the file is readable, otherwise it returns a non-zero error code.
> > diff --git a/test/py/tests/test_cat/conftest.py b/test/py/tests/test_cat/conftest.py
> > new file mode 100644
> > index 0000000000..426f64485f
> > --- /dev/null
> > +++ b/test/py/tests/test_cat/conftest.py
> > @@ -0,0 +1,33 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> > +
> > +"""Fixture for cat command test
> > +"""
> > +
> > +import os
> > +import shutil
> > +from subprocess import check_call
> > +import pytest
> > +
> > + at pytest.fixture(scope='session')
> > +def cat_data(u_boot_config):
> > + """Set up a file system to be used in cat tests
> > +
> > + Args:
> > + u_boot_config -- U-boot configuration.
> > +
> > + Return:
> > + A path to disk image to be used for testing
> > + """
> > + mnt_point = u_boot_config.persistent_data_dir + '/test_cat'
> > + image_path = u_boot_config.persistent_data_dir + '/cat.img'
> > +
> > + shutil.rmtree(mnt_point, ignore_errors=True)
> > + os.mkdir(mnt_point, mode = 0o755)
> > +
> > + with open(mnt_point + '/hello', 'w', encoding = 'ascii') as file:
> > + file.write('hello world\n')
> > +
> > + check_call(f'virt-make-fs --partition=gpt --size=+1M --type=vfat {mnt_point} {image_path}',
> > + shell=True)
> 
> 
> Best practice is to remove test data after usage. You could follow this
> pattern:
> 
> @pytest.fixture()
> def resource():
> print("setup")
> yield "resource"
> print("teardown")
> 
> See
> https://docs.pytest.org/en/latest/how-to/fixtures.html#teardown-cleanup-aka-fixture-finalization
> 
> virt-make-fs fails if the user has no read access to the kernel. So you
> should handle CalledProcessError like in test/py/tests/test_fs/conftest.py
> 

Will be fixed in the next patch.

> > +
> > + return image_path
> > diff --git a/test/py/tests/test_cat/test_cat.py b/test/py/tests/test_cat/test_cat.py
> > new file mode 100644
> > index 0000000000..41ddefa40c
> > --- /dev/null
> > +++ b/test/py/tests/test_cat/test_cat.py
> > @@ -0,0 +1,22 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> > +
> > +""" Unit test for cat command
> > +"""
> > +
> > +import pytest
> > +
> > + at pytest.mark.boardspec('sandbox')
> > + at pytest.mark.buildconfigspec('cmd_cat')
> > +def test_cat(u_boot_console, cat_data):
> > + """ Unit test for cat
> > +
> > + Args:
> > + u_boot_console -- U-Boot console
> > + cat_data -- Path to the disk image used for testing.
> > + """
> > + u_boot_console.run_command(cmd = f'host bind 0 {cat_data}')
> > +
> > + response = u_boot_console.run_command(cmd = 'cat host 0 hello')
> > + assert 'hello world' == response
> > +
> > + u_boot_console.run_command(cmd = 'exit', wait_for_echo=False)
> 
> 
> As Simon already indicated this line should be removed.

Will fix

> 
> Best regards
> 
> Heinrich
> 
> > --
> > 2.25.1

Thanks for the reviews.
I will send patch v5 soon.

Regards,
Roger