BIND 10 #1576: implement ZoneFinder::findNSEC3 in in-memory data source

[BIND 10 Development]

#1576: implement ZoneFinder::findNSEC3 in in-memory data source
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20120221
                  Component:  data   |            Resolution:
  source                             |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  6
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 Replying to [comment:15 jinmei]:
 > Replying to [comment:14 vorner]:
 > > I don't know. If I wanted to play with a hash algorithm, I'd want the
 rest to keep working and check how it works in practice (eg. plugging it
 into b10-auth or b10-resolver and putting it online somewhere). So the
 approach of replacing it doesn't seem to work.
 > >
 > > I understand it is too early to worry about it, but then, I think the
 comment should claim testing purposes only for now, because I don't see
 how this would help in the case of the experiments. It might be worth
 noting the interface might change for allowing of easy plugging in of new
 algorithms.
 >
 > One possible approach is to make DefaultNSEC3HashCreator public, and
 > let the user-defined creator call its create()s for all other cases
 > than the case that needs specialized behavior.  Would that address
 > your concern?

 That would probably help a little (with a comment recommending such use).
 It would still be impossible to „crossbreed“ two different creators, so it
 would still not be usable for plugin-provided hash algorithms, but we
 might not need that in the end anyway.

 > > Well, it is surprising, because nothing inherits from it (or, it does,
 hidden in the macros, but they would be static objects on the stack, so
 the dynamic resolution would not happen anyway). The first think I did
 when I saw it was looking if the test inherits from something unusual or
 if something inherits from the test. Then I asked if there's some other
 reason I overlooked.
 >
 > If you think it's rather confusing I'm okay with removing the
 > virtual.  For now I've not touched it.

 Yes, please.

 > > I was surprised by the empty „Actual“ field and put it into a
 debugger. It shows that the closest_labels is a char (so it would be nice
 to turn it to integer, or, why is the variable char-sized anyway? I know
 it can be up to 255 anyway, but is there a need to save space, if the
 return value is just an ephemeral object?). And the debugger showed it was
 4 at the time.
 >
 > I thought it's slightly more lightweight in copy.  It would also be
 > not bad to eliminate the possibility of getting a bogus value from a
 > buggy derived findNSEC3() implementation (such as 256).  But it's not
 > a strong opinion.  For now I've not changed that except the tests now
 > compare them as int.

 While I doubt it'll have any positive impact on performance (it might be
 smaller to copy, but I don't think there'll be any copy anyway in the
 optimised version, and it'll be converted to native-width whenever there's
 a manipulation with it, and due to data alignment, it'll be padded with
 empty space anyway). But the prevention of bogus data looks reasonable, so
 I don't think it needs to be changed.

 There's nothing substantial now, so after you update it, I think it could
 be merged without another review round.

 Thank you

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