-
Notifications
You must be signed in to change notification settings - Fork 3.1k
lib/libc: fix bug in parsing in getopt #1918
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Thank you for taking the time to contribute to FreeBSD! |
|
This probably needs to be backported, but as this is my first contribution I am not sure about the proper workflow on this._ |
jlduran
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should manually pass the following three tests (at least):
$ ./getoptest -l -w -d14 -s /usr/local/admin/tftproot -u admin -U 0
ch is l optarg is NULL
ch is w optarg is NULL
ch is d optarg is 14
ch is s optarg is /usr/local/admin/tftproot
ch is u optarg is admin
ch is U optarg is 0
$ ./getoptest -l -w -d 14 -s /usr/local/admin/tftproot -u admin -U 0
ch is l optarg is NULL
ch is w optarg is NULL
ch is d optarg is 14 # XXX <- (Or is it NULL? I'll have to test on Linux)
ch is s optarg is /usr/local/admin/tftproot
ch is u optarg is admin
ch is U optarg is 0
and
$ ./getoptest -l -w -d -s /usr/local/admin/tftproot -u admin -U 0
ch is l optarg is NULL
ch is w optarg is NULL
ch is d optarg is NULL
ch is s optarg is /usr/local/admin/tftproot
ch is u optarg is admin
ch is U optarg is 0
lib/libc/stdlib/getopt.c
Outdated
| if (*place) | ||
| optarg = place; | ||
| else if (oli[2] == ':') | ||
| else if (oli[2] == ':') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The style(9) fixes I was referring to in the bug report should be addressed, maybe using NetBSD style guide (https://cvsweb.netbsd.org/bsdweb.cgi/src/share/misc/style?rev=HEAD&content-type=text/x-cvsweb-markup).
Basically, use tabs, not spaces for indentation, and braces around single-line bodies are optional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review. I fixed the tabs. Not sure on the braces. It is a "single instruction" but it is another nested if/else. Not contesting any of it, just unsure which one would be preferred.
As for the tests:
rootnode@x220 ~/c/c/291374> ./getopttest -l -w -d14 -s /usr/local/admin/tftproot -u admin -U 0
ch is l optarg is NULL
ch is w optarg is NULL
ch is d optarg is 14
ch is s optarg is /usr/local/admin/tftproot
ch is u optarg is admin
ch is U optarg is 0
rootnode@x220 ~/c/c/291374> ./getopttest -l -w -d 14 -s /usr/local/admin/tftproot -u admin -U 0
ch is l optarg is NULL
ch is w optarg is NULL
ch is d optarg is 14
ch is s optarg is /usr/local/admin/tftproot
ch is u optarg is admin
ch is U optarg is 0
rootnode@x220 ~/c/c/291374> ./getopttest -l -w -d -s /usr/local/admin/tftproot -u admin -U 0
ch is l optarg is NULL
ch is w optarg is NULL
ch is d optarg is NULL
ch is s optarg is /usr/local/admin/tftproot
ch is u optarg is admin
ch is U optarg is 0
getopt had an issue when parsing optional arguments if they had a space between the opion and its value. Request: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=291374 Signed-off-by: Simon Wollwage <[email protected]>
getopt had an issue when parsing optional arguments if they had a space between the opion and its value.
Request: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=291374
Tested with
bricoler run freebsd-src-regression-suite