[noup] zephyr: unguard wpa_cli_cmds of CONFIG_WPA_CLI#145
Conversation
wpa_cli_cmds is the communication way between wifi_mgmt and hostap in zephyr. So many features depends on it. And itself has very low cost. Remove the CONFIG_WPA_CLI guard of wpa_cli_cmds and keep CONFIG_WPA_CLI to controll cmd_wpa_cli shell commands. Signed-off-by: Fengming Ye <frank.ye@nxp.com>
I don't see this change here? Is this missed? |
it is kept unchanged in CMakeLists |
Ah ok, I was confused by the statement, I understand now, you want all commands to be available and only the top shell command to be guarded by the |
Can you please share the footprint numbers without and with this PR? |
With PR and CONFIG_WPA_CLI disabled Without PR and CONFIG_WPA_CLI disabled There is 14KB flash increase due to command help context. |
krish2718
left a comment
krish2718
left a comment
There was a problem hiding this comment.
There is 14KB flash increase due to command help context.
Thanks for the data. To be precise: that's 1,191,872 − 1,176,644 = 15,228 bytes, or 14.87 KiB — so let's call it ~15KB rather than 14KB.
For context, that's being added to a path that's supposed to be inert when CONFIG_WPA_CLI is disabled. Many of our target platforms (e.g. nRF series) ship with 1MB or less of flash, where 15KB is not a rounding error — it's a meaningful chunk of the available budget, and it's being spent on something the disabled config shouldn't be paying for at all.
The stated goal here ("keep CONFIG_WPA_CLI to control cmd_wpa_cli shell commands") suggests the shell command itself is still gated, but the help/usage strings tied to the wpa_cli_cmds table are apparently being pulled in unconditionally now, which is where this cost is coming from. That's the part I'd push back on: unguarding the command dispatch table to fix the "unknown command" issue shouldn't require unguarding the help text along with it.
I'd like to see a version of this fix where the command structs/dispatch can be unguarded (to solve the actual bug you're fixing), but the help strings stay behind CONFIG_WPA_CLI — e.g. via a separate guarded array, __weak stub, or conditional compilation around just the string tables. If that's not feasible for some reason, I'd want to understand why before accepting a ~15KB unconditional cost across all production builds.
Holding off on merging until we have a version that doesn't carry this cost when the feature is off. Happy to discuss alternatives if there's a constraint here I'm missing.
3 participants
wpa_cli_cmds is the communication way between wifi_mgmt and hostap in zephyr. So many features depends on it. And itself has very low cost.
Remove the CONFIG_WPA_CLI guard of wpa_cli_cmds and keep CONFIG_WPA_CLI to controll cmd_wpa_cli shell commands.