[PATCH 0/2] test: cmd: add binop tests and enable on sandbox

[PATCH 0/2] test: cmd: add binop tests and enable on sandbox

[Maxin B. John]

binop has no tests today. Add some tests to catch regressions.

Each operation (xor, or, and) is tested with hex strings, environment
variables or memory as sources. Results go to an environment variable,
memory or the console. The tests also cover zero padding, console
wrapping, bad input, plus the heap overrun.

CMD_BINOP is enabled for sandbox/sandbox64, so both 32-bit/64-bit builds
run the tests.

The tests are a follow-up to the separate "cmd: binop: validate inputs
and fix heap overrun" patch.

Tested with:
  ./test/py/test.py --bd sandbox --build -k ut_cmd_cmd_test_binop
  ./test/py/test.py --bd sandbox64 --build -k ut_cmd_cmd_test_binop

Maxin B. John (2):
  configs: sandbox: Enable binop command
  test: cmd: add tests for the binop command

 configs/sandbox64_defconfig |   1 +
 configs/sandbox_defconfig   |   1 +
 test/cmd/Makefile           |   1 +
 test/cmd/binop.c            | 192 ++++++++++++++++++++++++++++++++++++
 4 files changed, 195 insertions(+)
 create mode 100644 test/cmd/binop.c

-- 
2.47.3

[PATCH 1/2] configs: sandbox: Enable binop command

[Maxin B. John]

Turn on CMD_BINOP for sandbox and sandbox64 so the binop tests are
built and run in CI on both 32-bit and 64-bit builds.

Signed-off-by: Maxin B. John <maxin.john@gmail.com>
---
 configs/sandbox64_defconfig | 1 +
 configs/sandbox_defconfig   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index f861d3ef06db..c4ad63ce63be 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -43,6 +43,7 @@ CONFIG_CMD_NVEDIT_EFI=y
 CONFIG_CMD_NVEDIT_INFO=y
 CONFIG_CMD_NVEDIT_LOAD=y
 CONFIG_CMD_NVEDIT_SELECT=y
+CONFIG_CMD_BINOP=y
 CONFIG_LOOPW=y
 CONFIG_CMD_MD5SUM=y
 CONFIG_CMD_MX_CYCLIC=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index a3fa52460827..c63a4df8befa 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -74,6 +74,7 @@ CONFIG_CMD_NVEDIT_EFI=y
 CONFIG_CMD_NVEDIT_INFO=y
 CONFIG_CMD_NVEDIT_LOAD=y
 CONFIG_CMD_NVEDIT_SELECT=y
+CONFIG_CMD_BINOP=y
 CONFIG_LOOPW=y
 CONFIG_CMD_MD5SUM=y
 CONFIG_CMD_MEM_SEARCH=y
-- 
2.47.3

[PATCH 2/2] test: cmd: add tests for the binop command

[Maxin B. John]

binop has had no tests so far. Add some, so that bugs like the heap
overrun are caught in CI.

Each operation (xor, or, and) is run on hex strings, env vars or
memory, with the result going to an env var, memory or the console.

The tests also check that:
- short inputs are zero-padded
- console output wraps after 16 bytes
- bad counts, operators, addresses or hex strings are rejected
  without touching the destination

There is also a test for the heap overrun.

Signed-off-by: Maxin B. John <maxin.john@gmail.com>
---
 test/cmd/Makefile |   1 +
 test/cmd/binop.c  | 192 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 193 insertions(+)
 create mode 100644 test/cmd/binop.c

diff --git a/test/cmd/Makefile b/test/cmd/Makefile
index e734933d5f73..79cbb291250b 100644
--- a/test/cmd/Makefile
+++ b/test/cmd/Makefile
@@ -13,6 +13,7 @@ endif
 obj-$(CONFIG_HUSH_PARSER) += exit.o
 obj-$(CONFIG_X86) += cpuid.o msr.o
 obj-$(CONFIG_CMD_ADDRMAP) += addrmap.o
+obj-$(CONFIG_CMD_BINOP) += binop.o
 ifdef CONFIG_CONSOLE_RECORD
 obj-$(CONFIG_CMD_ACPI) += acpi.o
 endif
diff --git a/test/cmd/binop.c b/test/cmd/binop.c
new file mode 100644
index 000000000000..4cea19607103
--- /dev/null
+++ b/test/cmd/binop.c
@@ -0,0 +1,192 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Tests for the binop command
+ */
+
+#include <command.h>
+#include <env.h>
+#include <mapmem.h>
+#include <test/cmd.h>
+#include <test/ut.h>
+
+#define MEM_ADDR	((ulong)CONFIG_SYS_LOAD_ADDR + 0x1000)
+
+static int cmd_test_binop_ops(struct unit_test_state *uts)
+{
+	ut_assertok(run_command("binop xor 4 01020304 ffffffff binop_out", 0));
+	ut_asserteq_str("fefdfcfb", env_get("binop_out"));
+
+	ut_assertok(run_command("binop or 4 01020304 00000000 binop_out", 0));
+	ut_asserteq_str("01020304", env_get("binop_out"));
+
+	ut_assertok(run_command("binop and 4 0f0f0f0f 00ff00ff binop_out", 0));
+	ut_asserteq_str("000f000f", env_get("binop_out"));
+
+	ut_assertok(run_command("binop xor 2 AABB CCDD binop_out", 0));
+	ut_asserteq_str("6666", env_get("binop_out"));
+
+	env_set("binop_out", NULL);
+
+	return 0;
+}
+
+CMD_TEST(cmd_test_binop_ops, 0);
+
+static int cmd_test_binop_env(struct unit_test_state *uts)
+{
+	ut_assertok(env_set("binop_a", "aabbccdd"));
+	ut_assertok(env_set("binop_b", "11223344"));
+
+	ut_assertok(run_command("binop xor 4 binop_a binop_b binop_out", 0));
+	ut_asserteq_str("bb99ff99", env_get("binop_out"));
+
+	/* Short env var is zero-padded */
+	ut_assertok(env_set("binop_a", "01"));
+	ut_assertok(run_command("binop xor 4 binop_a binop_b binop_out", 0));
+	ut_asserteq_str("10223344", env_get("binop_out"));
+
+	env_set("binop_a", NULL);
+	env_set("binop_b", NULL);
+	env_set("binop_out", NULL);
+
+	return 0;
+}
+
+CMD_TEST(cmd_test_binop_env, 0);
+
+static int cmd_test_binop_mem(struct unit_test_state *uts)
+{
+	const u8 expect[] = { 0x0e, 0x0d, 0x0c, 0x0b };
+	u8 *buf;
+
+	buf = map_sysmem(MEM_ADDR, 8);
+	memset(buf, 0, 8);
+	buf[0] = 0x01;
+	buf[1] = 0x02;
+	buf[2] = 0x03;
+	buf[3] = 0x04;
+
+	ut_assertok(run_commandf("binop xor 4 *0x%lx 01010101 binop_out",
+				 MEM_ADDR));
+	ut_asserteq_str("00030205", env_get("binop_out"));
+
+	ut_assertok(run_commandf("binop xor 4 *0x%lx 0f0f0f0f *0x%lx",
+				 MEM_ADDR, MEM_ADDR + 4));
+	ut_asserteq_mem(expect, buf + 4, sizeof(expect));
+
+	unmap_sysmem(buf);
+	env_set("binop_out", NULL);
+
+	return 0;
+}
+
+CMD_TEST(cmd_test_binop_mem, 0);
+
+static int cmd_test_binop_short(struct unit_test_state *uts)
+{
+	ut_assertok(run_command("binop xor 4 01 02 binop_out", 0));
+	ut_asserteq_str("03000000", env_get("binop_out"));
+
+	ut_assertok(env_set("binop_a", "0102"));
+	ut_assertok(run_command("binop or 4 binop_a 03 binop_out", 0));
+	ut_asserteq_str("03020000", env_get("binop_out"));
+
+	env_set("binop_a", NULL);
+	env_set("binop_out", NULL);
+
+	return 0;
+}
+
+CMD_TEST(cmd_test_binop_short, 0);
+
+static int cmd_test_binop_print(struct unit_test_state *uts)
+{
+	ut_assertok(run_command("binop xor 2 0102 0304", 0));
+	ut_assert_nextline("02 06 ");
+	ut_assert_console_end();
+
+	/* Output wraps after 16 bytes */
+	ut_assertok(run_command("binop xor 17 000102030405060708090a0b0c0d0e0f10 00",
+				0));
+	ut_assert_nextline("00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f ");
+	ut_assert_nextline("10 ");
+	ut_assert_console_end();
+
+	return 0;
+}
+
+CMD_TEST(cmd_test_binop_print, UTF_CONSOLE);
+
+static int cmd_test_binop_invalid(struct unit_test_state *uts)
+{
+	/* Failed operations must not change binop_out */
+	ut_assertok(env_set("binop_out", "deadbeef"));
+
+	/* Literal hex string: too long, odd length, not hex */
+	ut_asserteq(1, run_command("binop xor 2 010203 01 binop_out", 0));
+	ut_asserteq_str("deadbeef", env_get("binop_out"));
+
+	ut_asserteq(1, run_command("binop xor 2 010 01 binop_out", 0));
+	ut_asserteq_str("deadbeef", env_get("binop_out"));
+
+	ut_asserteq(1, run_command("binop xor 2 zz 01 binop_out", 0));
+	ut_asserteq_str("deadbeef", env_get("binop_out"));
+
+	/* Env var value: not hex (but fits the count), too long, odd length */
+	ut_assertok(env_set("binop_bad", "zzzz"));
+	ut_asserteq(1, run_command("binop xor 2 binop_bad 01 binop_out", 0));
+	ut_asserteq_str("deadbeef", env_get("binop_out"));
+
+	ut_assertok(env_set("binop_big", "0102030405"));
+	ut_asserteq(1, run_command("binop xor 2 binop_big 01 binop_out", 0));
+	ut_asserteq_str("deadbeef", env_get("binop_out"));
+
+	ut_assertok(env_set("binop_odd", "010"));
+	ut_asserteq(1, run_command("binop xor 2 binop_odd 01 binop_out", 0));
+	ut_asserteq_str("deadbeef", env_get("binop_out"));
+
+	/* Invalid count and operator */
+	ut_asserteq(1, run_command("binop xor 0 01 02 binop_out", 0));
+	ut_asserteq_str("deadbeef", env_get("binop_out"));
+
+	ut_asserteq(1, run_command("binop xor x 01 02 binop_out", 0));
+	ut_asserteq_str("deadbeef", env_get("binop_out"));
+
+	ut_asserteq(1, run_command("binop nope 2 0102 0304 binop_out", 0));
+	ut_asserteq_str("deadbeef", env_get("binop_out"));
+
+	/* Invalid source and destination addresses */
+	ut_asserteq(1, run_command("binop xor 2 *nope 01 binop_out", 0));
+	ut_asserteq_str("deadbeef", env_get("binop_out"));
+
+	ut_asserteq(1, run_command("binop xor 2 0102 0304 *nope", 0));
+
+	env_set("binop_bad", NULL);
+	env_set("binop_big", NULL);
+	env_set("binop_odd", NULL);
+	env_set("binop_out", NULL);
+
+	return 0;
+}
+
+CMD_TEST(cmd_test_binop_invalid, 0);
+
+static int cmd_test_binop_overrun(struct unit_test_state *uts)
+{
+	/*
+	 * A 25-byte value with a 4-byte count used to overrun the 4-byte
+	 * heap buffer. The command must now reject it.
+	 */
+	ut_assertok(env_set("binop_first", "01020304"));
+	ut_assertok(env_set("binop_second",
+			    "12334545434543534543534535456456456543534543534534"));
+
+	ut_asserteq(1, run_command("binop xor 4 binop_first binop_second", 0));
+
+	env_set("binop_first", NULL);
+	env_set("binop_second", NULL);
+
+	return 0;
+}
+
+CMD_TEST(cmd_test_binop_overrun, 0);
-- 
2.47.3

Re: [PATCH 1/2] configs: sandbox: Enable binop command

[Heinrich Schuchardt]

On 10/29/26 18:34, Maxin B. John wrote:
> Turn on CMD_BINOP for sandbox and sandbox64 so the binop tests are
> built and run in CI on both 32-bit and 64-bit builds.
> 
> Signed-off-by: Maxin B. John <maxin.john@gmail.com>

Which tests? I could not find any depending on CONFIG_CMD_BINOP.

HTML documentation for the command does not exist.
The online documentation leaves the user clueless:

Usage:
binop op count [*]src1 [*]src2 [[*]dest]
     - compute binary operation of data at/in src1 and
       src2 (either *memaddr, env var name or hex string)
       and store result in/at dest, where op is one of
       xor, or, and.

What is count good for?
What should a variable contain?

Can we simply eliminate the command? It is only used by a single board.

When I see comments like these in the code I am inclined to remove the 
board completely:

drivers/sysinfo/sysinfo-uclass.c:21:
      * There is some very dodgy error handling in gazerbeam,

Best regards

Heinrich

> ---
>   configs/sandbox64_defconfig | 1 +
>   configs/sandbox_defconfig   | 1 +
>   2 files changed, 2 insertions(+)
> 
> diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
> index f861d3ef06db..c4ad63ce63be 100644
> --- a/configs/sandbox64_defconfig
> +++ b/configs/sandbox64_defconfig
> @@ -43,6 +43,7 @@ CONFIG_CMD_NVEDIT_EFI=y
>   CONFIG_CMD_NVEDIT_INFO=y
>   CONFIG_CMD_NVEDIT_LOAD=y
>   CONFIG_CMD_NVEDIT_SELECT=y
> +CONFIG_CMD_BINOP=y
>   CONFIG_LOOPW=y
>   CONFIG_CMD_MD5SUM=y
>   CONFIG_CMD_MX_CYCLIC=y
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index a3fa52460827..c63a4df8befa 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -74,6 +74,7 @@ CONFIG_CMD_NVEDIT_EFI=y
>   CONFIG_CMD_NVEDIT_INFO=y
>   CONFIG_CMD_NVEDIT_LOAD=y
>   CONFIG_CMD_NVEDIT_SELECT=y
> +CONFIG_CMD_BINOP=y
>   CONFIG_LOOPW=y
>   CONFIG_CMD_MD5SUM=y
>   CONFIG_CMD_MEM_SEARCH=y