BIND 10 #893: TSIG verify, part 2

[BIND 10 Development]

#893: TSIG verify, part 2
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:         |             Milestone:
  enhancement                        |  Sprint-20110517
                   Priority:  major  |            Resolution:
                  Component:         |             Sensitive:  0
  DNSPacket API                      |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  3.0
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by stephen):

 * owner:  stephen => jinmei


Comment:

 > So, are you suggesting we should make the buffer local to helper
 methods, or are you okaying the current code? I'm okay with either way.
 I was suggesting using a local variable but like you, I'm OK with how it
 is at the moment.

 > Hmm, I see. But if we use the free function version of writeUint16 we
 need to update the intermediate 8 bytes separately, which would make the
 code look rather complicated (relatively).
 If std::copy is used, the copying would still be a one-line operation.

 I think that the performance gain would be very minor so I suggest that we
 leave the code as it is now, but bear this discussion in mind if we have
 to come back and do some low-level optimisation.

 > Hmm, actually I also previously (not for this task) thought about having
 a separate header file to define numeric protocol constants such as
 Name::MAX_WIRE/MAX_LABELS, Message::DEFAULT_MAX_UDPSIZE. I was not sure if
 such a separate header file was really useful and still am not so sure,
 but if you like I'll create a separate ticket for that (at least for
 considering it). It's a different question whether we want to publicly
 define constants for things like the size of DNS header section or offsets
 to particular fields such as QID or ARCOUNT...I personally think they are
 too minor to be defined publicly.
 I think I would argue for both.  I disagree that the constants are too
 trivial - if nothing else, using these constants in expressions (instead
 of "magic" numbers) would help the documentation of the code.

 > I added some more explanation (143b2c6). Hopefully this addressed your
 concern.
 It does, thank-you.

 All fine (including the !ChangeLog entry suggested earlier), please merge.

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