[PATCH 0/2] Convert env export and import to use getopt()
Quentin Schulz
quentin.schulz at cherry.de
Thu Jun 25 10:29:44 CEST 2026
On 6/24/26 8:19 PM, Tom Rini wrote:
> On Wed, Jun 24, 2026 at 07:35:49PM +0200, Quentin Schulz wrote:
>> On 5/22/26 7:03 PM, Tom Rini wrote:
>>> On Wed, May 20, 2026 at 02:12:12PM -0600, Simon Glass wrote:
>>>
>>>> U-Boot has had a getopt() implementation for over five years but it is
>>>> not used much; most commands hand-roll their own argv loops to spot
>>>> -x style flags. The env export and env import sub-commands have the
>>>> gnarliest of these parsers in nvedit.c
>>>>
>>>> Each one walks every -prefixed argv element by hand, opens an inner
>>>> loop to split grouped flags and tracks a counter to catch a repeated
>>>> format flag.
>>>>
>>>> This short series converts both sub-commands to getopt(). The mutex
>>>> check for the format flags is done after the option loop, since it is
>>>> a per-command rule rather than an option-parsing rule, and the trailing
>>>> positional list is read straight out of argv from gs.index onwards.
>>>>
>>>> There is no functional change. getopt() stops at the first non-option,
>>>> just as the hand-rolled loops did, so options still have to appear
>>>> before the positional arguments.
>>>>
>>>> Each command gains a 'select GETOPT' so the parser is linked in on
>>>> boards that do not already enable it.
>>>>
>>>> For firefly-rk3399:
>>>>
>>>> 03: cmd: nvedit: Use getopt() in env export
>>>> aarch64: (for 1/1 boards) all -45.0 rodata +27.0 text -72.0
>>>> 04: cmd: nvedit: Use getopt() in env import
>>>> aarch64: (for 1/1 boards) all -27.0 rodata -55.0 text +28.0
>>>>
>>>>
>>>> Simon Glass (2):
>>>> cmd: nvedit: Use getopt() in env export
>>>> cmd: nvedit: Use getopt() in env import
>>>>
>>>> cmd/Kconfig | 2 +
>>>> cmd/nvedit.c | 186 +++++++++++++++++++++++----------------------------
>>>> env/common.c | 2 +-
>>>> 3 files changed, 85 insertions(+), 105 deletions(-)
>>>
>>> For the record, here's a link to v2:
>>> https://lore.kernel.org/u-boot/20260519233207.2765755-1-sjg@chromium.org/
>>> And here's the full size change for firefly-rk3399:
>>> Summary of 3 commits for 1 boards (1 thread, 12 jobs per thread)
>>> 01: arm: Fix typo in linker script
>>> aarch64: w+ firefly-rk3399
>>> +(firefly-rk3399) Image 'simple-bin' is missing external blobs and is non-functional: atf-bl31
>>> +(firefly-rk3399)
>>> +(firefly-rk3399) /binman/simple-bin/fit/images/@atf-SEQ/atf-bl31 (atf-bl31):
>>> +(firefly-rk3399) See the documentation for your board. You may need to build ARM Trusted
>>> +(firefly-rk3399) Firmware and build with BL31=/path/to/bl31.bin
>>> +(firefly-rk3399) Image 'simple-bin' is missing optional external blobs but is still functional: tee-os
>>> +(firefly-rk3399) /binman/simple-bin/fit/images/@tee-SEQ/tee-os (tee-os):
>>> +(firefly-rk3399) See the documentation for your board. You may need to build Open Portable
>>> +(firefly-rk3399) Trusted Execution Environment (OP-TEE) and build with TEE=/path/to/tee.bin
>>> +(firefly-rk3399) Some images are invalid
>>> 02: cmd: nvedit: Use getopt() in env export
>>> aarch64: (for 1/1 boards) all +883.0 rodata +199.0 text +684.0
>>> firefly-rk3399 : all +883 rodata +199 text +684
>>> u-boot: add: 5/0, grow: 0/-2 bytes: 1184/-500 (684)
>>> function old new delta
>>> __getopt - 520 +520
>>> static.bdinfo_print_all - 324 +324
>>> print_eth - 236 +236
>>> print_bi_dram - 92 +92
>>> getopt_init_state - 12 +12
>>> do_env_export 548 476 -72
>>> do_bdinfo 588 160 -428
>>> 03: cmd: nvedit: Use getopt() in env import
>>> aarch64: (for 1/1 boards) all -27.0 rodata -55.0 text +28.0
>>> firefly-rk3399 : all -27 rodata -55 text +28
>>> u-boot: add: 0/0, grow: 1/0 bytes: 28/0 (28)
>>> function old new delta
>>> do_env_import 668 696 +28
>>>
>>> And so this is why I'm just deferring this until someone has the time to
>>> pick up and address the underlying problems with this potential
>>> migration that have been raised in the previous iterations.
>>>
>>
>> Chiming in because I was put in Cc...
>>
>> Do I understand correctly that this only impacts U-Boot proper? If so, is a
>> size increase in proper that much of a blocker? As opposed to xPL which
>> usually resides in SRAM, I'm assuming U-Boot proper would run in RAM where I
>> hope an additional ~700B shouldn't be too much to ask for? The issue however
>> could be on the storage medium where U-Boot proper is located, maybe this
>> gets the binary above the max space allowed for U-Boot proper to be stored
>> (e.g. on some of my boards I'm (self-imposed) limited to 2000KiB) though I'm
>> assuming it could be less than 700B if U-Boot proper is compressed for
>> example. But I'm also lucky enough to have storage space and (D)RAM to spare
>> on my boards.
>>
>> Out of the RFC, I see the strlower() part as being separate from the rest,
>> maybe that's something we could revisit separately? Reading the cover
>> letter, we should even spare 60B with that alone, with the added benefit of
>> easier maintainability?
>
> This was just impacting U-Boot proper. However, it showed I think two
> sets of problems. The first of which is that there was an expectation
> that switching the code should result in some sort of savings,
> somewhere. And it didn't. And we do have boards where the space between
> the end of U-Boot and the start of the environment is small and so
> global changes that aren't just bug fixes need careful consideration
> (and they've already turned off things that aren't needed for them such
> as EFI loader). The second thing was that the code wasn't really easier
> to read / follow. So I don't object to the concept but I think the
> outcome of this series was that our getopt needs to be looked at in a
> bit more detail, and figure out if it's really designed the way we need
> it.
>
Understood, thanks for the additional info.
Cheers,
Quentin
More information about the U-Boot
mailing list