Hi,

On Fri, 15 May 2026 at 16:27, Sean Anderson <[email protected]> wrote:
>
> 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.

It isn't really best-case as there is other logic- I would say
somewhere in the middle.

Re the interface, what are you thinkg?

>
> > 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.
>

IMO we should move to getopt, for the reasons I mentioned. But in
order to do that we need to swallow a 1K size increase on arm64 (a
little less with my v2 RFC.

If we can get over that hump I believe we can set a standard that
converting a cmd to getopt should not increase the size...I actually
believe that is mostly achieveable, so long as we are OK to introduce
various helpers to simplify arg access in the command code
(getopt_pop() and more).

There may be opportuntiies to reduce the size - some I have already
mentioned. But unless we can swallow the ~1K, this is a non-starter
(and in fact we may as well remove getopt()).

Regards,
Simon

Reply via email to