Bugs in program_options implementation

[lodos@…]

The function options_description::find_nothrow throws an exception when there are at least 2 exact matches. Since this function is in the public interface this could be considered a bug (it must not throw). Please note that this function is used several times within the implementation without try clauses. The exception thrown might be being used by callers of the method find, so you cannot just remove it. It is best to implement find and find_nothrow as follows:

const option_description& options_description::find(const std::string& name,

bool approx, bool long_ignore_case, bool short_ignore_case) const

{

shared_ptr<option_description> found; vector<string> approximate_matches; vector<string> full_matches;

We use linear search because matching specified option name with the declared option name need to take care about case sensitivity and trailing '*' and so we can't use simple map. for(unsigned i = 0; i < m_options.size(); ++i) {

option_description::match_result r =

m_options[i]->match(name, approx, long_ignore_case, short_ignore_case);

if (r == option_description::no_match)

continue;

if (r == option_description::full_match) {

full_matches.push_back(m_options[i]->key(name)); found = m_options[i];

} else {

FIXME: the use of 'key' here might not be the best approach. approximate_matches.push_back(m_options[i]->key(name)); if (!found)

found = m_options[i];

}

} if (full_matches.size() > 1)

boost::throw_exception(

ambiguous_option(name, full_matches));

If we have a full match, and an approximate match, ignore approximate match instead of reporting error. Say, if we have options "all" and "all-chroots", then "--all" on the command line should select the first one, without ambiguity. if (full_matches.empty() && approximate_matches.size() > 1)

boost::throw_exception(

ambiguous_option(name, approximate_matches));

if (!found)

boost::throw_exception(unknown_option(name));

return *found;

}

const option_description* options_description::find_nothrow(const std::string& name,

bool approx, bool long_ignore_case, bool short_ignore_case) const

{

try {

const option_description& d = find(name, approx,

long_ignore_case, short_ignore_case);

return &d;

} catch(const std::exception&) {

return NULL;

}

}

This modification uncovers a bug in function cmdline::finish_option (cmdline.cpp) where find_nothrow is being called assuming that no exception will be thrown. The first part of this function might be modified to:

void cmdline::finish_option(option& opt,

vector<string>& other_tokens, const vector<style_parser>& style_parsers)

{

if (opt.string_key.empty())

return;

const option_description* xd; try {

First check that the option is valid, and get its description. xd = &m_desc->find(opt.string_key,

is_style_active(allow_guessing), is_style_active(long_case_insensitive), is_style_active(short_case_insensitive));

} catch(const std::exception&) {

if (m_allow_unregistered) {

opt.unregistered = true; return;

} throw;

} const option_description& d = *xd;

Canonize the name opt.string_key = d.key(opt.string_key);

........

}

Now it can throw the correct exception thrown by the find method. This is the only way callers could know the correct exception. It was apparently working before because find_nothrow was throwing exceptions not captured within cmdline::finish_option. You may chose to eliminate the xd variable altogether and initialize and use d within the try clause. What is important is to call find instead of find_nothrow.

The following test would have detected these problems and the one solved in ​https://svn.boost.org/trac/boost/ticket/4159:

Multiple arguments

test_case test_cases2[] = {

{"--fname file --fname2 file2", s_success, "fname: file fname2: file2"}, {"--fnam file --fnam file2", s_ambiguous_option, ""}, {"--fnam file --fname2 file2", s_ambiguous_option, ""}, {"--fname2 file2 --fnam file", s_ambiguous_option, ""}, {0, 0, 0}

}; test_cmdline("fname fname2", style, test_cases2);

This test may be added at the end of the test_guessing function in cmdline_test.cpp.

Thank you.

[Vladimir Prus]

(In [67773]) Add more testscases. Addresses #4469.

[Vladimir Prus]

Thank you for the additional testcases -- this is a rare and very valuable form of contribution. I've added that testcase -- which revealed that the implementation has been fixed already, presumably due to independent bug report about the same.