[RFC PATCH 00/11] Tidy command option parsing and use it a bit

[RFC PATCH 00/11] Tidy command option parsing and use it a bit

[Simon Glass]

We have had getopt() for over five years but it is not much used. It is
much easier to understand arg-parsing using getopt() and it avoids
common errors. As the named maintainer I decided to look at how to make
more use of it.

So this series explores the impact of converting a few commands to use
getopt() instead of ad-hoc parsing. It updates getopt() to handle flags
anywhere in the cmdline and provides a few helpers to reduce
boilerplate.

The chosen commands are:

- echo: very simple with no flags
- hash: fairly simple with just one flag
- env grep/export/import - fuller examples

The series also adds a recommendation to use getopt() for new commands.

To try to reduce the code-size increase, a lower-case function is added
and called from a few places. The difference is fairly marginal.

Overall, the result is not pretty (see below) with about a 1.1K size
increase on arm64:

   01: Prepare v2026.07-rc2
      aarch64:  w+   firefly-rk3399
   02: lib: string: Add strlower()
      aarch64: (for 1/1 boards) all +40.0 rodata +40.0 spl/u-boot-spl:all
         +20.0 spl/u-boot-spl:rodata +20.0 tpl/u-boot-tpl:all +20.0
         tpl/u-boot-tpl:rodata +20.0
   03: cmd: ini: Use strlower() to normalise case
   04: fs: fat: Use strlower() to normalise case
      aarch64: (for 1/1 boards) all -24.0 text -24.0
   05: boot: pxe_utils: Use strlower() in get_string()
      aarch64: (for 1/1 boards) all -56.0 text -56.0
   06: doc: commands: Recommend getopt() for option parsing
   07: lib: getopt: Permute by default with inline reorder
   08: lib: getopt: Add getopt_pop() helper
   09: cmd: echo: Use getopt() with '+' prefix for option parsing
      aarch64: (for 1/1 boards) all +1235.0 rodata +175.0 text +1060.0
   10: cmd: hash: Use getopt() for option parsing
   11: cmd: nvedit: Use getopt() in env grep
   12: cmd: nvedit: Use getopt() in env export and env import
      aarch64: (for 1/1 boards) all +176.0 rodata +12.0 text +164.0

Ideally we would use getopt() everywhere, but at least with these
examples it does seem to produce at least a small increase in each case,
even ignoring the 'hit' of about 1K for the function itself.

One option would be to make the getopt_init_state() in command.c and
pass struct getopt_state to the commands. This would reduce code size at
each size, but obviously requires a different command signature, unless
a global is used.

Another small improvement could be to include a getopt function to
return the next arg as hex.

Ideas are welcome!


Simon Glass (11):
  lib: string: Add strlower()
  cmd: ini: Use strlower() to normalise case
  fs: fat: Use strlower() to normalise case
  boot: pxe_utils: Use strlower() in get_string()
  lib: getopt: Permute by default with inline reorder
  lib: getopt: Add getopt_pop() helper
  cmd: echo: Use getopt() with '+' prefix for option parsing
  cmd: hash: Use getopt() for option parsing
  cmd: nvedit: Use getopt() in env grep
  cmd: nvedit: Use getopt() in env export and env import
  doc: commands: Recommend getopt() for option parsing

 boot/pxe_utils.c         |  12 +--
 cmd/Kconfig              |   5 +
 cmd/bdinfo.c             |   4 +-
 cmd/echo.c               |  23 ++--
 cmd/hash.c               |  37 ++++---
 cmd/ini.c                |  21 ++--
 cmd/log.c                |  12 +--
 cmd/nvedit.c             | 219 +++++++++++++++++++--------------------
 doc/develop/commands.rst |  62 +++++++++++
 fs/fat/fat.c             |  20 +---
 fs/fat/fat_write.c       |   2 +-
 include/getopt.h         | 169 +++++++++++++++++-------------
 include/linux/string.h   |  12 +++
 lib/getopt.c             |  67 +++++++++---
 lib/string.c             |  10 ++
 test/cmd/test_echo.c     |  10 ++
 test/lib/getopt.c        |   6 +-
 test/lib/string.c        |  27 +++++
 18 files changed, 441 insertions(+), 277 deletions(-)

---
base-commit: 215496fec59b3fa09256b4fb62f92af46e2ec7f9
branch: clid

-- 
2.43.0

[RFC PATCH 01/11] lib: string: Add strlower()

[Simon Glass]

Add a helper that lower-cases an ASCII string in place. It returns
the input pointer so calls can be chained:

    do_thing(strlower(name));

A new test in test/lib/string.c covers the basic cases.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/linux/string.h | 12 ++++++++++++
 lib/string.c           | 10 ++++++++++
 test/lib/string.c      | 27 +++++++++++++++++++++++++++
 3 files changed, 49 insertions(+)

diff --git a/include/linux/string.h b/include/linux/string.h
index a8a6cf4af50..15efe0ff5dd 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -49,6 +49,18 @@ int strcasecmp(const char *s1, const char *s2);
 #ifndef __HAVE_ARCH_STRNCASECMP
 extern int strncasecmp(const char *s1, const char *s2, __kernel_size_t len);
 #endif
+
+/**
+ * strlower() - Lower-case an ASCII string in place
+ * @s: The string to convert
+ *
+ * Walks @s and replaces each character with its lower-case equivalent.
+ * Returns @s so the call can be chained, e.g. ``foo(strlower(buf))``.
+ *
+ * Return: @s.
+ */
+char *strlower(char *s);
+
 #ifndef __HAVE_ARCH_STRCHR
 extern char * strchr(const char *,int);
 #endif
diff --git a/lib/string.c b/lib/string.c
index 302efe048b0..8464366dac9 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -65,6 +65,16 @@ int strcasecmp(const char *s1, const char *s2)
 	return strncasecmp(s1, s2, -1U);
 }
 
+char *strlower(char *s)
+{
+	char *p;
+
+	for (p = s; *p; p++)
+		*p = tolower(*p);
+
+	return s;
+}
+
 char * ___strtok;
 
 #ifndef __HAVE_ARCH_STRCPY
diff --git a/test/lib/string.c b/test/lib/string.c
index f56c2e4c946..598be1101a1 100644
--- a/test/lib/string.c
+++ b/test/lib/string.c
@@ -298,3 +298,30 @@ static int lib_strim(struct unit_test_state *uts)
 	return 0;
 }
 LIB_TEST(lib_strim, 0);
+
+static int lib_strlower(struct unit_test_state *uts)
+{
+	char buf[32];
+	char *p;
+
+	/* Mixed case is fully lowered, returns @s */
+	strcpy(buf, "SHA256");
+	p = strlower(buf);
+	ut_asserteq_ptr(p, buf);
+	ut_asserteq_str("sha256", buf);
+
+	/* Already lower-case is unchanged */
+	strcpy(buf, "abc");
+	ut_asserteq_str("abc", strlower(buf));
+
+	/* Non-letters and digits pass through */
+	strcpy(buf, "Hello, World! 1234");
+	ut_asserteq_str("hello, world! 1234", strlower(buf));
+
+	/* Empty string */
+	strcpy(buf, "");
+	ut_asserteq_str("", strlower(buf));
+
+	return 0;
+}
+LIB_TEST(lib_strlower, 0);
-- 
2.43.0

[RFC PATCH 02/11] cmd: ini: Use strlower() to normalise case

[Simon Glass]

ini_handler() lowercases four strings when CONFIG_INI_CASE_INSENSITIVE
is set, each with a hand-rolled loop whose strlen() in the condition
makes it quadratic in the string length.

Replace each loop with strlower() and fold the #ifdef pair into a
runtime IS_ENABLED() check so the body reads linearly.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 cmd/ini.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/cmd/ini.c b/cmd/ini.c
index 96399017691..cf0a3ce8b3d 100644
--- a/cmd/ini.c
+++ b/cmd/ini.c
@@ -202,22 +202,17 @@ static int ini_parse(char *filestart, size_t filelen,
 static int ini_handler(void *user, char *section, char *name, char *value)
 {
 	char *requested_section = (char *)user;
-#ifdef CONFIG_INI_CASE_INSENSITIVE
-	int i;
 
-	for (i = 0; i < strlen(requested_section); i++)
-		requested_section[i] = tolower(requested_section[i]);
-	for (i = 0; i < strlen(section); i++)
-		section[i] = tolower(section[i]);
-#endif
+	if (IS_ENABLED(CONFIG_INI_CASE_INSENSITIVE)) {
+		strlower(requested_section);
+		strlower(section);
+	}
 
 	if (!strcmp(section, requested_section)) {
-#ifdef CONFIG_INI_CASE_INSENSITIVE
-		for (i = 0; i < strlen(name); i++)
-			name[i] = tolower(name[i]);
-		for (i = 0; i < strlen(value); i++)
-			value[i] = tolower(value[i]);
-#endif
+		if (IS_ENABLED(CONFIG_INI_CASE_INSENSITIVE)) {
+			strlower(name);
+			strlower(value);
+		}
 		env_set(name, value);
 		printf("ini: Imported %s as %s\n", name, value);
 	}
-- 
2.43.0

[RFC PATCH 03/11] fs: fat: Use strlower() to normalise case

[Simon Glass]

Drop the local downcase() helper. The two get_name() call sites
operate on buffers that are NUL-terminated before the lowercase
pass, and the trailing FAT-name padding is spaces which tolower()
leaves untouched, so strlower() gives the same result without
needing an explicit length.

Move the 'ptr[3] = '\0'' assignment in get_name() to before the
extension is lowercased so strlower() sees a terminated string.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 fs/fat/fat.c       | 20 ++++----------------
 fs/fat/fat_write.c |  2 +-
 2 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index c1ccf30771a..dd328f948fb 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -26,23 +26,11 @@
 #include <linux/compiler.h>
 #include <linux/ctype.h>
 #include <linux/log2.h>
+#include <linux/string.h>
 
 /* maximum number of clusters for FAT12 */
 #define MAX_FAT12	0xFF4
 
-/*
- * Convert a string to lowercase.  Converts at most 'len' characters,
- * 'len' may be larger than the length of 'str' if 'str' is NULL
- * terminated.
- */
-static void downcase(char *str, size_t len)
-{
-	while (*str != '\0' && len--) {
-		*str = tolower(*str);
-		str++;
-	}
-}
-
 static struct blk_desc *cur_dev;
 static struct disk_partition cur_part_info;
 static int fat_sect_size;
@@ -270,13 +258,13 @@ static void get_name(dir_entry *dirent, char *s_name)
 	while (*ptr && *ptr != ' ')
 		ptr++;
 	if (dirent->lcase & CASE_LOWER_BASE)
-		downcase(s_name, (unsigned)(ptr - s_name));
+		strlower(s_name);
 	if (dirent->nameext.ext[0] && dirent->nameext.ext[0] != ' ') {
 		*ptr++ = '.';
 		memcpy(ptr, dirent->nameext.ext, 3);
-		if (dirent->lcase & CASE_LOWER_EXT)
-			downcase(ptr, 3);
 		ptr[3] = '\0';
+		if (dirent->lcase & CASE_LOWER_EXT)
+			strlower(ptr);
 		while (*ptr && *ptr != ' ')
 			ptr++;
 	}
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index c98b530f747..ea8d301514c 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -1464,7 +1464,7 @@ static int normalize_longname(char *l_filename, const char *filename)
 	}
 
 	strcpy(l_filename, filename);
-	downcase(l_filename, VFAT_MAXLEN_BYTES);
+	strlower(l_filename);
 
 	return 0;
 }
-- 
2.43.0

[RFC PATCH 04/11] boot: pxe_utils: Use strlower() in get_string()

[Simon Glass]

The copy loop hand-rolls a per-character branch on the 'lower' flag.
Split it into a straight memcpy() followed by an optional strlower()
so the common path is a single copy and the lowercase case is obvious.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/pxe_utils.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
index 419ab1f1b0e..098ee9bbbfe 100644
--- a/boot/pxe_utils.c
+++ b/boot/pxe_utils.c
@@ -950,7 +950,7 @@ enum lex_state {
 static char *get_string(char **p, struct token *t, char delim, int lower)
 {
 	char *b, *e;
-	size_t len, i;
+	size_t len;
 
 	/*
 	 * b and e both start at the beginning of the input stream.
@@ -976,14 +976,10 @@ static char *get_string(char **p, struct token *t, char delim, int lower)
 	if (!t->val)
 		return NULL;
 
-	for (i = 0; i < len; i++, b++) {
-		if (lower)
-			t->val[i] = tolower(*b);
-		else
-			t->val[i] = *b;
-	}
-
+	memcpy(t->val, b, len);
 	t->val[len] = '\0';
+	if (lower)
+		strlower(t->val);
 
 	/* Update *p so the caller knows where to continue scanning */
 	*p = e;
-- 
2.43.0

[RFC PATCH 05/11] lib: getopt: Permute by default with inline reorder

[Simon Glass]

Currently getopt() does not reorder its argv: it stops at the first
non-option, which forces options to appear before positional arguments.
This is not the usual way that arguments work - people expect options to
be recognised wherever they appear on the command line, although some
U-Boot commands are hard-wires for particular positions.

Restructure the parser to permute by default. Add argc and a writable
args[CONFIG_SYS_MAXARGS + 1] buffer to struct getopt_state, plus a
nonopts counter. The init function getopt_init_state() takes argc and
argv, copies argv into args, and resets the parse position. Then
getopt() and getopt_silent() take only the state and an optstring; argc
and argv are read from the state.

When getopt() encounters a non-option, it bubbles that argv element to
the end of args and increments nonopts, then keeps scanning. The outer
condition stops when index + nonopts reaches argc, so the parked
non-options are never re-scanned. After parsing finishes, gs.index
points at the first parked non-option, with gs.nonopts of them sitting
at gs.args[gs.index .. argc - 1]

Callers that need the previous POSIX-style 'stop at first non-option'
behaviour can prefix optstring with '+'. This matches GNU getopt's
convention and keeps to two public entry points (getopt() and
getopt_silent()) instead of doubling them up for in-order variants.

Update the two existing in-tree callers, cmd/bdinfo.c and cmd/log.c, to
the new signatures. bdinfo has no positional arguments so it works the
same either way; log filter-list/add/remove preserve their original
semantics with the '+' prefix. Update test helper in test/lib/getopt.c
too.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 cmd/bdinfo.c      |   4 +-
 cmd/log.c         |  12 ++--
 include/getopt.h  | 146 +++++++++++++++++++++++-----------------------
 lib/getopt.c      |  67 ++++++++++++++++-----
 test/lib/getopt.c |   6 +-
 5 files changed, 135 insertions(+), 100 deletions(-)

diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c
index ddf77303735..c46094c3ddf 100644
--- a/cmd/bdinfo.c
+++ b/cmd/bdinfo.c
@@ -188,8 +188,8 @@ int do_bdinfo(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 	if (!CONFIG_IS_ENABLED(GETOPT) || argc == 1)
 		return bdinfo_print_all(bd);
 
-	getopt_init_state(&gs);
-	while ((opt = getopt(&gs, argc, argv, "aem")) > 0) {
+	getopt_init_state(&gs, argc, argv);
+	while ((opt = getopt(&gs, "aem")) > 0) {
 		switch (opt) {
 		case 'a':
 			return bdinfo_print_all(bd);
diff --git a/cmd/log.c b/cmd/log.c
index 64add6d8b5a..344f379d8cc 100644
--- a/cmd/log.c
+++ b/cmd/log.c
@@ -95,8 +95,8 @@ static int do_log_filter_list(struct cmd_tbl *cmdtp, int flag, int argc,
 	struct log_filter *filt;
 	struct log_device *ldev;
 
-	getopt_init_state(&gs);
-	while ((opt = getopt(&gs, argc, argv, "d:")) > 0) {
+	getopt_init_state(&gs, argc, argv);
+	while ((opt = getopt(&gs, "+d:")) > 0) {
 		switch (opt) {
 		case 'd':
 			drv_name = gs.arg;
@@ -157,8 +157,8 @@ static int do_log_filter_add(struct cmd_tbl *cmdtp, int flag, int argc,
 	enum log_level_t level = LOGL_MAX;
 	struct getopt_state gs;
 
-	getopt_init_state(&gs);
-	while ((opt = getopt(&gs, argc, argv, "Ac:d:Df:F:l:L:p")) > 0) {
+	getopt_init_state(&gs, argc, argv);
+	while ((opt = getopt(&gs, "+Ac:d:Df:F:l:L:p")) > 0) {
 		switch (opt) {
 		case 'A':
 #define do_type() do { \
@@ -250,8 +250,8 @@ static int do_log_filter_remove(struct cmd_tbl *cmdtp, int flag, int argc,
 	const char *drv_name = "console";
 	struct getopt_state gs;
 
-	getopt_init_state(&gs);
-	while ((opt = getopt(&gs, argc, argv, "ad:")) > 0) {
+	getopt_init_state(&gs, argc, argv);
+	while ((opt = getopt(&gs, "+ad:")) > 0) {
 		switch (opt) {
 		case 'a':
 			all = true;
diff --git a/include/getopt.h b/include/getopt.h
index 0cf7ee84d6f..5a9e7802e65 100644
--- a/include/getopt.h
+++ b/include/getopt.h
@@ -10,120 +10,120 @@
 #define __GETOPT_H
 
 #include <stdbool.h>
+#include <linux/kconfig.h>
 
 /**
  * struct getopt_state - Saved state across getopt() calls
+ *
+ * Initialise with getopt_init_state(); the embedded @args buffer holds a
+ * working copy of the argv passed to that initialiser.
  */
 struct getopt_state {
+	/** @argc: Argument count, as passed to getopt_init_state() */
+	int argc;
+	/**
+	 * @args: Working copy of argv. getopt() reorders this in place: as
+	 * options are consumed, non-options are bubbled to the end.
+	 */
+	char *args[CONFIG_SYS_MAXARGS + 1];
 	/**
-	 * @index: Index of the next unparsed argument of @argv. If getopt() has
-	 * parsed all of @argv, then @index will equal @argc.
+	 * @index: Index of the next unparsed argument of @args. When
+	 * parsing has finished, @index sits at the first parked
+	 * non-option (or @argc if none).
 	 */
 	int index;
-	/** @arg_index: Index within the current argument */
+	/**
+	 * @arg_index: Offset of the next unparsed character within
+	 * ``args[index]``. Lets the parser step through grouped short
+	 * options like ``-abc`` (1, 2, 3...). Reset to 1 each time
+	 * @index advances to the next argv slot.
+	 */
 	int arg_index;
+	/**
+	 * @nonopts: Number of non-option arguments parked at the tail
+	 * of @args. The parser bubbles them down so it can keep scanning
+	 * for options past them.
+	 */
+	int nonopts;
 	union {
 		/**
-		 * @opt: Option being parsed when an error occurs. @opt is only
-		 * valid when getopt() returns ``?`` or ``:``.
+		 * @opt: Option being parsed when an error occurs. @opt is
+		 * only valid when getopt() returns ``?`` or ``:``.
 		 */
 		int opt;
 		/**
-		 * @arg: The argument to an option, NULL if there is none. @arg
-		 * is only valid when getopt() returns an option character.
+		 * @arg: The argument to an option, NULL if there is none.
+		 * @arg is only valid when getopt() returns an option
+		 * character.
 		 */
 		char *arg;
 	};
 };
 
 /**
- * getopt_init_state() - Initialize a &struct getopt_state
- * @gs: The state to initialize
- *
- * This must be called before using @gs with getopt().
+ * getopt_init_state() - Initialise a &struct getopt_state from an argv
+ * @gs: The state to initialise
+ * @argc: Argument count
+ * @argv: Source argv. Copied into @gs->args; the original is not
+ *        modified. @argc must not exceed ``CONFIG_SYS_MAXARGS``;
+ *        excess arguments are silently dropped.
  */
-void getopt_init_state(struct getopt_state *gs);
+void getopt_init_state(struct getopt_state *gs, int argc,
+		       char *const argv[]);
 
-int __getopt(struct getopt_state *gs, int argc, char *const argv[],
-	     const char *optstring, bool silent);
+int __getopt(struct getopt_state *gs, const char *optstring, bool silent);
 
 /**
  * getopt() - Parse short command-line options
- * @gs: Internal state and out-of-band return arguments. This must be
- *      initialized with getopt_init_context() beforehand.
- * @argc: Number of arguments, not including the %NULL terminator
- * @argv: Argument list, terminated by %NULL
- * @optstring: Option specification, as described below
- *
- * getopt() parses short options. Short options are single characters. They may
- * be followed by a required argument or an optional argument. Arguments to
- * options may occur in the same argument as an option (like ``-larg``), or
- * in the following argument (like ``-l arg``). An argument containing
- * options begins with a ``-``. If an option expects no arguments, then it may
- * be immediately followed by another option (like ``ls -alR``).
- *
- * @optstring is a list of accepted options. If an option is followed by ``:``
- * in @optstring, then it expects a mandatory argument. If an option is followed
- * by ``::`` in @optstring, it expects an optional argument. @gs.arg points
- * to the argument, if one is parsed.
- *
- * getopt() stops parsing options when it encounters the first non-option
- * argument, when it encounters the argument ``--``, or when it runs out of
- * arguments. For example, in ``ls -l foo -R``, option parsing will stop when
- * getopt() encounters ``foo``, if ``l`` does not expect an argument. However,
- * the whole list of arguments would be parsed if ``l`` expects an argument.
+ * @gs: State, initialised with getopt_init_state()
+ * @optstring: Option specification (see getopt(3))
  *
- * An example invocation of getopt() might look like::
+ * Parses the next short option from @gs. By default, getopt() permutes
+ * the working argv: when it hits a non-option, it bubbles that argv
+ * element to the end and keeps scanning, so options can appear
+ * anywhere on the command line. After parsing finishes (returns -1),
+ * @gs.index sits at the first parked non-option, and there are
+ * @gs.nonopts of them at @gs.args[gs.index..argc-1].
  *
- *     char *argv[] = { "program", "-cbx", "-a", "foo", "bar", 0 };
- *     int opt, argc = ARRAY_SIZE(argv) - 1;
- *     struct getopt_state gs;
+ * If @optstring begins with ``+``, getopt() instead stops at the
+ * first non-option (POSIX ``getopt`` behaviour). Use this when the
+ * command's grammar requires options before positionals, or when the
+ * option string includes optional arguments (``::``) whose adjacency
+ * to a following positional would be ambiguous under permutation.
  *
- *     getopt_init_state(&gs);
- *     while ((opt = getopt(&gs, argc, argv, "a::b:c")) != -1)
- *         printf("opt = %c, index = %d, arg = \"%s\"\n", opt, gs.index, gs.arg);
- *     printf("%d argument(s) left\n", argc - gs.index);
+ * In @optstring proper, ``x`` is a flag, ``x:`` requires an argument,
+ * and ``x::`` takes an optional argument. The argument is delivered
+ * in @gs.arg.
  *
- * and would produce an output of::
- *
- *     opt = c, index = 1, arg = "<NULL>"
- *     opt = b, index = 2, arg = "x"
- *     opt = a, index = 4, arg = "foo"
- *     1 argument(s) left
- *
- * For further information, refer to the getopt(3) man page.
+ * A literal ``--`` argument terminates option scanning; the parser
+ * advances past it and returns -1.
  *
  * Return:
- * * An option character if an option is found. @gs.arg is set to the
- *   argument if there is one, otherwise it is set to ``NULL``.
- * * ``-1`` if there are no more options, if a non-option argument is
- *   encountered, or if an ``--`` argument is encountered.
- * * ``'?'`` if we encounter an option not in @optstring. @gs.opt is set to
- *   the unknown option.
- * * ``':'`` if an argument is required, but no argument follows the
- *   option. @gs.opt is set to the option missing its argument.
- *
- * @gs.index is always set to the index of the next unparsed argument in @argv.
+ * * An option character if an option is found. @gs.arg is set to
+ *   the argument if there is one, otherwise NULL.
+ * * ``-1`` if there are no more options.
+ * * ``'?'`` for an option not in @optstring; @gs.opt is the unknown
+ *   option character.
+ * * ``':'`` for an option missing its required argument; @gs.opt is
+ *   the option character.
  */
-static inline int getopt(struct getopt_state *gs, int argc,
-			 char *const argv[], const char *optstring)
+static inline int getopt(struct getopt_state *gs, const char *optstring)
 {
-	return __getopt(gs, argc, argv, optstring, false);
+	return __getopt(gs, optstring, false);
 }
 
 /**
- * getopt_silent() - Parse short command-line options silently
+ * getopt_silent() - Parse short command-line options without errors
  * @gs: State
- * @argc: Argument count
- * @argv: Argument list
  * @optstring: Option specification
  *
- * Same as getopt(), except no error messages are printed.
+ * Same as getopt() but does not print error messages for unknown
+ * options or missing arguments.
  */
-static inline int getopt_silent(struct getopt_state *gs, int argc,
-				char *const argv[], const char *optstring)
+static inline int getopt_silent(struct getopt_state *gs,
+				const char *optstring)
 {
-	return __getopt(gs, argc, argv, optstring, true);
+	return __getopt(gs, optstring, true);
 }
 
 #endif /* __GETOPT_H */
diff --git a/lib/getopt.c b/lib/getopt.c
index e9175e2fff4..70adb2d0faf 100644
--- a/lib/getopt.c
+++ b/lib/getopt.c
@@ -10,37 +10,71 @@
 
 #include <getopt.h>
 #include <log.h>
+#include <linux/kernel.h>
 #include <linux/string.h>
 
-void getopt_init_state(struct getopt_state *gs)
+void getopt_init_state(struct getopt_state *gs, int argc, char *const argv[])
 {
+	int max = ARRAY_SIZE(gs->args) - 1;
+
+	if (argc > max)
+		argc = max;
+
+	gs->argc = argc;
+	memcpy(gs->args, argv, (argc + 1) * sizeof(*gs->args));
+	gs->args[argc] = NULL;
 	gs->index = 1;
 	gs->arg_index = 1;
+	gs->nonopts = 0;
 }
 
-int __getopt(struct getopt_state *gs, int argc, char *const argv[],
-	     const char *optstring, bool silent)
+int __getopt(struct getopt_state *gs, const char *optstring, bool silent)
 {
-	char curopt;   /* current option character */
-	const char *curoptp; /* pointer to the current option in optstring */
+	char curopt;	/* current option character */
+	const char *curoptp;	/* pointer to the current option in optstring */
+	bool stop_nonopt = false;
+	char **argv = gs->args;
+	int argc = gs->argc;
+
+	if (*optstring == '+') {
+		stop_nonopt = true;
+		optstring++;
+	}
 
 	while (1) {
-		log_debug("arg_index: %d index: %d\n", gs->arg_index,
-			  gs->index);
+		log_debug("arg_index: %d index: %d nonopts: %d\n",
+			  gs->arg_index, gs->index, gs->nonopts);
 
 		/* `--` indicates the end of options */
-		if (gs->arg_index == 1 && argv[gs->index] &&
+		if (gs->arg_index == 1 && gs->index < argc &&
 		    !strcmp(argv[gs->index], "--")) {
 			gs->index++;
 			return -1;
 		}
 
-		/* Out of arguments */
-		if (gs->index >= argc)
-			return -1;
+		/*
+		 * Bubble non-options to the end so we can keep scanning for
+		 * options past them. In '+' mode (POSIX), stop at the first
+		 * non-option instead.
+		 */
+		while (gs->arg_index == 1 &&
+		       gs->index + gs->nonopts < argc &&
+		       *argv[gs->index] != '-') {
+			char *tmp;
+			int i;
+
+			if (stop_nonopt)
+				return -1;
+
+			tmp = argv[gs->index];
+			gs->nonopts++;
+			for (i = gs->index; i + 1 < argc; i++)
+				argv[i] = argv[i + 1];
+			argv[argc - 1] = tmp;
+		}
 
-		/* Can't parse non-options */
-		if (*argv[gs->index] != '-')
+		/* Out of options to scan */
+		if (gs->index + gs->nonopts >= argc)
 			return -1;
 
 		/* We have found an option */
@@ -48,7 +82,8 @@ int __getopt(struct getopt_state *gs, int argc, char *const argv[],
 		if (curopt)
 			break;
 		/*
-		 * no more options in current argv[] element; try the next one
+		 * No more options in current argv[] element; advance to the
+		 * next one
 		 */
 		gs->index++;
 		gs->arg_index = 1;
@@ -80,7 +115,7 @@ int __getopt(struct getopt_state *gs, int argc, char *const argv[],
 			gs->arg_index = 1;
 			return curopt;
 		}
-		if (gs->index + 1 == argc) {
+		if (gs->index + gs->nonopts + 1 == argc) {
 			/* We are at the last argv[] element */
 			gs->arg = NULL;
 			gs->index++;
@@ -113,7 +148,7 @@ int __getopt(struct getopt_state *gs, int argc, char *const argv[],
 	gs->index++;
 	gs->arg_index = 1;
 
-	if (gs->index >= argc || argv[gs->index][0] == '-') {
+	if (gs->index + gs->nonopts >= argc || argv[gs->index][0] == '-') {
 		if (!silent)
 			printf("option requires an argument -- %c\n", curopt);
 		gs->opt = curopt;
diff --git a/test/lib/getopt.c b/test/lib/getopt.c
index 388a076200b..6b384eb016a 100644
--- a/test/lib/getopt.c
+++ b/test/lib/getopt.c
@@ -18,9 +18,9 @@ static int do_test_getopt(struct unit_test_state *uts, int line,
 {
 	int opt;
 
-	getopt_init_state(gs);
+	getopt_init_state(gs, args, argv);
 	for (int i = 0; i < expected_count; i++) {
-		opt = getopt_silent(gs, args, argv, optstring);
+		opt = getopt_silent(gs, optstring);
 		if (expected[i] != opt) {
 			/*
 			 * Fudge the line number so we can tell which test
@@ -34,7 +34,7 @@ static int do_test_getopt(struct unit_test_state *uts, int line,
 		}
 	}
 
-	opt = getopt_silent(gs, args, argv, optstring);
+	opt = getopt_silent(gs, optstring);
 	if (opt != -1) {
 		ut_failf(uts, __FILE__, line, __func__,
 			 "getopt() != -1",
-- 
2.43.0

[RFC PATCH 06/11] lib: getopt: Add getopt_pop() helper

[Simon Glass]

Callers that consume positional arguments after parsing routinely write
the same shape:

    algo = gs.args[gs.index];
    /* advance past algo */
    return hash_command(algo, ..., gs.nonopts - 1, &gs.args[gs.index + 1]);

Wrap that in a small inline helper that returns the next positional,
advances gs.index, decrements gs.nonopts, and returns NULL when no
positionals remain. The same helper doubles as an iterator:

    while ((arg = getopt_pop(&gs)))
        process(arg);

stddef.h is now included from getopt.h for the NULL macro.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/getopt.h | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/include/getopt.h b/include/getopt.h
index 5a9e7802e65..013431807ff 100644
--- a/include/getopt.h
+++ b/include/getopt.h
@@ -10,6 +10,7 @@
 #define __GETOPT_H
 
 #include <stdbool.h>
+#include <stddef.h>
 #include <linux/kconfig.h>
 
 /**
@@ -126,4 +127,26 @@ static inline int getopt_silent(struct getopt_state *gs,
 	return __getopt(gs, optstring, true);
 }
 
+/**
+ * getopt_pop() - Take the next remaining positional argument
+ * @gs: State, after getopt() has returned -1
+ *
+ * Returns the first parked non-option (``gs->args[gs->index]``),
+ * advances the index, and decrements ``gs->nonopts``. Returns NULL
+ * when no positional arguments remain.
+ *
+ * Useful for consuming positionals one at a time after parsing::
+ *
+ *     algo = getopt_pop(&gs);
+ *     return hash_command(algo, ..., gs.nonopts, &gs.args[gs.index]);
+ */
+static inline char *getopt_pop(struct getopt_state *gs)
+{
+	if (gs->index >= gs->argc)
+		return NULL;
+	if (gs->nonopts > 0)
+		gs->nonopts--;
+	return gs->args[gs->index++];
+}
+
 #endif /* __GETOPT_H */
-- 
2.43.0

[RFC PATCH 07/11] cmd: echo: Use getopt() with '+' prefix for option parsing

[Simon Glass]

The 'echo' command's option parser is a single strcmp against argv[1]
that decides whether to suppress the trailing newline. Convert it to
getopt() so echo follows the same shape as the other commands in this
series and exercises the '+' prefix that POSIX-style 'stop at first
non-option' callers need.

The optstring uses the '+' prefix to preserve bash echo behaviour: -n is
honoured only as the very first argument, so 'echo hello -n' still
prints 'hello -n\n' verbatim.

Two minor differences from the bash builtin remain, both of which
the user can work around with quoting or --:

* -x (an unknown short option) returns CMD_RET_USAGE rather than
  printing literally; use 'echo -- -x' to print it.
* -nfoo (joined form) parses -n and then errors on the trailing
  characters.

Add three test cases that pin down the new behaviour: trailing -n stays
literal, -- ends option parsing, and -- is consumed even after a
recognised flag. The positional loop uses getopt_pop() as an iterator.
CMD_ECHO selects GETOPT so the parser is linked in on boards that don't
already enable it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 cmd/Kconfig          |  1 +
 cmd/echo.c           | 23 ++++++++++++++---------
 test/cmd/test_echo.c | 10 ++++++++++
 3 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index c71c6824a19..709696c3c41 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1916,6 +1916,7 @@ config CMD_CAT
 config CMD_ECHO
 	bool "echo"
 	default y
+	select GETOPT
 	help
 	  Echo args to console
 
diff --git a/cmd/echo.c b/cmd/echo.c
index d1346504cfb..63422c75cc6 100644
--- a/cmd/echo.c
+++ b/cmd/echo.c
@@ -5,27 +5,32 @@
  */
 
 #include <command.h>
-#include <linux/string.h>
+#include <getopt.h>
 
 static int do_echo(struct cmd_tbl *cmdtp, int flag, int argc,
 		   char *const argv[])
 {
-	int i = 1;
+	struct getopt_state gs;
 	bool space = false;
 	bool newline = true;
+	char *arg;
+	int opt;
 
-	if (argc > 1) {
-		if (!strcmp(argv[1], "-n")) {
+	getopt_init_state(&gs, argc, argv);
+	while ((opt = getopt(&gs, "+n")) > 0) {
+		switch (opt) {
+		case 'n':
 			newline = false;
-			++i;
+			break;
+		default:
+			return CMD_RET_USAGE;
 		}
 	}
 
-	for (; i < argc; ++i) {
-		if (space) {
+	while ((arg = getopt_pop(&gs))) {
+		if (space)
 			putc(' ');
-		}
-		puts(argv[i]);
+		puts(arg);
 		space = true;
 	}
 
diff --git a/test/cmd/test_echo.c b/test/cmd/test_echo.c
index 7ed534742f7..5fc139fbe68 100644
--- a/test/cmd/test_echo.c
+++ b/test/cmd/test_echo.c
@@ -35,6 +35,16 @@ static struct test_data echo_data[] = {
 	/* Test handling of shell variables. */
 	{"setenv jQx; for jQx in 1 2 3; do echo -n \"${jQx}, \"; done; echo;",
 	 "1, 2, 3, "},
+	/* -n only suppresses the newline when it comes before any
+	 * positional argument; a trailing -n is just another argument.
+	 */
+	{"echo hello -n", "hello -n"},
+	/* "--" ends option parsing, so a following dash-prefixed token
+	 * is printed verbatim instead of being rejected.
+	 */
+	{"echo -- -x", "-x"},
+	/* "--" is consumed even when it follows a recognised flag. */
+	{"echo -n -- foo; echo done", "foodone"},
 };
 
 static int lib_test_hush_echo(struct unit_test_state *uts)
-- 
2.43.0

[RFC PATCH 08/11] cmd: hash: Use getopt() for option parsing

[Simon Glass]

The open-coded check for -v only matches it as the first argument, so
'hash sha256 -v 0 4 <expected>' treats -v as the algorithm name and
fails. Replace with getopt(), which also lets -v appear anywhere in the
command line.

Guard the option-parsing loop with IS_ENABLED(CONFIG_HASH_VERIFY) so it
is compiled out when verification is disabled. After parsing,
getopt_pop() pulls the algorithm off the positional list, strlower()
normalises it, and the tail is forwarded to hash_command() unchanged.

CMD_HASH selects GETOPT so the parser is linked in on boards that did
not already enable it.

Tweak the var declarations to use Reverse Christmas Tree.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 cmd/Kconfig |  1 +
 cmd/hash.c  | 37 +++++++++++++++++++++----------------
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 709696c3c41..eb7c85c1fe9 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2798,6 +2798,7 @@ config CMD_BLOB
 
 config CMD_HASH
 	bool "Support 'hash' command"
+	select GETOPT
 	select HASH
 	help
 	  This provides a way to hash data in memory using various supported
diff --git a/cmd/hash.c b/cmd/hash.c
index 96d0e443a5b..566bb617328 100644
--- a/cmd/hash.c
+++ b/cmd/hash.c
@@ -11,8 +11,9 @@
 
 #include <command.h>
 #include <env.h>
+#include <getopt.h>
 #include <hash.h>
-#include <linux/ctype.h>
+#include <linux/string.h>
 
 #if IS_ENABLED(CONFIG_HASH_VERIFY)
 #define HARGS 6
@@ -23,25 +24,29 @@
 static int do_hash(struct cmd_tbl *cmdtp, int flag, int argc,
 		   char *const argv[])
 {
-	char *s;
 	int flags = HASH_FLAG_ENV;
+	struct getopt_state gs;
+	char *algo;
 
-	if (argc < 4)
-		return CMD_RET_USAGE;
+	getopt_init_state(&gs, argc, argv);
+	if (IS_ENABLED(CONFIG_HASH_VERIFY)) {
+		int opt;
 
-#if IS_ENABLED(CONFIG_HASH_VERIFY)
-	if (!strcmp(argv[1], "-v")) {
-		flags |= HASH_FLAG_VERIFY;
-		argc--;
-		argv++;
+		while ((opt = getopt(&gs, "v")) > 0) {
+			if (opt != 'v')
+				return CMD_RET_USAGE;
+			flags |= HASH_FLAG_VERIFY;
+		}
 	}
-#endif
-	/* Move forward to 'algorithm' parameter */
-	argc--;
-	argv++;
-	for (s = *argv; *s; s++)
-		*s = tolower(*s);
-	return hash_command(*argv, flags, cmdtp, flag, argc - 1, argv + 1);
+
+	/* Need at least: algorithm address count */
+	if (gs.nonopts < 3)
+		return CMD_RET_USAGE;
+
+	algo = strlower(getopt_pop(&gs));
+
+	return hash_command(algo, flags, cmdtp, flag,
+			    gs.nonopts, &gs.args[gs.index]);
 }
 
 U_BOOT_CMD(
-- 
2.43.0

[RFC PATCH 09/11] cmd: nvedit: Use getopt() in env grep

[Simon Glass]

The nested while/switch parser walks each -prefixed argv element by
hand. The -- end-of-options marker is implemented by a case '-' arm
that does 'goto DONE' to jump out of both the inner switch and the
outer argv walker, leaving the remaining argv to be treated as
positionals by hexport_r().

getopt() handles all of that natively: grouped flags (-nb), the '--'
boundary (it returns -1 and advances past --), unknown-option errors
and option permutation so 'env grep arch -n' works the same as
'env grep -n arch'. The goto, the label, and the case '-' arm all
disappear.

Pass "envb" or "nvb" to getopt() depending on IS_ENABLED(CONFIG_REGEX)
so -e is not advertised when regex is compiled out, and guard the
case 'e' arm with the same check so it costs nothing in the regex-off
build.

Move the missing-argument check to after parsing. The existing argc < 2
check fires only when no args at all are given; the new gs.nonopts < 1
check also catches 'env grep -n' (options but no search pattern), where
the previous version would silently let hexport_r() run with an empty
pattern list.

CMD_GREPENV selects GETOPT so the parser is linked in on boards that did
not already enable it.

Tweak the var declarations to use Reverse Christmas Tree.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 cmd/Kconfig  |  1 +
 cmd/nvedit.c | 56 +++++++++++++++++++++++++---------------------------
 2 files changed, 28 insertions(+), 29 deletions(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index eb7c85c1fe9..8ba7b1e6bd1 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -721,6 +721,7 @@ config CMD_EDITENV
 
 config CMD_GREPENV
 	bool "search env"
+	select GETOPT
 	help
 	  Allow for searching environment variables
 
diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index 636bddee1be..8561c408992 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -29,6 +29,7 @@
 #include <console.h>
 #include <env.h>
 #include <env_internal.h>
+#include <getopt.h>
 #include <log.h>
 #include <search.h>
 #include <errno.h>
@@ -131,45 +132,42 @@ static int do_env_print(struct cmd_tbl *cmdtp, int flag, int argc,
 static int do_env_grep(struct cmd_tbl *cmdtp, int flag,
 		       int argc, char *const argv[])
 {
-	char *res = NULL;
 	int len, grep_how, grep_what;
-
-	if (argc < 2)
-		return CMD_RET_USAGE;
+	struct getopt_state gs;
+	char *res = NULL;
+	int opt;
 
 	grep_how  = H_MATCH_SUBSTR;	/* default: substring search	*/
 	grep_what = H_MATCH_BOTH;	/* default: grep names and values */
 
-	while (--argc > 0 && **++argv == '-') {
-		char *arg = *argv;
-		while (*++arg) {
-			switch (*arg) {
-#ifdef CONFIG_REGEX
-			case 'e':		/* use regex matching */
-				grep_how  = H_MATCH_REGEX;
-				break;
-#endif
-			case 'n':		/* grep for name */
-				grep_what = H_MATCH_KEY;
-				break;
-			case 'v':		/* grep for value */
-				grep_what = H_MATCH_DATA;
-				break;
-			case 'b':		/* grep for both */
-				grep_what = H_MATCH_BOTH;
-				break;
-			case '-':
-				goto DONE;
-			default:
-				return CMD_RET_USAGE;
-			}
+	getopt_init_state(&gs, argc, argv);
+	while ((opt = getopt(&gs, IS_ENABLED(CONFIG_REGEX) ? "envb"
+							   : "nvb")) > 0) {
+		switch (opt) {
+		case 'e':		/* use regex matching */
+			if (IS_ENABLED(CONFIG_REGEX))
+				grep_how = H_MATCH_REGEX;
+			break;
+		case 'n':		/* grep for name */
+			grep_what = H_MATCH_KEY;
+			break;
+		case 'v':		/* grep for value */
+			grep_what = H_MATCH_DATA;
+			break;
+		case 'b':		/* grep for both */
+			grep_what = H_MATCH_BOTH;
+			break;
+		default:
+			return CMD_RET_USAGE;
 		}
 	}
 
-DONE:
+	if (gs.nonopts < 1)
+		return CMD_RET_USAGE;
+
 	len = hexport_r(&env_htab, '\n',
 			flag | grep_what | grep_how,
-			&res, 0, argc, argv);
+			&res, 0, gs.nonopts, &gs.args[gs.index]);
 
 	if (len > 0) {
 		puts(res);
-- 
2.43.0

[RFC PATCH 10/11] cmd: nvedit: Use getopt() in env export and env import

[Simon Glass]

These are the gnarliest hand-rolled parsers in nvedit.c

Each one walks each -prefixed argv element by hand, opens an inner
while (*++arg) to split grouped flags, and tracks a fmt counter to catch
a second -b / -c / -t

env export adds a 'goto NXTARG' that jumps over the inner loop after
consuming the argument to -s SIZE, since the rest of the current argv
element no longer makes sense once -s has eaten the next slot.

getopt() collapses all of that. The mutex check for the format flags
stays in the case arms (it is a per-command rule, not an option-parsing
rule), the -s argument arrives in gs.arg, and the trailing positional
list is read straight out of gs.args + gs.index with gs.nonopts as the
count.

Permutation now lets, for example, 'env export 1000000 -t -s 1000' work
the same way as the conventional 'env export -t -s 1000 1000000'.

Tests cover single flags, grouped flags, the mutex-violation path (both
-bt and -t -b forms), -s SIZE in joined and separated form, a
setenv/export/clear/import roundtrip with -d, and an out-of-order
'import 0x... -t -d' invocation.

Adjust CMD_EXPORTENV and CMD_IMPORTENV to select GETOPT so the parser is
linked in on boards that did not already enable it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 cmd/Kconfig  |   2 +
 cmd/nvedit.c | 159 ++++++++++++++++++++++++---------------------------
 2 files changed, 78 insertions(+), 83 deletions(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 8ba7b1e6bd1..e9bf58b39a6 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -704,12 +704,14 @@ config CMD_ASKENV
 config CMD_EXPORTENV
 	bool "env export"
 	default y
+	select GETOPT
 	help
 	  Export environments.
 
 config CMD_IMPORTENV
 	bool "env import"
 	default y
+	select GETOPT
 	help
 	  Import environments.
 
diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index 8561c408992..6af5f52907e 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -632,66 +632,58 @@ static int do_env_delete(struct cmd_tbl *cmdtp, int flag,
 static int do_env_export(struct cmd_tbl *cmdtp, int flag,
 			 int argc, char *const argv[])
 {
+	struct getopt_state gs;
 	char	buf[32];
 	ulong	addr;
-	char	*ptr, *cmd, *res;
+	char	*ptr, *res;
 	size_t	size = 0;
 	ssize_t	len;
 	env_t	*envp;
 	char	sep = '\n';
 	int	chk = 0;
 	int	fmt = 0;
+	int	opt;
 
-	cmd = *argv;
-
-	while (--argc > 0 && **++argv == '-') {
-		char *arg = *argv;
-		while (*++arg) {
-			switch (*arg) {
-			case 'b':		/* raw binary format */
-				if (fmt++)
-					goto sep_err;
-				sep = '\0';
-				break;
-			case 'c':		/* external checksum format */
-				if (fmt++)
-					goto sep_err;
-				sep = '\0';
-				chk = 1;
-				break;
-			case 's':		/* size given */
-				if (--argc <= 0)
-					return cmd_usage(cmdtp);
-				size = hextoul(*++argv, NULL);
-				goto NXTARG;
-			case 't':		/* text format */
-				if (fmt++)
-					goto sep_err;
-				sep = '\n';
-				break;
-			default:
-				return CMD_RET_USAGE;
-			}
+	getopt_init_state(&gs, argc, argv);
+	while ((opt = getopt(&gs, "bcs:t")) > 0) {
+		switch (opt) {
+		case 'b':		/* raw binary format */
+			if (fmt++)
+				goto sep_err;
+			sep = '\0';
+			break;
+		case 'c':		/* external checksum format */
+			if (fmt++)
+				goto sep_err;
+			sep = '\0';
+			chk = 1;
+			break;
+		case 's':		/* size given */
+			size = hextoul(gs.arg, NULL);
+			break;
+		case 't':		/* text format */
+			if (fmt++)
+				goto sep_err;
+			sep = '\n';
+			break;
+		default:
+			return CMD_RET_USAGE;
 		}
-NXTARG:		;
 	}
 
-	if (argc < 1)
+	if (gs.nonopts < 1)
 		return CMD_RET_USAGE;
 
-	addr = hextoul(argv[0], NULL);
+	addr = hextoul(getopt_pop(&gs), NULL);
 	ptr = map_sysmem(addr, size);
 
 	if (size)
 		memset(ptr, '\0', size);
 
-	argc--;
-	argv++;
-
 	if (sep) {		/* export as text file */
 		len = hexport_r(&env_htab, sep,
-				H_MATCH_KEY | H_MATCH_IDENT,
-				&ptr, size, argc, argv);
+				H_MATCH_KEY | H_MATCH_IDENT, &ptr, size,
+				gs.nonopts, &gs.args[gs.index]);
 		if (len < 0) {
 			pr_err("## Error: Cannot export environment: errno = %d\n",
 			       errno);
@@ -711,8 +703,8 @@ NXTARG:		;
 		res = ptr;
 
 	len = hexport_r(&env_htab, '\0',
-			H_MATCH_KEY | H_MATCH_IDENT,
-			&res, ENV_SIZE, argc, argv);
+			H_MATCH_KEY | H_MATCH_IDENT, &res, ENV_SIZE,
+			gs.nonopts, &gs.args[gs.index]);
 	if (len < 0) {
 		pr_err("## Error: Cannot export environment: errno = %d\n",
 		       errno);
@@ -732,7 +724,7 @@ NXTARG:		;
 
 sep_err:
 	printf("## Error: %s: only one of \"-b\", \"-c\" or \"-t\" allowed\n",
-	       cmd);
+	       argv[0]);
 	return 1;
 }
 #endif
@@ -765,8 +757,9 @@ sep_err:
 static int do_env_import(struct cmd_tbl *cmdtp, int flag,
 			 int argc, char *const argv[])
 {
+	struct getopt_state gs;
 	ulong	addr;
-	char	*cmd, *ptr;
+	char	*ptr, *tok;
 	char	sep = '\n';
 	int	chk = 0;
 	int	fmt = 0;
@@ -774,55 +767,53 @@ static int do_env_import(struct cmd_tbl *cmdtp, int flag,
 	int	crlf_is_lf = 0;
 	int	wl = 0;
 	size_t	size;
+	int	opt;
 
-	cmd = *argv;
-
-	while (--argc > 0 && **++argv == '-') {
-		char *arg = *argv;
-		while (*++arg) {
-			switch (*arg) {
-			case 'b':		/* raw binary format */
-				if (fmt++)
-					goto sep_err;
-				sep = '\0';
-				break;
-			case 'c':		/* external checksum format */
-				if (fmt++)
-					goto sep_err;
-				sep = '\0';
-				chk = 1;
-				break;
-			case 't':		/* text format */
-				if (fmt++)
-					goto sep_err;
-				sep = '\n';
-				break;
-			case 'r':		/* handle CRLF like LF */
-				crlf_is_lf = 1;
-				break;
-			case 'd':
-				del = 1;
-				break;
-			default:
-				return CMD_RET_USAGE;
-			}
+	getopt_init_state(&gs, argc, argv);
+	while ((opt = getopt(&gs, "bctrd")) > 0) {
+		switch (opt) {
+		case 'b':		/* raw binary format */
+			if (fmt++)
+				goto sep_err;
+			sep = '\0';
+			break;
+		case 'c':		/* external checksum format */
+			if (fmt++)
+				goto sep_err;
+			sep = '\0';
+			chk = 1;
+			break;
+		case 't':		/* text format */
+			if (fmt++)
+				goto sep_err;
+			sep = '\n';
+			break;
+		case 'r':		/* handle CRLF like LF */
+			crlf_is_lf = 1;
+			break;
+		case 'd':
+			del = 1;
+			break;
+		default:
+			return CMD_RET_USAGE;
 		}
 	}
 
-	if (argc < 1)
+	if (gs.nonopts < 1)
 		return CMD_RET_USAGE;
 
 	if (!fmt)
 		printf("## Warning: defaulting to text format\n");
 
-	if (sep != '\n' && crlf_is_lf )
+	if (sep != '\n' && crlf_is_lf)
 		crlf_is_lf = 0;
 
-	addr = hextoul(argv[0], NULL);
+	addr = hextoul(getopt_pop(&gs), NULL);
 	ptr = map_sysmem(addr, 0);
 
-	if (argc >= 2 && strcmp(argv[1], "-")) {
-		size = hextoul(argv[1], NULL);
+	tok = getopt_pop(&gs);		/* size, "-", or absent */
+	if (tok && strcmp(tok, "-")) {
+		size = hextoul(tok, NULL);
 	} else if (chk) {
 		puts("## Error: external checksum format must pass size\n");
 		return CMD_RET_FAILURE;
@@ -832,7 +823,7 @@ static int do_env_import(struct cmd_tbl *cmdtp, int flag,
 		size = 0;
 
 		while (size < MAX_ENV_SIZE) {
-			if ((*s == sep) && (*(s+1) == '\0'))
+			if ((*s == sep) && (*(s + 1) == '\0'))
 				break;
 			++s;
 			++size;
@@ -845,7 +836,7 @@ static int do_env_import(struct cmd_tbl *cmdtp, int flag,
 		printf("## Info: input data size = %zu = 0x%zX\n", size, size);
 	}
 
-	if (argc > 2)
+	if (gs.nonopts > 0)
 		wl = 1;
 
 	if (chk) {
@@ -868,7 +859,9 @@ static int do_env_import(struct cmd_tbl *cmdtp, int flag,
 	}
 
 	if (!himport_r(&env_htab, ptr, size, sep, del ? 0 : H_NOCLEAR,
-		       crlf_is_lf, wl ? argc - 2 : 0, wl ? &argv[2] : NULL)) {
+		       crlf_is_lf,
+		       wl ? gs.nonopts : 0,
+		       wl ? &gs.args[gs.index] : NULL)) {
 		pr_err("## Error: Environment import failed: errno = %d\n",
 		       errno);
 		return 1;
@@ -879,7 +872,7 @@ static int do_env_import(struct cmd_tbl *cmdtp, int flag,
 
 sep_err:
 	printf("## %s: only one of \"-b\", \"-c\" or \"-t\" allowed\n",
-		cmd);
+	       argv[0]);
 	return 1;
 }
 #endif
-- 
2.43.0

[RFC PATCH 11/11] doc: commands: Recommend getopt() for option parsing

[Simon Glass]

The getopt() helper in <getopt.h> has been available since 2020 but
only two files in cmd/ actually use it; everything else hand-rolls
argv loops to detect -x style flags. New commands keep using ad-hoc
parsers because the developer guide for commands does not mention that
a shared helper exists.

Add a "Parsing options" section to the commands documentation with a
worked example.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 doc/develop/commands.rst | 62 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/doc/develop/commands.rst b/doc/develop/commands.rst
index 77a7a4d9c02..a1f0b59501d 100644
--- a/doc/develop/commands.rst
+++ b/doc/develop/commands.rst
@@ -118,6 +118,68 @@ CMD_RET_USAGE
     The command was called with invalid parameters. This value
     leads to the display of the usage string.
 
+Parsing options
+---------------
+
+Commands that take options (``-x``, ``-f file``, etc.) should use the
+getopt() helper in ``<getopt.h>`` rather than walking ``argv`` by hand. This
+keeps option handling consistent across commands, supports grouped flags
+(``-abc``) and arguments attached either to the option (``-ffile``) or in
+the next argv entry (``-f file``), and produces sensible errors for unknown
+or incomplete options.
+
+A typical use looks like:
+
+.. code-block:: c
+
+    #include <getopt.h>
+
+    static int do_foo(struct cmd_tbl *cmdtp, int flag, int argc,
+                      char *const argv[])
+    {
+        struct getopt_state gs;
+        const char *file = NULL;
+        bool verbose = false;
+        int opt;
+
+        getopt_init_state(&gs);
+        while ((opt = getopt(&gs, argc, argv, "vf:")) > 0) {
+            switch (opt) {
+            case 'v':
+                verbose = true;
+                break;
+            case 'f':
+                file = gs.arg;
+                break;
+            default:
+                return CMD_RET_USAGE;
+            }
+        }
+
+        /* Positional arguments start at gs.index */
+        if (gs.index >= argc)
+            return CMD_RET_USAGE;
+
+        return do_the_work(file, verbose, argv[gs.index]);
+    }
+
+A few points worth noting:
+
+* Each command owns a ``struct getopt_state`` on the stack, so getopt() is
+  reentrant. There are no globals like ``optind`` or ``optarg``; the next
+  unparsed argument is at ``gs.index`` and the option argument (when one is
+  expected) is in ``gs.arg``.
+* In the option string, ``x`` is a flag, ``x:`` requires an argument, and
+  ``x::`` takes an optional argument.
+* Unlike POSIX getopt(), this implementation does **not** reorder ``argv``,
+  so options must appear before positional arguments.
+* Returning ``CMD_RET_USAGE`` from the ``default`` case prints the command's
+  usage string for free; there is no need to call ``cmd_usage()`` directly.
+* Use getopt_silent() if the command should suppress the built-in error
+  messages and report problems itself.
+
+For the full API, see :doc:`../api/getopt`.
+
 Completion function
 -------------------
 
-- 
2.43.0

Re: [RFC PATCH 05/11] lib: getopt: Permute by default with inline reorder

[Sean Anderson]

On 5/15/26 16:32, Simon Glass wrote:
> Currently getopt() does not reorder its argv: it stops at the first
> non-option, which forces options to appear before positional arguments.
> This is not the usual way that arguments work - people expect options to
> be recognised wherever they appear on the command line, although some
> U-Boot commands are hard-wires for particular positions.
> 
> Restructure the parser to permute by default. Add argc and a writable
> args[CONFIG_SYS_MAXARGS + 1] buffer to struct getopt_state, plus a
> nonopts counter. The init function getopt_init_state() takes argc and
> argv, copies argv into args, and resets the parse position. Then
> getopt() and getopt_silent() take only the state and an optstring; argc
> and argv are read from the state.
> 
> When getopt() encounters a non-option, it bubbles that argv element to
> the end of args and increments nonopts, then keeps scanning. The outer
> condition stops when index + nonopts reaches argc, so the parked
> non-options are never re-scanned. After parsing finishes, gs.index
> points at the first parked non-option, with gs.nonopts of them sitting
> at gs.args[gs.index .. argc - 1]
> 
> Callers that need the previous POSIX-style 'stop at first non-option'
> behaviour can prefix optstring with '+'. This matches GNU getopt's
> convention and keeps to two public entry points (getopt() and
> getopt_silent()) instead of doubling them up for in-order variants.
> 
> Update the two existing in-tree callers, cmd/bdinfo.c and cmd/log.c, to
> the new signatures. bdinfo has no positional arguments so it works the
> same either way; log filter-list/add/remove preserve their original
> semantics with the '+' prefix. Update test helper in test/lib/getopt.c
> too.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>   cmd/bdinfo.c      |   4 +-
>   cmd/log.c         |  12 ++--
>   include/getopt.h  | 146 +++++++++++++++++++++++-----------------------
>   lib/getopt.c      |  67 ++++++++++++++++-----
>   test/lib/getopt.c |   6 +-
>   5 files changed, 135 insertions(+), 100 deletions(-)
> 
> diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c
> index ddf77303735..c46094c3ddf 100644
> --- a/cmd/bdinfo.c
> +++ b/cmd/bdinfo.c
> @@ -188,8 +188,8 @@ int do_bdinfo(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>   	if (!CONFIG_IS_ENABLED(GETOPT) || argc == 1)
>   		return bdinfo_print_all(bd);
>   
> -	getopt_init_state(&gs);
> -	while ((opt = getopt(&gs, argc, argv, "aem")) > 0) {
> +	getopt_init_state(&gs, argc, argv);
> +	while ((opt = getopt(&gs, "aem")) > 0) {
>   		switch (opt) {
>   		case 'a':
>   			return bdinfo_print_all(bd);
> diff --git a/cmd/log.c b/cmd/log.c
> index 64add6d8b5a..344f379d8cc 100644
> --- a/cmd/log.c
> +++ b/cmd/log.c
> @@ -95,8 +95,8 @@ static int do_log_filter_list(struct cmd_tbl *cmdtp, int flag, int argc,
>   	struct log_filter *filt;
>   	struct log_device *ldev;
>   
> -	getopt_init_state(&gs);
> -	while ((opt = getopt(&gs, argc, argv, "d:")) > 0) {
> +	getopt_init_state(&gs, argc, argv);
> +	while ((opt = getopt(&gs, "+d:")) > 0) {
>   		switch (opt) {
>   		case 'd':
>   			drv_name = gs.arg;
> @@ -157,8 +157,8 @@ static int do_log_filter_add(struct cmd_tbl *cmdtp, int flag, int argc,
>   	enum log_level_t level = LOGL_MAX;
>   	struct getopt_state gs;
>   
> -	getopt_init_state(&gs);
> -	while ((opt = getopt(&gs, argc, argv, "Ac:d:Df:F:l:L:p")) > 0) {
> +	getopt_init_state(&gs, argc, argv);
> +	while ((opt = getopt(&gs, "+Ac:d:Df:F:l:L:p")) > 0) {
>   		switch (opt) {
>   		case 'A':
>   #define do_type() do { \
> @@ -250,8 +250,8 @@ static int do_log_filter_remove(struct cmd_tbl *cmdtp, int flag, int argc,
>   	const char *drv_name = "console";
>   	struct getopt_state gs;
>   
> -	getopt_init_state(&gs);
> -	while ((opt = getopt(&gs, argc, argv, "ad:")) > 0) {
> +	getopt_init_state(&gs, argc, argv);
> +	while ((opt = getopt(&gs, "+ad:")) > 0) {
>   		switch (opt) {
>   		case 'a':
>   			all = true;
> diff --git a/include/getopt.h b/include/getopt.h
> index 0cf7ee84d6f..5a9e7802e65 100644
> --- a/include/getopt.h
> +++ b/include/getopt.h
> @@ -10,120 +10,120 @@
>   #define __GETOPT_H
>   
>   #include <stdbool.h>
> +#include <linux/kconfig.h>
>   
>   /**
>    * struct getopt_state - Saved state across getopt() calls
> + *
> + * Initialise with getopt_init_state(); the embedded @args buffer holds a
> + * working copy of the argv passed to that initialiser.
>    */
>   struct getopt_state {
> +	/** @argc: Argument count, as passed to getopt_init_state() */
> +	int argc;
> +	/**
> +	 * @args: Working copy of argv. getopt() reorders this in place: as
> +	 * options are consumed, non-options are bubbled to the end.

Please use terminology from getopt(3) wherever possible. So "permute" instead
of "bubble" or "park".

non-options are permuted to the end.

> +	 */
> +	char *args[CONFIG_SYS_MAXARGS + 1];

This should come first to eliminate padding. And should probably be named argv for consistency.

>   	/**
> -	 * @index: Index of the next unparsed argument of @argv. If getopt() has
> -	 * parsed all of @argv, then @index will equal @argc.
> +	 * @index: Index of the next unparsed argument of @args. When
> +	 * parsing has finished, @index sits at the first parked
> +	 * non-option (or @argc if none).

Index of the next unparsed argument of @argv. If getopt() has parsed all of @argv,
then @index will be the first non-option argument of @args (or @argc if none).

>   	 */
>   	int index;
> -	/** @arg_index: Index within the current argument */
> +	/**
> +	 * @arg_index: Offset of the next unparsed character within
> +	 * ``args[index]``. Lets the parser step through grouped short
> +	 * options like ``-abc`` (1, 2, 3...). Reset to 1 each time
> +	 * @index advances to the next argv slot.

to the next argument.

> +	 */
>   	int arg_index;
> +	/**
> +	 * @nonopts: Number of non-option arguments parked at the tail> +	 * of @args. The parser bubbles them down so it can keep scanning
> +	 * for options past them.

Number of non-option arguments in @args.

> +	 */
> +	int nonopts;

Could we just modify argc instead? E.g. once we move a non-option to the end, then we
decrement argc. I think this also fixes the problem discussed below.

>   	union {
>   		/**
> -		 * @opt: Option being parsed when an error occurs. @opt is only
> -		 * valid when getopt() returns ``?`` or ``:``.
> +		 * @opt: Option being parsed when an error occurs. @opt is
> +		 * only valid when getopt() returns ``?`` or ``:``.
>   		 */
>   		int opt;
>   		/**
> -		 * @arg: The argument to an option, NULL if there is none. @arg
> -		 * is only valid when getopt() returns an option character.
> +		 * @arg: The argument to an option, NULL if there is none.
> +		 * @arg is only valid when getopt() returns an option
> +		 * character.
>   		 */
>   		char *arg;
>   	};
>   };
>   
>   /**
> - * getopt_init_state() - Initialize a &struct getopt_state
> - * @gs: The state to initialize
> - *
> - * This must be called before using @gs with getopt().
> + * getopt_init_state() - Initialise a &struct getopt_state from an argv
> + * @gs: The state to initialise
> + * @argc: Argument count
> + * @argv: Source argv. Copied into @gs->args; the original is not
> + *        modified. @argc must not exceed ``CONFIG_SYS_MAXARGS``;
> + *        excess arguments are silently dropped.
>    */
> -void getopt_init_state(struct getopt_state *gs);
> +void getopt_init_state(struct getopt_state *gs, int argc,
> +		       char *const argv[]);
>   
> -int __getopt(struct getopt_state *gs, int argc, char *const argv[],
> -	     const char *optstring, bool silent);
> +int __getopt(struct getopt_state *gs, const char *optstring, bool silent);
>   
>   /**
>    * getopt() - Parse short command-line options
> - * @gs: Internal state and out-of-band return arguments. This must be
> - *      initialized with getopt_init_context() beforehand.
> - * @argc: Number of arguments, not including the %NULL terminator
> - * @argv: Argument list, terminated by %NULL
> - * @optstring: Option specification, as described below
> - *
> - * getopt() parses short options. Short options are single characters. They may
> - * be followed by a required argument or an optional argument. Arguments to
> - * options may occur in the same argument as an option (like ``-larg``), or
> - * in the following argument (like ``-l arg``). An argument containing
> - * options begins with a ``-``. If an option expects no arguments, then it may
> - * be immediately followed by another option (like ``ls -alR``).
> - *
> - * @optstring is a list of accepted options. If an option is followed by ``:``
> - * in @optstring, then it expects a mandatory argument. If an option is followed
> - * by ``::`` in @optstring, it expects an optional argument. @gs.arg points
> - * to the argument, if one is parsed.
> - *
> - * getopt() stops parsing options when it encounters the first non-option
> - * argument, when it encounters the argument ``--``, or when it runs out of
> - * arguments. For example, in ``ls -l foo -R``, option parsing will stop when
> - * getopt() encounters ``foo``, if ``l`` does not expect an argument. However,
> - * the whole list of arguments would be parsed if ``l`` expects an argument.
> + * @gs: State, initialised with getopt_init_state()
> + * @optstring: Option specification (see getopt(3))

Please retain the original description. We are not 100% compatible
with getopt, and I think explicit examples are a good way to document what we
support as well as the new API, which doesn't quite match libc getopt.

>    *
> - * An example invocation of getopt() might look like::
> + * Parses the next short option from @gs. By default, getopt() permutes

Parse

> + * the working argv: when it hits a non-option, it bubbles that argv

bubbles -> moves

> + * element to the end and keeps scanning, so options can appear
> + * anywhere on the command line. After parsing finishes (returns -1),
> + * @gs.index sits at the first parked non-option, and there are
> + * @gs.nonopts of them at @gs.args[gs.index..argc-1].
>    *
> - *     char *argv[] = { "program", "-cbx", "-a", "foo", "bar", 0 };
> - *     int opt, argc = ARRAY_SIZE(argv) - 1;
> - *     struct getopt_state gs;
> + * If @optstring begins with ``+``, getopt() instead stops at the
> + * first non-option (POSIX ``getopt`` behaviour). Use this when the
> + * command's grammar requires options before positionals, or when the
> + * option string includes optional arguments (``::``) whose adjacency
> + * to a following positional would be ambiguous under permutation.

when the command requires options before non-options, or when options
with optional arguments would be ambiguous under permutation.

>    *
> - *     getopt_init_state(&gs);
> - *     while ((opt = getopt(&gs, argc, argv, "a::b:c")) != -1)
> - *         printf("opt = %c, index = %d, arg = \"%s\"\n", opt, gs.index, gs.arg);
> - *     printf("%d argument(s) left\n", argc - gs.index);
> + * In @optstring proper, ``x`` is a flag, ``x:`` requires an argument,
> + * and ``x::`` takes an optional argument. The argument is delivered
> + * in @gs.arg.
>   *
> - * and would produce an output of::
> - *
> - *     opt = c, index = 1, arg = "<NULL>"
> - *     opt = b, index = 2, arg = "x"
> - *     opt = a, index = 4, arg = "foo"
> - *     1 argument(s) left
> - *
> - * For further information, refer to the getopt(3) man page.
> + * A literal ``--`` argument terminates option scanning; the parser
> + * advances past it and returns -1.
>    *
>    * Return:
> - * * An option character if an option is found. @gs.arg is set to the
> - *   argument if there is one, otherwise it is set to ``NULL``.
> - * * ``-1`` if there are no more options, if a non-option argument is
> - *   encountered, or if an ``--`` argument is encountered.
> - * * ``'?'`` if we encounter an option not in @optstring. @gs.opt is set to
> - *   the unknown option.
> - * * ``':'`` if an argument is required, but no argument follows the
> - *   option. @gs.opt is set to the option missing its argument.
> - *
> - * @gs.index is always set to the index of the next unparsed argument in @argv.
> + * * An option character if an option is found. @gs.arg is set to
> + *   the argument if there is one, otherwise NULL.
> + * * ``-1`` if there are no more options.
> + * * ``'?'`` for an option not in @optstring; @gs.opt is the unknown
> + *   option character.
> + * * ``':'`` for an option missing its required argument; @gs.opt is
> + *   the option character.
>    */
> -static inline int getopt(struct getopt_state *gs, int argc,
> -			 char *const argv[], const char *optstring)
> +static inline int getopt(struct getopt_state *gs, const char *optstring)
>   {
> -	return __getopt(gs, argc, argv, optstring, false);
> +	return __getopt(gs, optstring, false);
>   }
>   
>   /**
> - * getopt_silent() - Parse short command-line options silently
> + * getopt_silent() - Parse short command-line options without errors
>    * @gs: State
> - * @argc: Argument count
> - * @argv: Argument list
>    * @optstring: Option specification
>    *
> - * Same as getopt(), except no error messages are printed.
> + * Same as getopt() but does not print error messages for unknown
> + * options or missing arguments.
>    */
> -static inline int getopt_silent(struct getopt_state *gs, int argc,
> -				char *const argv[], const char *optstring)
> +static inline int getopt_silent(struct getopt_state *gs,
> +				const char *optstring)
>   {
> -	return __getopt(gs, argc, argv, optstring, true);
> +	return __getopt(gs, optstring, true);
>   }
>   
>   #endif /* __GETOPT_H */
> diff --git a/lib/getopt.c b/lib/getopt.c
> index e9175e2fff4..70adb2d0faf 100644
> --- a/lib/getopt.c
> +++ b/lib/getopt.c
> @@ -10,37 +10,71 @@
>   
>   #include <getopt.h>
>   #include <log.h>
> +#include <linux/kernel.h>
>   #include <linux/string.h>
>   
> -void getopt_init_state(struct getopt_state *gs)
> +void getopt_init_state(struct getopt_state *gs, int argc, char *const argv[])
>   {
> +	int max = ARRAY_SIZE(gs->args) - 1;
> +
> +	if (argc > max)
> +		argc = max;
> +
> +	gs->argc = argc;
> +	memcpy(gs->args, argv, (argc + 1) * sizeof(*gs->args));
> +	gs->args[argc] = NULL;
>   	gs->index = 1;
>   	gs->arg_index = 1;
> +	gs->nonopts = 0;
>   }
>   
> -int __getopt(struct getopt_state *gs, int argc, char *const argv[],
> -	     const char *optstring, bool silent)
> +int __getopt(struct getopt_state *gs, const char *optstring, bool silent)
>   {
> -	char curopt;   /* current option character */
> -	const char *curoptp; /* pointer to the current option in optstring */
> +	char curopt;	/* current option character */
> +	const char *curoptp;	/* pointer to the current option in optstring */
> +	bool stop_nonopt = false;
> +	char **argv = gs->args;
> +	int argc = gs->argc;
> +
> +	if (*optstring == '+') {
> +		stop_nonopt = true;
> +		optstring++;
> +	}
>   
>   	while (1) {
> -		log_debug("arg_index: %d index: %d\n", gs->arg_index,
> -			  gs->index);
> +		log_debug("arg_index: %d index: %d nonopts: %d\n",
> +			  gs->arg_index, gs->index, gs->nonopts);
>   
>   		/* `--` indicates the end of options */
> -		if (gs->arg_index == 1 && argv[gs->index] &&
> +		if (gs->arg_index == 1 && gs->index < argc &&
>   		    !strcmp(argv[gs->index], "--")) {
>   			gs->index++;
>   			return -1;
>   		}
>   
> -		/* Out of arguments */
> -		if (gs->index >= argc)
> -			return -1;
> +		/*
> +		 * Bubble non-options to the end so we can keep scanning for
> +		 * options past them. In '+' mode (POSIX), stop at the first
> +		 * non-option instead.
> +		 */
> +		while (gs->arg_index == 1 &&
> +		       gs->index + gs->nonopts < argc &&
> +		       *argv[gs->index] != '-') {
> +			char *tmp;
> +			int i;
> +
> +			if (stop_nonopt)
> +				return -1;
> +
> +			tmp = argv[gs->index];
> +			gs->nonopts++;
> +			for (i = gs->index; i + 1 < argc; i++)
> +				argv[i] = argv[i + 1];
> +			argv[argc - 1] = tmp;
> +		}
>   
> -		/* Can't parse non-options */
> -		if (*argv[gs->index] != '-')
> +		/* Out of options to scan */
> +		if (gs->index + gs->nonopts >= argc)
>   			return -1;
>   
>   		/* We have found an option */
> @@ -48,7 +82,8 @@ int __getopt(struct getopt_state *gs, int argc, char *const argv[],
>   		if (curopt)
>   			break;
>   		/*
> -		 * no more options in current argv[] element; try the next one
> +		 * No more options in current argv[] element; advance to the
> +		 * next one
>   		 */
>   		gs->index++;
>   		gs->arg_index = 1;
> @@ -80,7 +115,7 @@ int __getopt(struct getopt_state *gs, int argc, char *const argv[],
>   			gs->arg_index = 1;
>   			return curopt;
>   		}

Is the above condition (not included in the diff) correct for something like

getopt({ "prog", "foo", "-a" }, 3, "a:")

? That is, getopt should return ':' and not 'a' in this situation, but I'm not sure we're
tracking the non-options correctly.

> -		if (gs->index + 1 == argc) {
> +		if (gs->index + gs->nonopts + 1 == argc) {
>   			/* We are at the last argv[] element */
>   			gs->arg = NULL;
>   			gs->index++;
> @@ -113,7 +148,7 @@ int __getopt(struct getopt_state *gs, int argc, char *const argv[],
>   	gs->index++;
>   	gs->arg_index = 1;
>   
> -	if (gs->index >= argc || argv[gs->index][0] == '-') {
> +	if (gs->index + gs->nonopts >= argc || argv[gs->index][0] == '-') {
>   		if (!silent)
>   			printf("option requires an argument -- %c\n", curopt);
>   		gs->opt = curopt;
> diff --git a/test/lib/getopt.c b/test/lib/getopt.c
> index 388a076200b..6b384eb016a 100644
> --- a/test/lib/getopt.c
> +++ b/test/lib/getopt.c
> @@ -18,9 +18,9 @@ static int do_test_getopt(struct unit_test_state *uts, int line,
>   {
>   	int opt;
>   
> -	getopt_init_state(gs);
> +	getopt_init_state(gs, args, argv);
>   	for (int i = 0; i < expected_count; i++) {
> -		opt = getopt_silent(gs, args, argv, optstring);
> +		opt = getopt_silent(gs, optstring);
>   		if (expected[i] != opt) {
>   			/*
>   			 * Fudge the line number so we can tell which test
> @@ -34,7 +34,7 @@ static int do_test_getopt(struct unit_test_state *uts, int line,
>   		}
>   	}
>   
> -	opt = getopt_silent(gs, args, argv, optstring);
> +	opt = getopt_silent(gs, optstring);
>   	if (opt != -1) {
>   		ut_failf(uts, __FILE__, line, __func__,
>   			 "getopt() != -1",

Please add a few tests for reordered arguments, especially the scenario described above.

Re: [RFC PATCH 06/11] lib: getopt: Add getopt_pop() helper

[Sean Anderson]

On 5/15/26 16:32, Simon Glass wrote:
> Callers that consume positional arguments after parsing routinely write
> the same shape:
> 
>      algo = gs.args[gs.index];
>      /* advance past algo */
>      return hash_command(algo, ..., gs.nonopts - 1, &gs.args[gs.index + 1]);

I think it's better to be explicit.

> 
> Wrap that in a small inline helper that returns the next positional,
> advances gs.index, decrements gs.nonopts, and returns NULL when no
> positionals remain. The same helper doubles as an iterator:
> 
>      while ((arg = getopt_pop(&gs)))
>          process(arg);
> 
> stddef.h is now included from getopt.h for the NULL macro.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>   include/getopt.h | 23 +++++++++++++++++++++++
>   1 file changed, 23 insertions(+)
> 
> diff --git a/include/getopt.h b/include/getopt.h
> index 5a9e7802e65..013431807ff 100644
> --- a/include/getopt.h
> +++ b/include/getopt.h
> @@ -10,6 +10,7 @@
>   #define __GETOPT_H
>   
>   #include <stdbool.h>
> +#include <stddef.h>
>   #include <linux/kconfig.h>
>   
>   /**
> @@ -126,4 +127,26 @@ static inline int getopt_silent(struct getopt_state *gs,
>   	return __getopt(gs, optstring, true);
>   }
>   
> +/**
> + * getopt_pop() - Take the next remaining positional argument
> + * @gs: State, after getopt() has returned -1
> + *
> + * Returns the first parked non-option (``gs->args[gs->index]``),
> + * advances the index, and decrements ``gs->nonopts``. Returns NULL
> + * when no positional arguments remain.
> + *
> + * Useful for consuming positionals one at a time after parsing::
> + *
> + *     algo = getopt_pop(&gs);
> + *     return hash_command(algo, ..., gs.nonopts, &gs.args[gs.index]);
> + */
> +static inline char *getopt_pop(struct getopt_state *gs)
> +{
> +	if (gs->index >= gs->argc)
> +		return NULL;
> +	if (gs->nonopts > 0)
> +		gs->nonopts--;
> +	return gs->args[gs->index++];
> +}
> +
>   #endif /* __GETOPT_H */

Re: [RFC PATCH 00/11] Tidy command option parsing and use it a bit

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1147 bytes --]

On Fri, May 15, 2026 at 02:32:51PM -0600, Simon Glass wrote:

> We have had getopt() for over five years but it is not much used. It is
> much easier to understand arg-parsing using getopt() and it avoids
> common errors. As the named maintainer I decided to look at how to make
> more use of it.
> 
> So this series explores the impact of converting a few commands to use
> getopt() instead of ad-hoc parsing. It updates getopt() to handle flags
> anywhere in the cmdline and provides a few helpers to reduce
> boilerplate.
> 
> The chosen commands are:
> 
> - echo: very simple with no flags
> - hash: fairly simple with just one flag
> - env grep/export/import - fuller examples
> 
> The series also adds a recommendation to use getopt() for new commands.
> 
> To try to reduce the code-size increase, a lower-case function is added
> and called from a few places. The difference is fairly marginal.
> 
> Overall, the result is not pretty (see below) with about a 1.1K size
> increase on arm64:

I think that means this is a no-go, and you need to work out what might
lead to a much smaller growth here.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [RFC PATCH 07/11] cmd: echo: Use getopt() with '+' prefix for option parsing

[Sean Anderson]

On 5/15/26 16:32, Simon Glass wrote:
> The 'echo' command's option parser is a single strcmp against argv[1]
> that decides whether to suppress the trailing newline. Convert it to
> getopt() so echo follows the same shape as the other commands in this
> series and exercises the '+' prefix that POSIX-style 'stop at first
> non-option' callers need.
> 
> The optstring uses the '+' prefix to preserve bash echo behaviour: -n is
> honoured only as the very first argument, so 'echo hello -n' still
> prints 'hello -n\n' verbatim.
> 
> Two minor differences from the bash builtin remain, both of which
> the user can work around with quoting or --:
> 
> * -x (an unknown short option) returns CMD_RET_USAGE rather than
>    printing literally; use 'echo -- -x' to print it.
> * -nfoo (joined form) parses -n and then errors on the trailing
>    characters.

Why? This introduces incompatibility with echo as it exists today as
well as unix-style echo. We can have an (almost) completely-compatible
echo with

	getopt_init_state(&gs, argc, argv);
	while (getopt_silent(&gs, "+n") == 'n')
		newline = false;

	for (i = gs.index; i < argc; ++i) {
		<snip>

The only difference is that something like "echo -na" will result in
"-na" and not "-na\n". IMO echo is not a good candidate for getopt
due to its unusual argument handling. Even coreutils echo does not
use getopt. So I think we should really leave echo as-is.

> Add three test cases that pin down the new behaviour: trailing -n stays
> literal, -- ends option parsing, and -- is consumed even after a
> recognised flag. The positional loop uses getopt_pop() as an iterator.
> CMD_ECHO selects GETOPT so the parser is linked in on boards that don't
> already enable it.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>   cmd/Kconfig          |  1 +
>   cmd/echo.c           | 23 ++++++++++++++---------
>   test/cmd/test_echo.c | 10 ++++++++++
>   3 files changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index c71c6824a19..709696c3c41 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1916,6 +1916,7 @@ config CMD_CAT
>   config CMD_ECHO
>   	bool "echo"
>   	default y
> +	select GETOPT
>   	help
>   	  Echo args to console
>   
> diff --git a/cmd/echo.c b/cmd/echo.c
> index d1346504cfb..63422c75cc6 100644
> --- a/cmd/echo.c
> +++ b/cmd/echo.c
> @@ -5,27 +5,32 @@
>    */
>   
>   #include <command.h>
> -#include <linux/string.h>
> +#include <getopt.h>
>   
>   static int do_echo(struct cmd_tbl *cmdtp, int flag, int argc,
>   		   char *const argv[])
>   {
> -	int i = 1;
> +	struct getopt_state gs;
>   	bool space = false;
>   	bool newline = true;
> +	char *arg;
> +	int opt;
>   
> -	if (argc > 1) {
> -		if (!strcmp(argv[1], "-n")) {
> +	getopt_init_state(&gs, argc, argv);
> +	while ((opt = getopt(&gs, "+n")) > 0) {
> +		switch (opt) {
> +		case 'n':
>   			newline = false;
> -			++i;
> +			break;
> +		default:
> +			return CMD_RET_USAGE;
>   		}
>   	}
>   
> -	for (; i < argc; ++i) {
> -		if (space) {
> +	while ((arg = getopt_pop(&gs))) {
> +		if (space)
>   			putc(' ');
> -		}
> -		puts(argv[i]);
> +		puts(arg);
>   		space = true;
>   	}
>   
> diff --git a/test/cmd/test_echo.c b/test/cmd/test_echo.c
> index 7ed534742f7..5fc139fbe68 100644
> --- a/test/cmd/test_echo.c
> +++ b/test/cmd/test_echo.c
> @@ -35,6 +35,16 @@ static struct test_data echo_data[] = {
>   	/* Test handling of shell variables. */
>   	{"setenv jQx; for jQx in 1 2 3; do echo -n \"${jQx}, \"; done; echo;",
>   	 "1, 2, 3, "},
> +	/* -n only suppresses the newline when it comes before any
> +	 * positional argument; a trailing -n is just another argument.
> +	 */
> +	{"echo hello -n", "hello -n"},
> +	/* "--" ends option parsing, so a following dash-prefixed token
> +	 * is printed verbatim instead of being rejected.
> +	 */
> +	{"echo -- -x", "-x"},
> +	/* "--" is consumed even when it follows a recognised flag. */
> +	{"echo -n -- foo; echo done", "foodone"},
>   };
>   
>   static int lib_test_hush_echo(struct unit_test_state *uts)

Re: [RFC PATCH 00/11] Tidy command option parsing and use it a bit

[Sean Anderson]

On 5/15/26 17:43, Tom Rini wrote:
> On Fri, May 15, 2026 at 02:32:51PM -0600, Simon Glass wrote:
> 
>> We have had getopt() for over five years but it is not much used. It is
>> much easier to understand arg-parsing using getopt() and it avoids
>> common errors. As the named maintainer I decided to look at how to make
>> more use of it.
>>
>> So this series explores the impact of converting a few commands to use
>> getopt() instead of ad-hoc parsing. It updates getopt() to handle flags
>> anywhere in the cmdline and provides a few helpers to reduce
>> boilerplate.
>>
>> The chosen commands are:
>>
>> - echo: very simple with no flags
>> - hash: fairly simple with just one flag
>> - env grep/export/import - fuller examples
>>
>> The series also adds a recommendation to use getopt() for new commands.
>>
>> To try to reduce the code-size increase, a lower-case function is added
>> and called from a few places. The difference is fairly marginal.
>>
>> Overall, the result is not pretty (see below) with about a 1.1K size
>> increase on arm64:
> 
> I think that means this is a no-go, and you need to work out what might
> lead to a much smaller growth here.
> 

Unfortunately, the only way to get a size reduction is to convert as many
commands as possible.

--Sean

Re: [RFC PATCH 00/11] Tidy command option parsing and use it a bit

[Sean Anderson]

On 5/15/26 17:59, Sean Anderson wrote:
> On 5/15/26 17:43, Tom Rini wrote:
>> On Fri, May 15, 2026 at 02:32:51PM -0600, Simon Glass wrote:
>>
>>> We have had getopt() for over five years but it is not much used. It is
>>> much easier to understand arg-parsing using getopt() and it avoids
>>> common errors. As the named maintainer I decided to look at how to make
>>> more use of it.
>>>
>>> So this series explores the impact of converting a few commands to use
>>> getopt() instead of ad-hoc parsing. It updates getopt() to handle flags
>>> anywhere in the cmdline and provides a few helpers to reduce
>>> boilerplate.
>>>
>>> The chosen commands are:
>>>
>>> - echo: very simple with no flags
>>> - hash: fairly simple with just one flag
>>> - env grep/export/import - fuller examples
>>>
>>> The series also adds a recommendation to use getopt() for new commands.
>>>
>>> To try to reduce the code-size increase, a lower-case function is added
>>> and called from a few places. The difference is fairly marginal.
>>>
>>> Overall, the result is not pretty (see below) with about a 1.1K size
>>> increase on arm64:
>>
>> I think that means this is a no-go, and you need to work out what might
>> lead to a much smaller growth here.
>>
> 
> Unfortunately, the only way to get a size reduction is to convert as many
> commands as possible.
> 
> --Sean

And to expand on this, you really only get a reduction for any given command
when there are several options being parsed already like in the nvedit stuff.
Since most U-Boot commands don't use options it will be difficult to get a
reduction even when converting most commands on a board.

That's why I used it for new commands where no one could complain that
I grew their board.

I think it would be nice for all u-boot commands to have proper option support
like in barebox, but it's a lot of effort.

--Sean

Re: [RFC PATCH 00/11] Tidy command option parsing and use it a bit

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 4763 bytes --]

On Fri, May 15, 2026 at 06:06:38PM -0400, Sean Anderson wrote:
> On 5/15/26 17:59, Sean Anderson wrote:
> > On 5/15/26 17:43, Tom Rini wrote:
> > > On Fri, May 15, 2026 at 02:32:51PM -0600, Simon Glass wrote:
> > > 
> > > > We have had getopt() for over five years but it is not much used. It is
> > > > much easier to understand arg-parsing using getopt() and it avoids
> > > > common errors. As the named maintainer I decided to look at how to make
> > > > more use of it.
> > > > 
> > > > So this series explores the impact of converting a few commands to use
> > > > getopt() instead of ad-hoc parsing. It updates getopt() to handle flags
> > > > anywhere in the cmdline and provides a few helpers to reduce
> > > > boilerplate.
> > > > 
> > > > The chosen commands are:
> > > > 
> > > > - echo: very simple with no flags
> > > > - hash: fairly simple with just one flag
> > > > - env grep/export/import - fuller examples
> > > > 
> > > > The series also adds a recommendation to use getopt() for new commands.
> > > > 
> > > > To try to reduce the code-size increase, a lower-case function is added
> > > > and called from a few places. The difference is fairly marginal.
> > > > 
> > > > Overall, the result is not pretty (see below) with about a 1.1K size
> > > > increase on arm64:
> > > 
> > > I think that means this is a no-go, and you need to work out what might
> > > lead to a much smaller growth here.
> > > 
> > 
> > Unfortunately, the only way to get a size reduction is to convert as many
> > commands as possible.
> > 
> > --Sean
> 
> And to expand on this, you really only get a reduction for any given command
> when there are several options being parsed already like in the nvedit stuff.
> Since most U-Boot commands don't use options it will be difficult to get a
> reduction even when converting most commands on a board.

We don't even see a reduction on "qcom" which is a platform that already
enabled GETOPT. nvedit is growth too:
01: Prepare v2026.07-rc2
02: lib: string: Add strlower()
   aarch64: (for 1/1 boards) data -40.0 rodata +40.0
            qcom           : data -40 rodata +40
03: cmd: ini: Use strlower() to normalise case
04: fs: fat: Use strlower() to normalise case
05: boot: pxe_utils: Use strlower() in get_string()
06: lib: getopt: Permute by default with inline reorder
   aarch64: (for 1/1 boards) all -6.0 data -24.0 rodata +18.0
            qcom           : all -6 data -24 rodata +18
               u-boot: add: 0/0, grow: 5/-1 bytes: 272/-8 (264)
                 function                                   old     new   delta
                 __getopt                                   576     756    +180
                 getopt_init_state                           12      88     +76
                 do_log_filter_remove                       364     372      +8
                 do_log_filter_list                         444     448      +4
                 do_log_filter_add                          580     584      +4
                 do_bdinfo                                  160     152      -8
07: lib: getopt: Add getopt_pop() helper
08: cmd: echo: Use getopt() with '+' prefix for option parsing
   aarch64: (for 1/1 boards) all +3.0 rodata +3.0
            qcom           : all +3 rodata +3
               u-boot: add: 0/0, grow: 1/0 bytes: 68/0 (68)
                 function                                   old     new   delta
                 do_echo                                    156     224     +68
09: cmd: hash: Use getopt() for option parsing
10: cmd: nvedit: Use getopt() in env grep
11: cmd: nvedit: Use getopt() in env export and env import
   aarch64: (for 1/1 boards) all -4.0 data -16.0 rodata +12.0
            qcom           : all -4 data -16 rodata +12
               u-boot: add: 0/0, grow: 2/0 bytes: 164/0 (164)
                 function                                   old     new   delta
                 do_env_import                              692     832    +140
                 do_env_export                              608     632     +24
12: doc: commands: Recommend getopt() for option parsing

> That's why I used it for new commands where no one could complain that
> I grew their board.
> 
> I think it would be nice for all u-boot commands to have proper option support
> like in barebox, but it's a lot of effort.

Yes, it would be good to move in that direction. But we need to figure
out how to do it in such a way that's not such a big size loss. If we
have to do a big conversion to start with, fine. If we have to
restructure a few other things, fine. But "convert echo so everyone
links it now and it's sunk-cost" isn't the right path.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [RFC PATCH 00/11] Tidy command option parsing and use it a bit

[Sean Anderson]

On 5/15/26 18:21, Tom Rini wrote:
> On Fri, May 15, 2026 at 06:06:38PM -0400, Sean Anderson wrote:
>> On 5/15/26 17:59, Sean Anderson wrote:
>>> On 5/15/26 17:43, Tom Rini wrote:
>>>> On Fri, May 15, 2026 at 02:32:51PM -0600, Simon Glass wrote:
>>>>
>>>>> We have had getopt() for over five years but it is not much used. It is
>>>>> much easier to understand arg-parsing using getopt() and it avoids
>>>>> common errors. As the named maintainer I decided to look at how to make
>>>>> more use of it.
>>>>>
>>>>> So this series explores the impact of converting a few commands to use
>>>>> getopt() instead of ad-hoc parsing. It updates getopt() to handle flags
>>>>> anywhere in the cmdline and provides a few helpers to reduce
>>>>> boilerplate.
>>>>>
>>>>> The chosen commands are:
>>>>>
>>>>> - echo: very simple with no flags
>>>>> - hash: fairly simple with just one flag
>>>>> - env grep/export/import - fuller examples
>>>>>
>>>>> The series also adds a recommendation to use getopt() for new commands.
>>>>>
>>>>> To try to reduce the code-size increase, a lower-case function is added
>>>>> and called from a few places. The difference is fairly marginal.
>>>>>
>>>>> Overall, the result is not pretty (see below) with about a 1.1K size
>>>>> increase on arm64:
>>>>
>>>> I think that means this is a no-go, and you need to work out what might
>>>> lead to a much smaller growth here.
>>>>
>>>
>>> Unfortunately, the only way to get a size reduction is to convert as many
>>> commands as possible.
>>>
>>> --Sean
>>
>> And to expand on this, you really only get a reduction for any given command
>> when there are several options being parsed already like in the nvedit stuff.
>> Since most U-Boot commands don't use options it will be difficult to get a
>> reduction even when converting most commands on a board.
> 
> We don't even see a reduction on "qcom" which is a platform that already
> enabled GETOPT. nvedit is growth too:
> 01: Prepare v2026.07-rc2
> 02: lib: string: Add strlower()
>     aarch64: (for 1/1 boards) data -40.0 rodata +40.0
>              qcom           : data -40 rodata +40
> 03: cmd: ini: Use strlower() to normalise case
> 04: fs: fat: Use strlower() to normalise case
> 05: boot: pxe_utils: Use strlower() in get_string()
> 06: lib: getopt: Permute by default with inline reorder
>     aarch64: (for 1/1 boards) all -6.0 data -24.0 rodata +18.0
>              qcom           : all -6 data -24 rodata +18
>                 u-boot: add: 0/0, grow: 5/-1 bytes: 272/-8 (264)
>                   function                                   old     new   delta
>                   __getopt                                   576     756    +180
>                   getopt_init_state                           12      88     +76
>                   do_log_filter_remove                       364     372      +8
>                   do_log_filter_list                         444     448      +4
>                   do_log_filter_add                          580     584      +4
>                   do_bdinfo                                  160     152      -8

I expected this to grow due to reordering

> 07: lib: getopt: Add getopt_pop() helper
> 08: cmd: echo: Use getopt() with '+' prefix for option parsing
>     aarch64: (for 1/1 boards) all +3.0 rodata +3.0
>              qcom           : all +3 rodata +3
>                 u-boot: add: 0/0, grow: 1/0 bytes: 68/0 (68)
>                   function                                   old     new   delta
>                   do_echo                                    156     224     +68
> 09: cmd: hash: Use getopt() for option parsing
> 10: cmd: nvedit: Use getopt() in env grep
> 11: cmd: nvedit: Use getopt() in env export and env import
>     aarch64: (for 1/1 boards) all -4.0 data -16.0 rodata +12.0
>              qcom           : all -4 data -16 rodata +12
>                 u-boot: add: 0/0, grow: 2/0 bytes: 164/0 (164)
>                   function                                   old     new   delta
>                   do_env_import                              692     832    +140
>                   do_env_export                              608     632     +24

But I thought the nvedit stuff would result in a size reduction. There are 4-5 options
for each command, so the env stuff is basically a best-case scenario. Maybe we
need to revisit the getopt interface.

> 12: doc: commands: Recommend getopt() for option parsing
> 
>> That's why I used it for new commands where no one could complain that
>> I grew their board.
>>
>> I think it would be nice for all u-boot commands to have proper option support
>> like in barebox, but it's a lot of effort.
> 
> Yes, it would be good to move in that direction. But we need to figure
> out how to do it in such a way that's not such a big size loss. If we
> have to do a big conversion to start with, fine. If we have to
> restructure a few other things, fine. But "convert echo so everyone
> links it now and it's sunk-cost" isn't the right path.