BIND 10 #2432: define and implement base and libdns++ version of RRsetCollection

BIND 10 Development do-not-reply at isc.org
Fri Dec 21 16:53:32 UTC 2012


#2432: define and implement base and libdns++ version of RRsetCollection
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:  muks
                Type:  task          |                       Status:
            Priority:  medium        |  reviewing
           Component:  libdns++      |                    Milestone:
            Keywords:                |  Sprint-20130108
           Sensitive:  0             |                   Resolution:
         Sub-Project:  DNS           |                 CVSS Scoring:
Estimated Difficulty:  5             |              Defect Severity:  N/A
         Total Hours:  0             |  Feature Depending on Ticket:
                                     |  loadzone-ng
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => muks


Comment:

 Hello

 While it is nice to have an std iterator interface, I'm not sure it is
 working
 out well. If I don't count the magic with shared pointers (which I don't
 think
 I can do better), I think it'll be a real challenge with stuff like the
 data
 sources, or a database. Imagine you have a database handle and you invoke
 a
 statement and then you try to iterate through it. But then, if you do the
 postfix version of iterator, what do you do with the handle? Or, if you
 copy
 the iterator and try to iterate independently with each copy. I don't
 think the
 database handle can be „forked“ like this. I'm not sure I'm explaining my
 mind
 well enough.

 Another high-level thing is the only constructor of the `RRsetCollection`
 takes
 the filename to master file. This is quite limiting. What if I didn't want
 to
 create one by loading, but wanted an empty one and fill some data in in
 some
 other ways?

 Why is there the rrclass parameter both with the constructor and the
 add/remove
 interface, and not in find? I think it should be consistent and in case
 the
 rrclass is there, the collection should allow storing RRsets of different
 classes (which does make some sense) or the rrclass should be
 nowhere/possibly
 only in the constructor and different rrclass rrsets rejected in add.

 Also, there are some implementation comments as well as design ones.
  * With the `operator ==` and `operator !=`, how are iterators from
 different
    types of collections handled? Which brings me to the concrete
 implementaion
    of `equal` from the libdns++ implementation, which would throw in case
    a wrong type is passed there. I'd guess the intuitive behaviour would
 be
    to return false (as they aren't the same, because they are of different
 type).
  * I'm not sure if it is possible in the datasrc case, but I'd expect the
    find, begin and end to be const, because they return const RRsets. Or
 don't they?
  * Do we really? `// We just ignore errors and warnings.`
  * The tests findBase, find and findConst look very similar to me. What is
 the
    difference there?
  * It would be nice to test various error cases, what happens with
 duplicate
    RRset is add, etc.

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


More information about the bind10-tickets mailing list