[Boost-bugs] [Boost C++ Libraries] #4469: Bugs in program_options implementation

Subject: [Boost-bugs] [Boost C++ Libraries] #4469: Bugs in program_options implementation
From: Boost C++ Libraries (noreply_at_[hidden])
Date: 2010-07-26 15:34:10

#4469: Bugs in program_options implementation
----------------------------------+-----------------------------------------
 Reporter:  lodos@…               |       Owner:  vladimir_prus  
     Type:  Bugs                  |      Status:  new            
Milestone:  Boost 1.44.0          |   Component:  program_options
  Version:  Boost 1.44.0          |    Severity:  Problem        
 Keywords:                        |  
----------------------------------+-----------------------------------------
 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.

-- 
Ticket URL: <https://svn.boost.org/trac/boost/ticket/4469>
Boost C++ Libraries <http://www.boost.org/>
Boost provides free peer-reviewed portable C++ source libraries.