BIND 10 #2096: Define and implement "RdataReader" class

BIND 10 Development do-not-reply at isc.org
Mon Aug 27 08:18:44 UTC 2012


#2096: Define and implement "RdataReader" class
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20120904
  medium                             |            Resolution:
                  Component:  data   |             Sensitive:  0
  source                             |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  5
            Defect Severity:  N/A    |           Total Hours:  14
Feature Depending on Ticket:         |
  scalable inmemory                  |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 Replying to [comment:26 jinmei]:
 > {{{#!cpp
 >     for (size_t i = 0; i < rdata_count; ++i) {
 >          // write name, type, class, TTL
 >          // skip space for RDATA len
 >        reader.iterateRdata(); // must return true
 >          // write RDATA len
 >     }
 > }}}

 If you're checking the reader.iterateRdata returns true every time (which
 you probably should, to check consistency), maybe you should check the
 next call after the loop returns false. Then it would have the positions
 computed already.

 But I'm not against applying the patch anyway.

 > Further, this makes me wonder whether we want to keep the behavior of
 > next().  At the moment, I don't see the need for it: we only need
 > iterate() (for additional section handling) and iterateRdata() (for
 > `RRset` rendering).  And, since we implement these based on the next()
 > behavior, these need additional layer of function calls for each
 > single "data field".  I wonder whether we could just expand the
 > `nextInternal` behavior so it examines all fields of each RDATA,
 > reducing the additional layer.

 Maybe we don't need it now. Maybe sometime in future, it could come handy.
 So I'll ask differently ‒ is there any good reason to spend the time
 removing next() from the tests and changing it? If it's performance, I
 think it's premature and really minor (this is a call to fixed address in
 the code, if the compiler does not optimise it out completely, compared
 with a call with boost::bind and boost::function in the callback, which is
 at least one virtual call, probably flushing the processor pipeline,
 etc…).

 Thank you

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


More information about the bind10-tickets mailing list