search

zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.75k stars 6.56k forks source link

wifi: hostap: A better way to issue CLI commands #79811

Open krish2718 opened 1 week ago

krish2718 commented 1 week ago

Is your enhancement proposal related to a problem? Please describe.

https://github.com/zephyrproject-rtos/zephyr/blob/main/modules/hostap/src/supp_api.c is a wrapper around wpa_cli or hostapd_cli, before https://github.com/zephyrproject-rtos/zephyr/commit/6fdb6d5169f7d45ac28a6d12c81eda1b99e89bc6 the code was simple and readable as both the invocation and error handling was handled using a macro with flow-control statements, but as that isn't allowed by default in Zephyr, the commit removes the flow control from the macro, IMHO now code lost it's readability.

Describe the solution you'd like This is a contentious topic but we should make an exception (add to checkpatch ignore) in this case and allow code with flow control. I understand that hiding the branching isn't great, but it is a common technique used in OSS, e.g., https://elixir.bootlin.com/linux/v6.11.3/source/net/mac80211/tx.c#L1809 it nicely hides the logic esp. in this case as the entire file relies on this.

Describe alternatives you've considered We could leave it as is or switch from macro to function that can take an array of strings?

Additional context Improving readability and simplifying maintenance is well worth the discussion and effort.

jukkar commented 1 week ago

I agree, the older code is a bit more readable. I think the compliance check is done like that because of a MISRA rule. I think we could do things without a goto by having an outer do { .... } while(0); loop and breaking from it if there is an error. This way we probably do not need to have an exception (unless the break statement is also prohibited). I could experiment with this a bit.