BIND 10 #1611: ZoneFinder::find refactoring: separate normal result codes and special case flags

BIND 10 Development do-not-reply at isc.org
Wed Jan 25 14:19:15 UTC 2012


#1611: ZoneFinder::find refactoring: separate normal result codes and special case
flags
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20120207
                  Component:  data   |            Resolution:
  source                             |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  4
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 First, is it OK that we're kind of inconsistent with python? When we had
 the previous version of FindResult, it acted mostly as a tuple. But now
 the C++ version has methods, while the python one has item in the tuple.
 But it is probably the easiest way to implement it.

 Is the removal of `ZoneFinder::` from the front of FindResult returns in
 database.cc an unrelated cleanup or does it have any meaning in the scope
 of the ticket?

 Is this missing the NSEC3 flag?

 {{{#!c++
 // For wildcard case with DNSSEC required, the caller would need to know
 // whether it's NSEC or NSEC3 signed.  So we need to do an additional
 // search here, even though the NSEC RR may not be returned.
 if (wild && (options & FIND_DNSSEC) != 0 &&
     found.second.find(RRType::NSEC()) != found.second.end()) {
     flags = flags | RESULT_NSEC_SIGNED;
 }
 }}}

 This is not true in the python part, as it returns a tuple, not an object,
 so it has no methods:
 {{{
 The returned FindResult object can also provide supplemental\n\
 information about the search result via its methods returning a\n\
 boolean value. Such information may be useful for the caller if the\n\
 caller wants to collect additional DNSSEC proofs based on the search\n\
 result.\n\
 }}}

 Why is this change included here? Because it is wrong, the python find_all
 doesn't have a target parameter (because return parameters are very
 unpythonish).
 {{{#!diff
  Parameters:\n\
 -  name       The domain name to be searched for.\n\
 +  target     the successfull result is returned through this\n\
    options    The search options.\n\
 }}}

 This is missing one combination (if there are 3 items, we can as well
 check comparing all of them):
 {{{#!python
 def test_findresultflags(self):
     self.assertNotEqual(ZoneFinder.RESULT_WILDCARD,
                         ZoneFinder.RESULT_NSEC_SIGNED)
     self.assertNotEqual(ZoneFinder.RESULT_NSEC_SIGNED,
                         ZoneFinder.RESULT_NSEC3_SIGNED)
 }}}

 Should this be checked for equality, so no extra flag is returned
 (multiple occurrences)
 {{{#!diff
  self.assertEqual(finder.SUCCESS, result)
 +self.assertTrue((flags & finder.RESULT_WILDCARD) != 0)
  self.assertEqual("foo.wild.example.com. 3600 IN A 192.0.2.255\n",
 }}}

-- 
Ticket URL: <http://bind10.isc.org/ticket/1611#comment:6>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development


More information about the bind10-tickets mailing list