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

[Sean Anderson <seanga2@gmail.com>]

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.