-- Nadav Har'El [email protected] On Thu, Dec 29, 2016 at 6:49 PM, Justin Cinkelj <[email protected]> wrote:
> parse_options both parses command line and executes required actions > (it sets environment etc). Move parsing code into parse_options_parse > function, so it can be reused in other places. > Since the actual options - and what you do to handle them, is not part of the "reusable" parse_options_parse, I didn't understand why this function is useful. It just calls the boost::program_options stuff, and you could call these functions directly from the other caller. Looks to me like you're saving 5 lines of code but at the same time complicating stuff. > > Also remove unused variable cmds from main_cont. > > Signed-off-by: Justin Cinkelj <[email protected]> > --- > loader.cc | 46 ++++++++++++++++++++++++++++------------------ > 1 file changed, 28 insertions(+), 18 deletions(-) > > diff --git a/loader.cc b/loader.cc > index 126a9f0..47a547f 100644 > --- a/loader.cc > +++ b/loader.cc > @@ -143,7 +143,9 @@ int maxnic; > static int sampler_frequency; > static bool opt_enable_sampler = false; > > -std::tuple<int, char**> parse_options(int ac, char** av) > +std::tuple<int, char**> parse_options_parse(int ac, char** av, > + const boost::program_options::options_description& desc, > + boost::program_options::variables_map& vars) > { > namespace bpo = boost::program_options; > namespace bpos = boost::program_options::command_line_style; > @@ -158,6 +160,28 @@ std::tuple<int, char**> parse_options(int ac, char** > av) > [](const char* arg) { return arg[0] != > '-'; }) - av; > std::copy(av, av + nr_options, std::back_inserter(args)); > > + // don't allow --foo bar (require --foo=bar) so we can find the first > non-option > + // argument > + int style = bpos::unix_style & ~(bpos::long_allow_next | > bpos::short_allow_next); > + try { > + bpo::store(bpo::parse_command_line(args.size(), args.data(), > desc, style), vars); > + } catch(std::exception &e) { > + std::cout << e.what() << '\n'; > + std::cout << desc << '\n'; > + osv::poweroff(); > + } > + bpo::notify(vars); > + > + av += nr_options; > + ac -= nr_options; > + return std::make_tuple(ac, av); > +} > + > +std::tuple<int, char**> parse_options(int ac, char** av) /**/ > +{ > + namespace bpo = boost::program_options; > + namespace bpos = boost::program_options::command_line_style; > + > bpo::options_description desc("OSv options"); > desc.add_options() > ("help", "show help text") > @@ -171,7 +195,7 @@ std::tuple<int, char**> parse_options(int ac, char** > av) > ("maxnic", bpo::value<int>(), "maximum NIC number") > ("norandom", "don't initialize any random device") > ("noshutdown", "continue running after main() returns") > - ("power-off-on-abort", "use poweroff instead of halt if it's > aborted") > + ("power-off-on-abort", "use poweroff instead of halt if it's > aborted") > Good change, but unrelated to this patch. > ("noinit", "don't run commands from /init") > ("verbose", "be verbose, print debug messages") > ("console", bpo::value<std::vector<std::string>>(), "select > console driver") > @@ -185,17 +209,7 @@ std::tuple<int, char**> parse_options(int ac, char** > av) > ("redirect", bpo::value<std::string>(), "redirect stdout and > stderr to file") > ; > bpo::variables_map vars; > - // don't allow --foo bar (require --foo=bar) so we can find the first > non-option > - // argument > - int style = bpos::unix_style & ~(bpos::long_allow_next | > bpos::short_allow_next); > - try { > - bpo::store(bpo::parse_command_line(args.size(), args.data(), > desc, style), vars); > - } catch(std::exception &e) { > - std::cout << e.what() << '\n'; > - std::cout << desc << '\n'; > - osv::poweroff(); > - } > - bpo::notify(vars); > + auto ac_av = parse_options_parse(ac, av, desc, vars); > > if (vars.count("help")) { > std::cout << desc << "\n"; > @@ -297,9 +311,7 @@ std::tuple<int, char**> parse_options(int ac, char** > av) > > boot_delay = std::chrono::duration_cast<std::chrono::nanoseconds>(1_s > * vars["delay"].as<float>()); > > - av += nr_options; > - ac -= nr_options; > - return std::make_tuple(ac, av); > + return ac_av; > } > > // return the std::string and the commands_args poiting to them as a move > @@ -536,8 +548,6 @@ void main_cont(int ac, char** av) > > elf::create_main_program(); > > - std::vector<std::vector<std::string> > cmds; > - > Good change, but unrelated tot his patch (as you said too). > std::tie(ac, av) = parse_options(ac, av); > > setenv("OSV_VERSION", osv::version().c_str(), 1); > -- > 2.9.3 > > -- > You received this message because you are subscribed to the Google Groups > "OSv Development" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "OSv Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
