BIND 10 #2095: Define and implement RdataEncoder class

[BIND 10 Development]

#2095: Define and implement RdataEncoder class
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20120807
  medium                             |            Resolution:
                  Component:  data   |             Sensitive:  0
  source                             |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  5
            Defect Severity:  N/A    |           Total Hours:  2.96
Feature Depending on Ticket:         |
  scalable inmemory                  |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:15 vorner]:

 > > I agree it's not very understandable.  In fact I didn't really like
 > > that implementation either.  On closer look I found it could be much
 > > more simplified, and I did it at commit 8560883.
 >
 > It is much better now. However, there some problems introduced here:
 > {{{#!c++
 > /// <0 or more fixed/var-len data fields>,
 > /// <0 or more domain name fields>,
 > /// <0 or more fixed/var-len data fields>,
 > /// <0 or more domain name fields>,
 > }}}
 >
 > I don't think this is accurate. There is 0 or 1 data fields. And there's
 always 1 or more name fields.

 Ah, right.  And, for the second (and later) data fields, it should be
 exactly 1 ('0' is not necessary incorrect but just redundant).  I've
 fixed these.

 > Also, here:
 > {{{#!c++
 >         // Then, we still have a field in the spec, and it must be a
 domain
 >         // name field.  Since we know we've passed any prior data field,
 the
 >         // next field must be a domain name as long as it exists;
 otherwise
 >         // it's a bug in the spec (not a bogus input).  So we assert()
 that
 >         // condition.
 >         if (current_field_ >= encode_spec_->field_count) {
 >             isc_throw(BadValue,
 >                       "RDATA encoder encounters an unexpected name data:
 " <<
 >                       name);
 >         }
 > }}}
 >
 > The comment talks about assert, but I see none. Also, the comment looks
 slightly repetitive to me?

 There's an assert() a few lines below.  I thought they were close
 enough and I still think so, but move the latter part of the comment
 to the place immediately before the assert() statement.

 > So, except for the comments, I think it can be merged.

 Okay, thanks, merge done.  Closing.

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