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