From af78348d4a4b4a37daa91fa3a531bb00dfa1196f Mon Sep 17 00:00:00 2001 From: Alex Lyon Date: Sun, 18 Mar 2018 00:28:31 -0700 Subject: [PATCH 1/3] unistd: add a preliminary implementation of getopt() --- Cargo.lock | 2 + src/unistd/Cargo.toml | 2 + src/unistd/src/getopt.rs | 142 ++++++++++++++++++++++++++++ src/unistd/src/lib.rs | 10 +- tests/.gitignore | 1 + tests/Makefile | 1 + tests/expected/unistd/getopt.stderr | 0 tests/expected/unistd/getopt.stdout | 36 +++++++ tests/unistd/getopt.c | 68 +++++++++++++ 9 files changed, 257 insertions(+), 5 deletions(-) create mode 100644 src/unistd/src/getopt.rs create mode 100644 tests/expected/unistd/getopt.stderr create mode 100644 tests/expected/unistd/getopt.stdout create mode 100644 tests/unistd/getopt.c diff --git a/Cargo.lock b/Cargo.lock index b69becf544..70a043de61 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -534,6 +534,8 @@ version = "0.1.0" dependencies = [ "cbindgen 0.5.2", "platform 0.1.0", + "stdio 0.1.0", + "string 0.1.0", ] [[package]] diff --git a/src/unistd/Cargo.toml b/src/unistd/Cargo.toml index 3edd54a814..8de8a2b933 100644 --- a/src/unistd/Cargo.toml +++ b/src/unistd/Cargo.toml @@ -9,3 +9,5 @@ cbindgen = { path = "../../cbindgen" } [dependencies] platform = { path = "../platform" } +stdio = { path = "../stdio" } +string = { path = "../string" } diff --git a/src/unistd/src/getopt.rs b/src/unistd/src/getopt.rs new file mode 100644 index 0000000000..40560fae2b --- /dev/null +++ b/src/unistd/src/getopt.rs @@ -0,0 +1,142 @@ +//! getopt implementation for Redox, following http://pubs.opengroup.org/onlinepubs/009695399/functions/getopt.html + +use super::platform::types::*; +use super::platform; +use super::stdio; +use super::string; +use core::ptr; + +#[no_mangle] +pub static mut optarg: *mut c_char = ptr::null_mut(); + +#[no_mangle] +pub static mut optind: c_int = 1; + +#[no_mangle] +pub static mut opterr: c_int = 1; + +#[no_mangle] +pub static mut optopt: c_int = -1; + +static mut current_opt: *mut c_char = ptr::null_mut(); + +#[no_mangle] +pub unsafe extern "C" fn getopt( + argc: c_int, + argv: *const *mut c_char, + optstring: *const c_char, +) -> c_int { + if current_opt == ptr::null_mut() || *current_opt == 0 { + let current_arg = *argv.offset(optind as isize); + // XXX: is argc check needed? + if optind > argc || current_arg == ptr::null_mut() || *current_arg != b'-' as c_char + || string::strcmp(current_arg, b"-\0".as_ptr() as _) == 0 + { + -1 + } else if string::strcmp(current_arg, b"--\0".as_ptr() as _) == 0 { + optind += 1; + -1 + } else { + // remove the '-' + let current_arg = current_arg.offset(1); + + parse_arg(argc, argv, current_arg, optstring) + } + } else { + parse_arg(argc, argv, current_opt, optstring) + } +} + +unsafe fn parse_arg( + argc: c_int, + argv: *const *mut c_char, + current_arg: *mut c_char, + optstring: *const c_char, +) -> c_int { + let update_current_opt = || { + current_opt = current_arg.offset(1); + if *current_opt == 0 { + optind += 1; + } + }; + + let print_error = |desc: &[u8]| { + // NOTE: we don't use fprintf to get around the usage of va_list + stdio::fputs(*argv as _, stdio::stderr); + stdio::fputs(desc.as_ptr() as _, stdio::stderr); + stdio::fputc(*current_arg as _, stdio::stderr); + stdio::fputc(b'\n' as _, stdio::stderr); + }; + + match find_option(*current_arg, optstring) { + Some(GetoptOption::Flag) => { + update_current_opt(); + + *current_arg as c_int + } + Some(GetoptOption::OptArg) => { + current_opt = b"\0".as_ptr() as _; + if *current_arg.offset(1) == 0 { + optind += 2; + if optind > argc { + current_opt = ptr::null_mut(); + + optopt = *current_arg as c_int; + let errch = if *optstring == b':' as c_char { + b':' + } else { + if opterr != 0 { + print_error(b": option requries an argument -- \0"); + } + + b'?' + }; + errch as c_int + } else { + optarg = *argv.offset(optind as isize - 1); + + *current_arg as c_int + } + } else { + optarg = current_arg.offset(1); + optind += 1; + + *current_arg as c_int + } + } + None => { + // couldn't find the given option in optstring + if opterr != 0 { + print_error(b": illegal option -- \0"); + } + + update_current_opt(); + + optopt = *current_arg as _; + b'?' as c_int + } + } +} + +enum GetoptOption { + Flag, + OptArg, +} + +unsafe fn find_option(ch: c_char, optstring: *const c_char) -> Option { + let mut i = 0; + + while *optstring.offset(i) != 0 { + if *optstring.offset(i) == ch { + let result = if *optstring.offset(i + 1) == b':' as c_char { + GetoptOption::OptArg + } else { + GetoptOption::Flag + }; + return Some(result); + } + i += 1; + } + + None +} diff --git a/src/unistd/src/lib.rs b/src/unistd/src/lib.rs index 7915a7e142..e5aa6bc926 100644 --- a/src/unistd/src/lib.rs +++ b/src/unistd/src/lib.rs @@ -3,10 +3,15 @@ #![no_std] extern crate platform; +extern crate stdio; +extern crate string; pub use platform::types::*; +pub use getopt::*; use core::ptr; +mod getopt; + pub const R_OK: c_int = 1; pub const W_OK: c_int = 2; pub const X_OK: c_int = 4; @@ -204,11 +209,6 @@ pub extern "C" fn getlogin_r(name: *mut c_char, namesize: size_t) -> c_int { unimplemented!(); } -#[no_mangle] -pub extern "C" fn getopt(argc: c_int, argv: *const *mut c_char, opstring: *const c_char) -> c_int { - unimplemented!(); -} - #[no_mangle] pub extern "C" fn getpagesize() -> c_int { unimplemented!(); diff --git a/tests/.gitignore b/tests/.gitignore index ff5cc28a8c..dceeae741b 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -33,6 +33,7 @@ /string/strchr /string/strrchr /string/strspn +/unistd/getopt /unlink /waitpid /write diff --git a/tests/Makefile b/tests/Makefile index e1b95b1839..52983f3f6a 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -32,6 +32,7 @@ EXPECT_BINS=\ string/strpbrk \ string/strtok \ string/strtok_r \ + unistd/getopt \ unlink \ waitpid \ write diff --git a/tests/expected/unistd/getopt.stderr b/tests/expected/unistd/getopt.stderr new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/expected/unistd/getopt.stdout b/tests/expected/unistd/getopt.stdout new file mode 100644 index 0000000000..c48147eb6f --- /dev/null +++ b/tests/expected/unistd/getopt.stdout @@ -0,0 +1,36 @@ +bflg: 0 +aflg: 1 +errflg: 0 +ifile: +ofile: arg +result: 0 +bflg: 0 +aflg: 1 +errflg: 0 +ifile: +ofile: arg +result: 0 +bflg: 0 +aflg: 1 +errflg: 0 +ifile: +ofile: arg +result: 0 +bflg: 0 +aflg: 1 +errflg: 0 +ifile: +ofile: arg +result: 0 +bflg: 0 +aflg: 1 +errflg: 0 +ifile: +ofile: arg +result: 0 +bflg: 0 +aflg: 1 +errflg: 0 +ifile: +ofile: arg +result: 0 diff --git a/tests/unistd/getopt.c b/tests/unistd/getopt.c new file mode 100644 index 0000000000..f8e95ac955 --- /dev/null +++ b/tests/unistd/getopt.c @@ -0,0 +1,68 @@ +#include +#include + +#define RUN(...) \ + { \ + optind = 1; \ + optarg = NULL; \ + opterr = 1; \ + optopt = -1; \ + char *args_arr[] = { __VA_ARGS__ }; \ + printf("result: %d\n", runner(sizeof(args_arr) / sizeof(args_arr[0]), args_arr)); \ + } + +int runner(int argc, const char *argv[]) { + int c; + int bflg = 0, aflg = 0, errflg = 0; + char *ifile = ""; + char *ofile = ""; + + while((c = getopt(argc, argv, ":abf:o:")) != -1) { + switch(c) { + case 'a': + if(bflg) + errflg++; + else + aflg++; + break; + case 'b': + if(aflg) + errflg++; + else + bflg++; + break; + case 'f': + ifile = optarg; + break; + case 'o': + ofile = optarg; + break; + case ':': + printf("Option -%c requires an operand\n", optopt); + errflg++; + break; + case '?': + printf("Unrecognized option: -%c\n", optopt); + errflg++; + } + } + printf("bflg: %d\n", bflg); + printf("aflg: %d\n", aflg); + printf("errflg: %d\n", errflg); + printf("ifile: %s\n", ifile); + printf("ofile: %s\n", ofile); + if(errflg) { + printf("Usage: info goes here\n"); + return 2; + } + return 0; +} + +int main(int argc, const char *argv[]) { + RUN("test", "-ao", "arg", "path", "path"); + RUN("test", "-a", "-o", "arg", "path", "path"); + RUN("test", "-o", "arg", "-a", "path", "path"); + RUN("test", "-a", "-o", "arg", "--", "path", "path"); + RUN("test", "-a", "-oarg", "path", "path"); + RUN("test", "-aoarg", "path", "path"); +} From 42a6693a0bd283d4c2eacb2c3c096d0903c1f45a Mon Sep 17 00:00:00 2001 From: Alex Lyon Date: Mon, 19 Mar 2018 14:48:38 -0700 Subject: [PATCH 2/3] unistd: fix off-by-one in getopt() --- src/unistd/src/getopt.rs | 43 +++++++++++++++++------------ tests/expected/unistd/getopt.stdout | 6 ++++ tests/unistd/getopt.c | 3 +- 3 files changed, 33 insertions(+), 19 deletions(-) diff --git a/src/unistd/src/getopt.rs b/src/unistd/src/getopt.rs index 40560fae2b..0f8e538f20 100644 --- a/src/unistd/src/getopt.rs +++ b/src/unistd/src/getopt.rs @@ -6,19 +6,23 @@ use super::stdio; use super::string; use core::ptr; +#[allow(non_upper_case_globals)] #[no_mangle] pub static mut optarg: *mut c_char = ptr::null_mut(); +#[allow(non_upper_case_globals)] #[no_mangle] pub static mut optind: c_int = 1; +#[allow(non_upper_case_globals)] #[no_mangle] pub static mut opterr: c_int = 1; +#[allow(non_upper_case_globals)] #[no_mangle] pub static mut optopt: c_int = -1; -static mut current_opt: *mut c_char = ptr::null_mut(); +static mut CURRENT_OPT: *mut c_char = ptr::null_mut(); #[no_mangle] pub unsafe extern "C" fn getopt( @@ -26,24 +30,27 @@ pub unsafe extern "C" fn getopt( argv: *const *mut c_char, optstring: *const c_char, ) -> c_int { - if current_opt == ptr::null_mut() || *current_opt == 0 { - let current_arg = *argv.offset(optind as isize); - // XXX: is argc check needed? - if optind > argc || current_arg == ptr::null_mut() || *current_arg != b'-' as c_char - || string::strcmp(current_arg, b"-\0".as_ptr() as _) == 0 - { - -1 - } else if string::strcmp(current_arg, b"--\0".as_ptr() as _) == 0 { - optind += 1; + if CURRENT_OPT == ptr::null_mut() || *CURRENT_OPT == 0 { + if optind >= argc { -1 } else { - // remove the '-' - let current_arg = current_arg.offset(1); + let current_arg = *argv.offset(optind as isize); + if current_arg == ptr::null_mut() || *current_arg != b'-' as c_char + || string::strcmp(current_arg, b"-\0".as_ptr() as _) == 0 + { + -1 + } else if string::strcmp(current_arg, b"--\0".as_ptr() as _) == 0 { + optind += 1; + -1 + } else { + // remove the '-' + let current_arg = current_arg.offset(1); - parse_arg(argc, argv, current_arg, optstring) + parse_arg(argc, argv, current_arg, optstring) + } } } else { - parse_arg(argc, argv, current_opt, optstring) + parse_arg(argc, argv, CURRENT_OPT, optstring) } } @@ -54,8 +61,8 @@ unsafe fn parse_arg( optstring: *const c_char, ) -> c_int { let update_current_opt = || { - current_opt = current_arg.offset(1); - if *current_opt == 0 { + CURRENT_OPT = current_arg.offset(1); + if *CURRENT_OPT == 0 { optind += 1; } }; @@ -75,11 +82,11 @@ unsafe fn parse_arg( *current_arg as c_int } Some(GetoptOption::OptArg) => { - current_opt = b"\0".as_ptr() as _; + CURRENT_OPT = b"\0".as_ptr() as _; if *current_arg.offset(1) == 0 { optind += 2; if optind > argc { - current_opt = ptr::null_mut(); + CURRENT_OPT = ptr::null_mut(); optopt = *current_arg as c_int; let errch = if *optstring == b':' as c_char { diff --git a/tests/expected/unistd/getopt.stdout b/tests/expected/unistd/getopt.stdout index c48147eb6f..aba2d44d3f 100644 --- a/tests/expected/unistd/getopt.stdout +++ b/tests/expected/unistd/getopt.stdout @@ -34,3 +34,9 @@ errflg: 0 ifile: ofile: arg result: 0 +bflg: 0 +aflg: 0 +errflg: 0 +ifile: +ofile: +result: 0 diff --git a/tests/unistd/getopt.c b/tests/unistd/getopt.c index f8e95ac955..b1eb262556 100644 --- a/tests/unistd/getopt.c +++ b/tests/unistd/getopt.c @@ -11,7 +11,7 @@ printf("result: %d\n", runner(sizeof(args_arr) / sizeof(args_arr[0]), args_arr)); \ } -int runner(int argc, const char *argv[]) { +int runner(int argc, char *argv[]) { int c; int bflg = 0, aflg = 0, errflg = 0; char *ifile = ""; @@ -65,4 +65,5 @@ int main(int argc, const char *argv[]) { RUN("test", "-a", "-o", "arg", "--", "path", "path"); RUN("test", "-a", "-oarg", "path", "path"); RUN("test", "-aoarg", "path", "path"); + RUN("test"); } From 2751d457bf815333b2ba8f96b544d1fe5aa49b83 Mon Sep 17 00:00:00 2001 From: Alex Lyon Date: Mon, 19 Mar 2018 15:05:38 -0700 Subject: [PATCH 3/3] unistd: use .is_null() for pointers --- src/unistd/src/getopt.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/unistd/src/getopt.rs b/src/unistd/src/getopt.rs index 0f8e538f20..2790529a35 100644 --- a/src/unistd/src/getopt.rs +++ b/src/unistd/src/getopt.rs @@ -30,12 +30,12 @@ pub unsafe extern "C" fn getopt( argv: *const *mut c_char, optstring: *const c_char, ) -> c_int { - if CURRENT_OPT == ptr::null_mut() || *CURRENT_OPT == 0 { + if CURRENT_OPT.is_null() || *CURRENT_OPT == 0 { if optind >= argc { -1 } else { let current_arg = *argv.offset(optind as isize); - if current_arg == ptr::null_mut() || *current_arg != b'-' as c_char + if current_arg.is_null() || *current_arg != b'-' as c_char || string::strcmp(current_arg, b"-\0".as_ptr() as _) == 0 { -1