BIND 10 #1113: RR type implementation: MINFO

[BIND 10 Development]

#1113: RR type implementation: MINFO
-------------------------------------+-------------------------------------
                   Reporter:  shane  |                 Owner:  zzchen_pku
                       Type:         |                Status:  reviewing
  enhancement                        |             Milestone:
                   Priority:  minor  |  Sprint-20110816
                  Component:         |            Resolution:
  libdns++                           |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  0.0
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by zzchen_pku):

 Replying to [comment:11 jinmei]:
 > The latest version basically looks fine and ready for merge.
 >
 > I have some minor additional comments though.  I don't think we need
 > another cycle of review for this, so please update the code
 > accordingly and then merge it.
 >
 > - Unfortunately this comment is not 100% correct:
 > {{{
 > +    /// This method never throws an exception.
 > +    MINFO& operator=(const MINFO& source);
 > }}}
 >   because coping a name may result in std::bad_alloc.  I personally
 >   even note this obvious, but others seem to rather ignore this level
 >   of exceptions in documentation.  Either is okay for me, but at least
 >   "never throws" is not correct and should be fixed.
 >
 > - I'd remove the comment line because it now has MINFO specific code
 >   in it (although technically it may not be considered
 >   "specialization"):
 > {{{
 > +class Rdata_MINFO_Test : public RdataTest {
 > +    // there's nothing to specialize
 > }}}
 >
 > - The assignment test: this is fine, although it's probably too much
 >   because the implementation is a trivial copy (rather than using
 >   pimpl which requires a bit more care).  But I'm okay with keeping
 >   the current test case.
 >
 > - rdata_minfo_toWireUncompressed2.spec: the comment doesn't seem to be
 >   correct (it customizes the rmailbox).  Ideally it describes the
 >   point of the test correctly.  Or at least the incorrect part of the
 >   comment should be removed.
 Fixed.
 > > > The copy constructor still isn't tested.  Also, when we want to
 define
 > > > the copy constructor we should normally also define the assignment
 > > > operator.  Why don't you omit the latter?
 > > Added assignment operator.
 > > As fast as I know, the default copy constructor and assginment
 operator should be enough for MINFO,  but it seems gen-rdatacode.py.in
 requires all rdata types to define their own copy constructor ,don't know
 the reason.
 >
 > This is not a requirement of gen-rdatacode.  It's a consequence of the
 > fact that the base class Rdata declares the copy constructor and
 > operator= as private methods (to make it non copyable, which is for
 > avoiding unexpected slicing).  Even if they are private in terms of
 > accessibility, the compiler finds it when it tries to resolve the
 > definition in a derived class and then throw a compilation error due
 > to the accessibility restriction.  So, admittedly it's a bit
 > inconvenient, we always have to define these methods in derived
 > classes explicitly.
 Yeah, got it, thanks.

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