BIND 10 #2497: introduce wrapper version of "from lexer" rdata factory
BIND 10 Development
do-not-reply at isc.org
Mon Dec 3 11:46:08 UTC 2012
#2497: introduce wrapper version of "from lexer" rdata factory
-------------------------------------+-------------------------------------
Reporter: jinmei | Owner:
Type: task | jinmei
Priority: medium | Status:
Component: libdns++ | reviewing
Keywords: | Milestone:
Sensitive: 0 | Sprint-20121204
Sub-Project: DNS | Resolution:
Estimated Difficulty: 0 | CVSS Scoring:
Total Hours: 0 | Defect Severity: N/A
| Feature Depending on Ticket:
| loadzone-ng
| Add Hours to Ticket: 0
| Internal?: 0
-------------------------------------+-------------------------------------
Changes (by muks):
* owner: muks => jinmei
Comment:
Hi Jinmei
Replying to [comment:17 jinmei]:
> '''gen-rdatacode.py.in'''
>
> - `new_rdatafactory_users` must be able to work pairs of RR type and
> RR class, not only for types (e.g. A/IN and A/CH are different types).
> - Also, please add a comment on what should be added
`new_rdatafactory_users`
> to with an example:
> {{{#!python
> new_rdatafactory_users = []
> }}}
> No one except the original author can figure out what should be
> added to the list when a new type is supported unless they bother to
> read the code to understand how this list is used.
* Comments were added
* The list takes tuples of (rrtype, rrclass) now.
> '''rrparamregistry-placeholder.cc'''
>
> - not really for this particular task, but the repeated patter of
> code to identify the factory now look redundant duplicate:
> {{{#!cpp
> RdataFactoryMap::const_iterator found =
> impl_->rdata_factories.find(RRTypeClass(rrtype, rrclass));
> if (found != impl_->rdata_factories.end()) {
> return (found->second->create(PARAMETERS));
> }
>
> GenericRdataFactoryMap::const_iterator genfound =
> impl_->genericrdata_factories.find(rrtype);
> if (genfound != impl_->genericrdata_factories.end()) {
> return (genfound->second->create(PARAMETERS));
> }
> }}}
> I'm attaching a proposed diff to unify these.
Patch looks good. I've applied it.
> '''rdata_hinfo_unittest.cc'''
>
> - I suspect this is not a good example of bad input:
> {{{
> + // Exceptions cause NULL to be returned.
> + EXPECT_FALSE(test::createRdataUsingLexer(RRType::HINFO(),
RRClass::IN(),
> + "\"Pentium\"\"Linux\""));
> }}}
> With the generic lexer it would become valid (and BIND 9 would
> accept it too)
I don't follow this. I can't think of another way to get an invalid HINFO
without similar syntax.
> '''rdata_txt_like_unittest.cc'''
> - I'd remove these cases:
> {{{#!cpp
> EXPECT_EQ(0, this->rdata_txt_like.compare(
> *test::createRdataUsingLexer(RRTYPE<TypeParam>(), RRClass::IN(),
> "Test String")));
> EXPECT_EQ(0, this->rdata_txt_like_empty.compare(
> *test::createRdataUsingLexer(RRTYPE<TypeParam>(), RRClass::IN(),
> "")));
> }}}
> they will soon result in different results with the generic lexer.
These tests have been removed now.
Also there's a commit pushed to `master` branch (`403af33`, reviewed by
vorner) which generates `gen-rdatacode.py` from the `gen-rdatacode.py.in`
file.
--
Ticket URL: <http://bind10.isc.org/ticket/2497#comment:20>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development
More information about the bind10-tickets
mailing list