[PATCH v3] cmd: cat: add new command
Add cat command to print file content to standard out
Signed-off-by: Roger Knecht rknecht@pm.me --- 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 | 67 ++++++++++++++++++++++++++++++ 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, 185 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@chromium.org S: Maintained F: tools/buildman/
+CAT +M: Roger Knecht rknecht@pm.me +S: Maintained +F: cmd/cat.c + CFI FLASH M: Stefan Roese sr@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..c217617cd6 --- /dev/null +++ b/cmd/cat.c @@ -0,0 +1,67 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright 2022 + * Roger Knecht rknecht@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 *buffer; + phys_addr_t buffer_sysmem_addr; + loff_t file_size; + int ret; + + if (argc < 4) + return CMD_RET_USAGE; + + // get file size + ret = fs_set_blk_dev(argv[1], argv[2], FS_TYPE_ANY); + if (ret) + return ret; + + ret = fs_size(argv[3], &file_size); + if (ret) + return ret; + + // allocate memory for file content + buffer = malloc(file_size + 1); + if (!buffer) + return -ENOMEM; + + memset(buffer, 0, file_size + 1); + + // map pointer to system memory + buffer_sysmem_addr = map_to_sysmem(buffer); + + // read file to memory + ret = fs_set_blk_dev(argv[1], argv[2], FS_TYPE_ANY); + if (ret) + return ret; + + ret = fs_read(argv[3], buffer_sysmem_addr, 0, 0, &file_size); + if (ret) + return ret; + + // unmap system memory + unmap_sysmem(buffer); + + // print file content + buffer[file_size] = '\0'; + puts(buffer); + + free(buffer); + + return 0; +} + +U_BOOT_CMD(cat, 4, 1, do_cat, + "print file to standard output", + "<interface> <dev[:part]> <file>\n" + " - Print file from 'dev' on 'interface' to standard output"); 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 + +@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) + + 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 + +@pytest.mark.boardspec('sandbox') +@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) -- 2.25.1
On 8/18/22 18:54, Roger Knecht wrote:
Add cat command to print file content to standard out
Signed-off-by: Roger Knecht rknecht@pm.me
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 | 67 ++++++++++++++++++++++++++++++ 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, 185 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@chromium.org S: Maintained F: tools/buildman/
+CAT +M: Roger Knecht rknecht@pm.me +S: Maintained +F: cmd/cat.c
- CFI FLASH M: Stefan Roese sr@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..c217617cd6 --- /dev/null +++ b/cmd/cat.c @@ -0,0 +1,67 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright 2022
- Roger Knecht rknecht@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 *buffer;
- phys_addr_t buffer_sysmem_addr;
- loff_t file_size;
- int ret;
- if (argc < 4)
return CMD_RET_USAGE;- // get file size
- ret = fs_set_blk_dev(argv[1], argv[2], FS_TYPE_ANY);
- if (ret)
return ret;- ret = fs_size(argv[3], &file_size);
- if (ret)
return ret;- // allocate memory for file content
- buffer = malloc(file_size + 1);
- if (!buffer)
return -ENOMEM;
Please, do_cat must only return values from enum command_ret_t.
- memset(buffer, 0, file_size + 1);
Our calloc() implementation already sets the buffer to zero. So you can save one function call.
- // map pointer to system memory
- buffer_sysmem_addr = map_to_sysmem(buffer);
- // read file to memory
- ret = fs_set_blk_dev(argv[1], argv[2], FS_TYPE_ANY);
- if (ret)
return ret;- ret = fs_read(argv[3], buffer_sysmem_addr, 0, 0, &file_size);
- if (ret)
return ret;- // unmap system memory
- unmap_sysmem(buffer);
- // print file content
- buffer[file_size] = '\0';
- puts(buffer);
- free(buffer);
- return 0;
+}
+U_BOOT_CMD(cat, 4, 1, do_cat,
"print file to standard output",
Please, observe CONFIG_SYS_LONGHELP. Have a look at cmd/bootefi.c or others.
Best regards
Heinrich
"<interface> <dev[:part]> <file>\n"" - Print file from 'dev' on 'interface' to standard output");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
+@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)- 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
+@pytest.mark.boardspec('sandbox') +@pytest.mark.buildconfigspec('cmd_cat') +def test_cat(u_boot_console, cat_data):
- """ Unit test for cat
- Args:
u_boot_console -- U-Boot consolecat_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)
-- 2.25.1
Hi,
On Thu, 18 Aug 2022 at 11:08, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 8/18/22 18:54, Roger Knecht wrote:
Add cat command to print file content to standard out
Signed-off-by: Roger Knecht rknecht@pm.me
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 | 67 ++++++++++++++++++++++++++++++ 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, 185 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
Very nice patch, could be a good example for others to use.
diff --git a/MAINTAINERS b/MAINTAINERS index 5857fbf398..2864f84274 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -765,6 +765,11 @@ M: Simon Glass sjg@chromium.org S: Maintained F: tools/buildman/
+CAT +M: Roger Knecht rknecht@pm.me +S: Maintained +F: cmd/cat.c
- CFI FLASH M: Stefan Roese sr@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
Not needed as things default to n
helpPrint 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..c217617cd6 --- /dev/null +++ b/cmd/cat.c @@ -0,0 +1,67 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright 2022
- Roger Knecht rknecht@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 *buffer;phys_addr_t buffer_sysmem_addr;
'addr' is shorter
loff_t file_size;int ret;if (argc < 4)return CMD_RET_USAGE;// get file sizeret = fs_set_blk_dev(argv[1], argv[2], FS_TYPE_ANY);if (ret)return ret;ret = fs_size(argv[3], &file_size);if (ret)return ret;// allocate memory for file contentbuffer = malloc(file_size + 1);if (!buffer)return -ENOMEM;Please, do_cat must only return values from enum command_ret_t.
The easiest way to do this is to put your code (from the 'get file size' onwards) in a separate function which is called by this function. It can take args like filename and device. Then when it returns an error code you can print it and return CMD_RET_FAILURE
memset(buffer, 0, file_size + 1);Our calloc() implementation already sets the buffer to zero. So you can save one function call.
// map pointer to system memorybuffer_sysmem_addr = map_to_sysmem(buffer);// read file to memoryret = fs_set_blk_dev(argv[1], argv[2], FS_TYPE_ANY);if (ret)return ret;ret = fs_read(argv[3], buffer_sysmem_addr, 0, 0, &file_size);if (ret)return ret;// unmap system memoryunmap_sysmem(buffer);
Actually map_to_sysmem() does not create a map, so you don't need this. It is only needed with a call to map_sysmem(). I will send a patch to update the docs.
// print file contentbuffer[file_size] = '\0';puts(buffer);free(buffer);return 0;+}
+U_BOOT_CMD(cat, 4, 1, do_cat,
"print file to standard output",Please, observe CONFIG_SYS_LONGHELP. Have a look at cmd/bootefi.c or others.
Also how about doc/usage/cmd/cat.rst ?
Regards, Simon
------- Original Message ------- On Friday, August 19th, 2022 at 16:08, Simon Glass sjg@chromium.org wrote:
Hi,
Hi Simon,
On Thu, 18 Aug 2022 at 11:08, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 8/18/22 18:54, Roger Knecht wrote:
Add cat command to print file content to standard out
Signed-off-by: Roger Knecht rknecht@pm.me
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 | 67 ++++++++++++++++++++++++++++++ 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, 185 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
Very nice patch, could be a good example for others to use.
diff --git a/MAINTAINERS b/MAINTAINERS index 5857fbf398..2864f84274 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -765,6 +765,11 @@ M: Simon Glass sjg@chromium.org S: Maintained F: tools/buildman/
+CAT +M: Roger Knecht rknecht@pm.me +S: Maintained +F: cmd/cat.c
CFI FLASH M: Stefan Roese sr@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
Not needed as things default to n
Will be fixed in v5.
- 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..c217617cd6 --- /dev/null +++ b/cmd/cat.c @@ -0,0 +1,67 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright 2022
- Roger Knecht rknecht@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 *buffer;
- phys_addr_t buffer_sysmem_addr;
'addr' is shorter
Ok
- loff_t file_size;
- int ret;
- if (argc < 4)
- return CMD_RET_USAGE;
- // get file size
- ret = fs_set_blk_dev(argv[1], argv[2], FS_TYPE_ANY);
- if (ret)
- return ret;
- ret = fs_size(argv[3], &file_size);
- if (ret)
- return ret;
- // allocate memory for file content
- buffer = malloc(file_size + 1);
- if (!buffer)
- return -ENOMEM;
Please, do_cat must only return values from enum command_ret_t.
The easiest way to do this is to put your code (from the 'get file size' onwards) in a separate function which is called by this function. It can take args like filename and device. Then when it returns an error code you can print it and return CMD_RET_FAILURE
- memset(buffer, 0, file_size + 1);
Our calloc() implementation already sets the buffer to zero. So you can save one function call.
- // map pointer to system memory
- buffer_sysmem_addr = map_to_sysmem(buffer);
- // read file to memory
- ret = fs_set_blk_dev(argv[1], argv[2], FS_TYPE_ANY);
- if (ret)
- return ret;
- ret = fs_read(argv[3], buffer_sysmem_addr, 0, 0, &file_size);
- if (ret)
- return ret;
- // unmap system memory
- unmap_sysmem(buffer);
Actually map_to_sysmem() does not create a map, so you don't need this. It is only needed with a call to map_sysmem(). I will send a patch to update the docs.
The map_to_sysmem() is used to solve a problem in the sandbox which I described in more detail here: https://lists.denx.de/pipermail/u-boot/2022-June/486883.html Let me know if there is a better solution.
- // print file content
- buffer[file_size] = '\0';
- puts(buffer);
- free(buffer);
- return 0;
+}
+U_BOOT_CMD(cat, 4, 1, do_cat,
- "print file to standard output",
Please, observe CONFIG_SYS_LONGHELP. Have a look at cmd/bootefi.c or others.
Also how about doc/usage/cmd/cat.rst ?
I don't understand this remark. There is already a `doc/usage/cmd/cat.rst` in the patch.
Regards, Simon
Thanks, Roger
Hi Roger,
On Sun, 21 Aug 2022 at 07:27, Roger Knecht rknecht@pm.me wrote:
------- Original Message ------- On Friday, August 19th, 2022 at 16:08, Simon Glass sjg@chromium.org wrote:
Hi,
Hi Simon,
On Thu, 18 Aug 2022 at 11:08, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 8/18/22 18:54, Roger Knecht wrote:
Add cat command to print file content to standard out
Signed-off-by: Roger Knecht rknecht@pm.me
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:map_to_sysmem
- 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 | 67 ++++++++++++++++++++++++++++++ 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, 185 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
Very nice patch, could be a good example for others to use.
diff --git a/MAINTAINERS b/MAINTAINERS index 5857fbf398..2864f84274 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -765,6 +765,11 @@ M: Simon Glass sjg@chromium.org S: Maintained F: tools/buildman/
+CAT +M: Roger Knecht rknecht@pm.me +S: Maintained +F: cmd/cat.c
CFI FLASH M: Stefan Roese sr@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
Not needed as things default to n
Will be fixed in v5.
- 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..c217617cd6 --- /dev/null +++ b/cmd/cat.c @@ -0,0 +1,67 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright 2022
- Roger Knecht rknecht@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 *buffer;
- phys_addr_t buffer_sysmem_addr;
'addr' is shorter
Ok
- loff_t file_size;
- int ret;
- if (argc < 4)
- return CMD_RET_USAGE;
- // get file size
- ret = fs_set_blk_dev(argv[1], argv[2], FS_TYPE_ANY);
- if (ret)
- return ret;
- ret = fs_size(argv[3], &file_size);
- if (ret)
- return ret;
- // allocate memory for file content
- buffer = malloc(file_size + 1);
- if (!buffer)
- return -ENOMEM;
Please, do_cat must only return values from enum command_ret_t.
The easiest way to do this is to put your code (from the 'get file size' onwards) in a separate function which is called by this function. It can take args like filename and device. Then when it returns an error code you can print it and return CMD_RET_FAILURE
- memset(buffer, 0, file_size + 1);
Our calloc() implementation already sets the buffer to zero. So you can save one function call.
- // map pointer to system memory
- buffer_sysmem_addr = map_to_sysmem(buffer);
- // read file to memory
- ret = fs_set_blk_dev(argv[1], argv[2], FS_TYPE_ANY);
- if (ret)
- return ret;
- ret = fs_read(argv[3], buffer_sysmem_addr, 0, 0, &file_size);
- if (ret)
- return ret;
- // unmap system memory
- unmap_sysmem(buffer);
Actually map_to_sysmem() does not create a map, so you don't need this. It is only needed with a call to map_sysmem(). I will send a patch to update the docs.
The map_to_sysmem() is used to solve a problem in the sandbox which I described in more detail here: https://lists.denx.de/pipermail/u-boot/2022-June/486883.html Let me know if there is a better solution.
Actually I was talking the unmap_sysmem(), the line above my comment. The unmap is used after a map, where as the map_to_sysmem() is doing something different. Yes you need map_to_sysmem(), but you should not need the unmap_sysmem(). If I am missing something, let me know.
[..]
Also how about doc/usage/cmd/cat.rst ?
I don't understand this remark. There is already a `doc/usage/cmd/cat.rst` in the patch.
Yes sorry about that. I thought I deleted that line.
Regards, Simon
------- Original Message ------- On Sunday, August 21st, 2022 at 14:18, Simon Glass sjg@chromium.org wrote:
Hi Roger,
On Sun, 21 Aug 2022 at 07:27, Roger Knecht rknecht@pm.me wrote:
------- Original Message ------- On Friday, August 19th, 2022 at 16:08, Simon Glass sjg@chromium.org wrote:
Hi, Hi Simon,
On Thu, 18 Aug 2022 at 11:08, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 8/18/22 18:54, Roger Knecht wrote:
Add cat command to print file content to standard out
Signed-off-by: Roger Knecht rknecht@pm.me
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:map_to_sysmem
- 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 | 67 ++++++++++++++++++++++++++++++ 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, 185 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
Very nice patch, could be a good example for others to use.
diff --git a/MAINTAINERS b/MAINTAINERS index 5857fbf398..2864f84274 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -765,6 +765,11 @@ M: Simon Glass sjg@chromium.org S: Maintained F: tools/buildman/
+CAT +M: Roger Knecht rknecht@pm.me +S: Maintained +F: cmd/cat.c
CFI FLASH M: Stefan Roese sr@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
Not needed as things default to n Will be fixed in v5.
- 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..c217617cd6 --- /dev/null +++ b/cmd/cat.c @@ -0,0 +1,67 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright 2022
- Roger Knecht rknecht@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 *buffer;
- phys_addr_t buffer_sysmem_addr;
'addr' is shorter Ok
- loff_t file_size;
- int ret;
- if (argc < 4)
- return CMD_RET_USAGE;
- // get file size
- ret = fs_set_blk_dev(argv[1], argv[2], FS_TYPE_ANY);
- if (ret)
- return ret;
- ret = fs_size(argv[3], &file_size);
- if (ret)
- return ret;
- // allocate memory for file content
- buffer = malloc(file_size + 1);
- if (!buffer)
- return -ENOMEM;
Please, do_cat must only return values from enum command_ret_t.
The easiest way to do this is to put your code (from the 'get file size' onwards) in a separate function which is called by this function. It can take args like filename and device. Then when it returns an error code you can print it and return CMD_RET_FAILURE
- memset(buffer, 0, file_size + 1);
Our calloc() implementation already sets the buffer to zero. So you can save one function call.
- // map pointer to system memory
- buffer_sysmem_addr = map_to_sysmem(buffer);
- // read file to memory
- ret = fs_set_blk_dev(argv[1], argv[2], FS_TYPE_ANY);
- if (ret)
- return ret;
- ret = fs_read(argv[3], buffer_sysmem_addr, 0, 0, &file_size);
- if (ret)
- return ret;
- // unmap system memory
- unmap_sysmem(buffer);
Actually map_to_sysmem() does not create a map, so you don't need this. It is only needed with a call to map_sysmem(). I will send a patch to update the docs.
The map_to_sysmem() is used to solve a problem in the sandbox which I described in more detail here: https://lists.denx.de/pipermail/u-boot/2022-June/486883.html Let me know if there is a better solution.
Actually I was talking the unmap_sysmem(), the line above my comment. The unmap is used after a map, where as the map_to_sysmem() is doing something different. Yes you need map_to_sysmem(), but you should not need the unmap_sysmem(). If I am missing something, let me know.
[..]
Also how about doc/usage/cmd/cat.rst ? I don't understand this remark. There is already a `doc/usage/cmd/cat.rst` in the patch.
Yes sorry about that. I thought I deleted that line.
Regards, Simon
Thanks for the clarification Simon. I will remove the unmap_sysmem() call in the next patch.
Regards, Roger
participants (3)
-
Heinrich Schuchardt -
Roger Knecht -
Simon Glass