sys/shell: fix getopt() support#20209
Conversation
Fixed broken Markdown code spans. Also added a code span around mention of getopt.
This patch adds details to the rationale behind the design of the argv/argc handling for shell command handlers. It also fixes a misspelling.
This patch adds the missing NULL terminator to the argv passed to shell command handlers. Without it, Newlib's getop() was intermittently causing hardfaults. Closer inspection of NewLib's code revealed that it relies in this NULL termination. ANSI-C also requires it of the argv passed to main().
This patch updates shell.c's doc that undersells the shells complexity. The comment seems to have been written prior to the shell's ability to parse command args and handle quoting sequences.
benpicco
left a comment
There was a problem hiding this comment.
Thank you!
I wasn't even aware we had getopt available, this sure makes makes arg parsing easier.
The required changes are only minor too.
Always happy to contribute.
Yeah, I've been thrilled with using
:) Yep one of those problems that takes hours to figure out and 5 minutes to fix. To be honest, I thought that Newlib was to blame at first. Its getopt.c wasn't the cleanest in my opinion. Kind of amusing that I sometimes assume these well-known projects must be perfect. Then you go look, and realize it's authors are mere mortals too! |
Contribution description
This patch adds the missing NULL terminator to the argv passed to shell command handlers. Without it, Newlib's getop() was intermittently causing hardfaults. Closer inspection of NewLib's code revealed that it relies in this NULL termination. ANSI-C also requires it of the argv passed to main().
Also included are some minor improvements to the documentation.
Testing procedure
For me
getopt()no longer causes hardfaults. Also code in shell.c now does what the doc in shell.h advertises that it does.Issues/PRs references