Hi Tom, On Tue, 19 May 2026 at 19:06, Tom Rini <[email protected]> wrote: > > On Tue, May 19, 2026 at 05:31:26PM -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 one flag and fixed ordering (worst case?) > > - 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. > > > > v2 includes further attempts to reduce code size, by making reordering > > a Kconfig option, dropping some printing and some intensive optimisation > > of the env commands. > > Let us return to the comments from v1 again: > https://lore.kernel.org/u-boot/[email protected]/ > > Sean was saying that nvedit should be a best case for the conversion. In > v1, grep was no change and "export and import" was 164 bytes on "qcom", > which I chose because it already enabled GETOPT. Let us now look at v2. > Here, grep, export and import are three commits: > 10: cmd: nvedit: Use getopt() in env grep > 11: cmd: nvedit: Use getopt() in env export > aarch64: w+ qcom > +(qcom) 636 | char buf[32]; > +(qcom) | ^~~ > w+(qcom) cmd/nvedit.c: In function 'do_env_export': > w+(qcom) cmd/nvedit.c:636:17: warning: unused variable 'buf' > [-Wunused-variable] > 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: 0/-1 bytes: 0/-32 (-32) > function old new > delta > do_env_export 608 576 > -32 > 12: cmd: nvedit: Use getopt() in env import > aarch64: (for 1/1 boards) all +1.0 data +56.0 rodata -55.0 > qcom : all +1 data +56 rodata -55 > u-boot: add: 0/0, grow: 1/0 bytes: 100/0 (100) > function old new > delta > do_env_import 692 792 > +100 > > So grep is unchanged, export is a small reduction but import is growing, > but less. > > My feedback is this. If you are going to bother with RFC v3, please just > limit it to converting nvedit and showcasing that it's an improvement to > convert. And if it's not, as Sean said in v1 we might need to revisit > the interface.
It is an improvement in terms of the interface, but not really in code size. I've fiddled with it quite a bit - I'm not saying it can't be done but if you look at the code you can see that it is similar before and after. Any thoughts on this point?: "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." We could do it piecemeal, i.e. cmd by cmd. Removing the init call and passing gs instead of argc/v would improve things. Regards, Simon
