BIND 10 #2097: Define and Implement RdataSet class

[BIND 10 Development]

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

Comment (by jinmei):

 Replying to [comment:13 jelte]:

 Thanks for the prompt review.

 You seem to have gone, but I'm responding to the comments anyway.
 This branch can't be merged until #2096 is done anyway, so if this
 ticket is still in the queue when you're back (which is actually
 pretty likely) and if you can take a look at the additional changes,
 that would be nice.  So I'm giving the ticket back to you.  If #2096
 is merged before that, I'll merge this branch at that point.

 > The code looks fine, and can be merged. I have a few minor suggestions,
 which you can take or leave :)
 >
 > given the nature of this beast, I think it should be explicitely
 noncopyable. In effect compilers will fail on stack-allocation due to the
 private destructor, but I think it would be a bit cleaner to also use
 boost::noncopyable.

 Okay, done.

 > Also, I wonder if convertRRTTL() should be a native public method of the
 RRTTL class, it sounds reasonable to have a way to get it in wire format
 (or have it as a util function an make writeUint32 use it too). Not that
 it's complicated code or anything, so no strong opinion, but it does seem
 to go against the DRY principle)

 Hmm, at least for now, I'd not extend the RRTTL class.
 Functionality-wise, it's possible to do this conversion by writing to
 an output buffer via RRTTL::toWire() then memcpy the data to uint32_t,
 although the code would be less concise and less efficient; or, if the
 caller is okay with using htonl(), they can do it by
 htonl(RRTTL::getValue()).  Besides, this doesn't seem to be specific
 to TTL; the same extension would apply to `RRType` and `RRClass`.

 So, for now, I'll keep this customization here until we see a second
 need for the same thing (at the risk of forgetting we have one at that
 point).  If we see the need for providing unified utility, I guess
 what we should is to provide a homebrew portable version of htonl.

 > And some additional test suggestions:
 >
 > createManyRRs: perhaps also try with a count more than 8192 (to make
 sure the code uses > not == 8192 in its check)
 >
 > manyRRSigCount: ditto (but obviously with something more than 65536),

 Okay, and I've updated the helper getRRSIGWithRdataCount() so it runs
 faster.  I already realized it was a bit slow for a huge number of
 RRSIGs, and adding one more will make it worse (in general we wouldn't
 want to see unit tests run for multiple seconds).

 > also perhaps since we check that the magic number 7 is used here, check
 that MANY_RRSIG_COUNT is 7?

 MANY_RRSIG_COUNT is private so cannot be directly tested.  And, in
 this case, it doesn't seem to make much sense to me to make it public
 just for tests.  I guess the definition is simple enough, and tests
 in createManyRRSIGs() sufficiently cover possible issues with this
 constant.  So, at the moment, I didn't revise the code for this point.

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