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