* [PATCH 0/2] test: cmd: add binop tests and enable on sandbox
@ 2026-10-29 17:34 Maxin B. John
2026-10-29 17:34 ` [PATCH 1/2] configs: sandbox: Enable binop command Maxin B. John
2026-10-29 17:34 ` [PATCH 2/2] test: cmd: add tests for the " Maxin B. John
0 siblings, 2 replies; 8+ messages in thread
From: Maxin B. John @ 2026-10-29 17:34 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] configs: sandbox: Enable binop command
2026-10-29 17:34 [PATCH 0/2] test: cmd: add binop tests and enable on sandbox Maxin B. John
@ 2026-10-29 17:34 ` Maxin B. John
2026-10-29 23:26 ` Heinrich Schuchardt
2026-10-29 17:34 ` [PATCH 2/2] test: cmd: add tests for the " Maxin B. John
1 sibling, 1 reply; 8+ messages in thread
From: Maxin B. John @ 2026-10-29 17:34 UTC (permalink / raw)
To: u-boot; +Cc: Simon Glass, Mario Six, Tom Rini, Heinrich Schuchardt,
Daniel Golle
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] test: cmd: add tests for the binop command
2026-10-29 17:34 [PATCH 0/2] test: cmd: add binop tests and enable on sandbox Maxin B. John
2026-10-29 17:34 ` [PATCH 1/2] configs: sandbox: Enable binop command Maxin B. John
@ 2026-10-29 17:34 ` Maxin B. John
1 sibling, 0 replies; 8+ messages in thread
From: Maxin B. John @ 2026-10-29 17:34 UTC (permalink / raw)
To: u-boot; +Cc: Tom Rini, Simon Glass, Dario Binacchi, Rasmus Villemoes,
Mehmet Fide
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] configs: sandbox: Enable binop command
2026-10-29 17:34 ` [PATCH 1/2] configs: sandbox: Enable binop command Maxin B. John
@ 2026-10-29 23:26 ` Heinrich Schuchardt
2026-10-30 15:58 ` Maxin John
0 siblings, 1 reply; 8+ messages in thread
From: Heinrich Schuchardt @ 2026-10-29 23:26 UTC (permalink / raw)
To: Maxin B. John; +Cc: Simon Glass, Mario Six, Tom Rini, Daniel Golle, u-boot
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] configs: sandbox: Enable binop command
2026-10-29 23:26 ` Heinrich Schuchardt
@ 2026-10-30 15:58 ` Maxin John
2026-10-30 16:39 ` Tom Rini
0 siblings, 1 reply; 8+ messages in thread
From: Maxin John @ 2026-10-30 15:58 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Simon Glass, Mario Six, Tom Rini, Daniel Golle, u-boot
Hi Heinrich,
On Fri, Oct 30, 2026 at 1:26 AM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> 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.
This change is a follow-up to my recently submitted binop clean-up patch:
https://lists.u-boot-project.org/pipermail/u-boot/2026-October/637804.html
Following the review comments on that patch, I have also submitted tests to
improve binop test coverage:
https://lists.u-boot-project.org/pipermail/u-boot/2026-October/637974.html
These tests depend on CONFIG_CMD_BINOP, which is why I enabled it in
sandbox and sandbox64.
> HTML documentation for the command does not exist.
> The online documentation leaves the user clueless:
Agreed. The documentation for binop is insufficient and could be improved.
> 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.
I am not aware of any other users of this command apart from the
gazerbeam board.
Personally, I can work around its removal if that is what the
maintainers decide.
> 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,
I will leave that decision to the maintainers.
> Best regards
>
> Heinrich
Best Regards,
Maxin
> > ---
> > 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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] configs: sandbox: Enable binop command
2026-10-30 15:58 ` Maxin John
@ 2026-10-30 16:39 ` Tom Rini
2026-10-30 18:25 ` Maxin John
0 siblings, 1 reply; 8+ messages in thread
From: Tom Rini @ 2026-10-30 16:39 UTC (permalink / raw)
To: Maxin John
Cc: Heinrich Schuchardt, Simon Glass, Mario Six, Daniel Golle, u-boot
[-- Attachment #1: Type: text/plain, Size: 1836 bytes --]
On Fri, Oct 30, 2026 at 05:58:58PM +0200, Maxin John wrote:
> Hi Heinrich,
>
> On Fri, Oct 30, 2026 at 1:26 AM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> > 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.
>
> This change is a follow-up to my recently submitted binop clean-up patch:
> https://lists.u-boot-project.org/pipermail/u-boot/2026-October/637804.html
>
> Following the review comments on that patch, I have also submitted tests to
> improve binop test coverage:
> https://lists.u-boot-project.org/pipermail/u-boot/2026-October/637974.html
>
> These tests depend on CONFIG_CMD_BINOP, which is why I enabled it in
> sandbox and sandbox64.
>
> > HTML documentation for the command does not exist.
> > The online documentation leaves the user clueless:
>
> Agreed. The documentation for binop is insufficient and could be improved.
>
> > 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.
>
> I am not aware of any other users of this command apart from the
> gazerbeam board.
> Personally, I can work around its removal if that is what the
> maintainers decide.
Perhaps what's missing here is what your motivation is for this cleanup?
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] configs: sandbox: Enable binop command
2026-10-30 16:39 ` Tom Rini
@ 2026-10-30 18:25 ` Maxin John
2026-10-30 18:34 ` Tom Rini
0 siblings, 1 reply; 8+ messages in thread
From: Maxin John @ 2026-10-30 18:25 UTC (permalink / raw)
To: Tom Rini
Cc: Heinrich Schuchardt, Simon Glass, Mario Six, Daniel Golle, u-boot
Hi Tom,
On Fri, Oct 30, 2026 at 6:39 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Oct 30, 2026 at 05:58:58PM +0200, Maxin John wrote:
> > Hi Heinrich,
> >
> > On Fri, Oct 30, 2026 at 1:26 AM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > >
> > > 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.
> >
> > This change is a follow-up to my recently submitted binop clean-up patch:
> > https://lists.u-boot-project.org/pipermail/u-boot/2026-October/637804.html
> >
> > Following the review comments on that patch, I have also submitted tests to
> > improve binop test coverage:
> > https://lists.u-boot-project.org/pipermail/u-boot/2026-October/637974.html
> >
> > These tests depend on CONFIG_CMD_BINOP, which is why I enabled it in
> > sandbox and sandbox64.
> >
> > > HTML documentation for the command does not exist.
> > > The online documentation leaves the user clueless:
> >
> > Agreed. The documentation for binop is insufficient and could be improved.
> >
> > > 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.
> >
> > I am not aware of any other users of this command apart from the
> > gazerbeam board.
> > Personally, I can work around its removal if that is what the
> > maintainers decide.
>
> Perhaps what's missing here is what your motivation is for this cleanup?
My primary motivation for the cleanup was to address existing safety issues
and make the command more robust, specifically fixing corner cases such as heap
corruption and crashes caused by over-sized input.
I originally enabled binop while experimenting with U-Boot on the Milk-V Duo.
There, I was exploring ways to manipulate binary configuration data using binop.
I agree that the command has limited usage today, and that similar functionality
can often be achieved using other U-Boot commands. I don't have a strong
preference either way and am happy to go with whatever the maintainers
feel is best.
> --
> Tom
Best Regards,
Maxin
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] configs: sandbox: Enable binop command
2026-10-30 18:25 ` Maxin John
@ 2026-10-30 18:34 ` Tom Rini
0 siblings, 0 replies; 8+ messages in thread
From: Tom Rini @ 2026-10-30 18:34 UTC (permalink / raw)
To: Maxin John
Cc: Heinrich Schuchardt, Simon Glass, Mario Six, Daniel Golle, u-boot
[-- Attachment #1: Type: text/plain, Size: 2950 bytes --]
On Fri, Oct 30, 2026 at 08:25:32PM +0200, Maxin John wrote:
> Hi Tom,
>
> On Fri, Oct 30, 2026 at 6:39 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Fri, Oct 30, 2026 at 05:58:58PM +0200, Maxin John wrote:
> > > Hi Heinrich,
> > >
> > > On Fri, Oct 30, 2026 at 1:26 AM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > > >
> > > > 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.
> > >
> > > This change is a follow-up to my recently submitted binop clean-up patch:
> > > https://lists.u-boot-project.org/pipermail/u-boot/2026-October/637804.html
> > >
> > > Following the review comments on that patch, I have also submitted tests to
> > > improve binop test coverage:
> > > https://lists.u-boot-project.org/pipermail/u-boot/2026-October/637974.html
> > >
> > > These tests depend on CONFIG_CMD_BINOP, which is why I enabled it in
> > > sandbox and sandbox64.
> > >
> > > > HTML documentation for the command does not exist.
> > > > The online documentation leaves the user clueless:
> > >
> > > Agreed. The documentation for binop is insufficient and could be improved.
> > >
> > > > 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.
> > >
> > > I am not aware of any other users of this command apart from the
> > > gazerbeam board.
> > > Personally, I can work around its removal if that is what the
> > > maintainers decide.
> >
> > Perhaps what's missing here is what your motivation is for this cleanup?
>
> My primary motivation for the cleanup was to address existing safety issues
> and make the command more robust, specifically fixing corner cases such as heap
> corruption and crashes caused by over-sized input.
>
> I originally enabled binop while experimenting with U-Boot on the Milk-V Duo.
> There, I was exploring ways to manipulate binary configuration data using binop.
>
> I agree that the command has limited usage today, and that similar functionality
> can often be achieved using other U-Boot commands. I don't have a strong
> preference either way and am happy to go with whatever the maintainers
> feel is best.
Thanks for explaining. Yes, if you don't mind, please do finish cleaning
up the command and adding a documentation page for it as well.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-10-30 18:34 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-29 17:34 [PATCH 0/2] test: cmd: add binop tests and enable on sandbox Maxin B. John
2026-10-29 17:34 ` [PATCH 1/2] configs: sandbox: Enable binop command Maxin B. John
2026-10-29 23:26 ` Heinrich Schuchardt
2026-10-30 15:58 ` Maxin John
2026-10-30 16:39 ` Tom Rini
2026-10-30 18:25 ` Maxin John
2026-10-30 18:34 ` Tom Rini
2026-10-29 17:34 ` [PATCH 2/2] test: cmd: add tests for the " Maxin B. John
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox