Skip to content

system/nxinit: increase SERVICE_ARGS_MAX default and add 'set' builtin - #3758

Merged
acassis merged 2 commits into
apache:masterfrom
JianyuWang0623:nxinit-args-max-and-set-builtin
Aug 28, 2026
Merged

system/nxinit: increase SERVICE_ARGS_MAX default and add 'set' builtin#3758
acassis merged 2 commits into
apache:masterfrom
JianyuWang0623:nxinit-args-max-and-set-builtin

Conversation

@JianyuWang0623

@JianyuWang0623 JianyuWang0623 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Two independent, low-risk fixes carried over from the internal (vela) fork of this component:

  1. Increase SYSTEM_NXINIT_SERVICE_ARGS_MAX default from 8 to 16. The previous default is insufficient for services with many arguments (e.g. ptpd needs 10 argv slots). When exceeded, argv lacks a NULL terminator, causing posix_spawnp to read out of bounds.
  2. Add a set KEY VALUE builtin command. Previously set in init.rc fell through to posix_spawnp(), which ran it in a temporary child shell — the environment variable was set only in that child and lost when it exited, so later services never inherited it. cmd_set now calls setenv() directly in the init process, guarded by CONFIG_DISABLE_ENVIRON, so child processes (which inherit init's environment) see it correctly.

Impact

  • system/nxinit/Kconfig: change default range value, no new config surface.
  • system/nxinit/builtin.c: add one builtin command entry, no changes to existing command behavior.

Testing

Built for esp32p4-function-ev-board:nsh (CONFIG_SYSTEM_NXINIT=y) with the actual toolchain (riscv-none-elf-gcc):

$ make -j$(nproc)
...
LD:  nuttx
MKIMAGE: NuttX binary
Generated: nuttx.bin

Verified via nm nuttx that cmd_set is linked into the image, and confirmed CONFIG_SYSTEM_NXINIT_SERVICE_ARGS_MAX=16 takes effect in the generated .config.

Ran nxstyle on the changed file (builtin.c) with no violations.

Runtime verification (esp32p4-pico-wifi-wareshare, MAC 80:f1:b2:d0:84:c4, /dev/ttyACM3)

The lckfb-szpi-esp32s3 board normally used for hardware verification was not reachable on any /dev/ttyACM*//dev/ttyUSB* this time (esptool.py read_mac failed on all of them, likely not connected). Both changes in this PR live in apps/system/nxinit and are board-agnostic, so an esp32p4 board was used as an equivalent target.

Flashed the branch build and captured the nsh serial console:

$ esptool.py -c esp32p4 -p /dev/ttyACM3 -b 921600 write_flash 0x2000 nuttx.bin
...
Wrote 281368 bytes (139619 compressed) at 0x00002000 in 3.1 seconds...
Hash of data verified.
Hard resetting via RTS pin...
nsh> ps
  TID   PID  PPID PRI POLICY   TYPE    NPX STATE    EVENT     SIGMASK            STACK COMMAND
    0     0     0   0 FIFO     Kthread   - Ready              0000000000000000 0002016 Idle_Task
    2     2     0 100 RR       Task      - Waiting  Semaphore 0000000000000000 0008120 init_main
    3     3     2 100 RR       Task      - Running            0000000000000000 0008144 sh

init_main (nxinit, PID 2) is the parent of sh (PID 3), confirming nxinit is functioning correctly as system init with CONFIG_SYSTEM_NXINIT_SERVICE_ARGS_MAX=16 in the running firmware.

To exercise the new set builtin specifically, a temporary set NXINIT_SET_TEST hello_from_set_builtin line was added to init.rc's on init block (build/flash/verify only — reverted afterward, not part of this PR):

nsh> env
PWD=/
NXINIT_SET_TEST=hello_from_set_builtin

This confirms set KEY VALUE is parsed and executed from init.rc, setenv() takes effect in the init process, and the spawned child (sh) inherits the variable. After this check, the init.rc change was reverted (git status clean on both nuttx and apps repos) — no code in this PR was altered.

@JianyuWang0623
JianyuWang0623 marked this pull request as draft August 26, 2026 14:56
@JianyuWang0623
JianyuWang0623 marked this pull request as ready for review August 26, 2026 15:46
@JianyuWang0623

This comment has been minimized.

The previous default of 8 is insufficient for services with many
arguments (e.g. ptpd needs 10 argv slots). When exceeded, argv lacks
a NULL terminator, causing posix_spawnp to read out of bounds.

Increase default to 16 to prevent argument truncation for typical
daemon services.

Signed-off-by: wangjianyu3 <wangjianyu3@xiaomi.com>
Previously, 'set KEY VALUE' in init.rc was not recognized as a builtin
command. It fell through to posix_spawnp(), which ran it in a
temporary child shell. The environment variable was set only in the
child process and lost when it exited, so services started afterward
never inherited it.

Register cmd_set as an init builtin that calls setenv(key, value, 1)
directly in the init process. The command takes exactly 2 arguments
(key and value). All code is guarded by CONFIG_DISABLE_ENVIRON so it
compiles out when environment support is disabled.

Since child processes inherit init's environment, 'set TZ Asia/Shanghai'
in init.rc now correctly propagates to all subsequently started
services.

Signed-off-by: wangjianyu3 <wangjianyu3@xiaomi.com>
@JianyuWang0623
JianyuWang0623 force-pushed the nxinit-args-max-and-set-builtin branch from 9015fd8 to 2f4019d Compare August 27, 2026 12:11
@acassis
acassis merged commit 11c1c6b into apache:master Aug 28, 2026
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants