BIND 10 #997: Create the ACL class

[BIND 10 Development]

#997: Create the ACL class
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  vorner                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20110614
                  Component:         |            Resolution:
  Unclassified                       |             Sensitive:  0
                   Keywords:         |           Sub-Project:  Core
            Defect Severity:  N/A    |  Estimated Difficulty:  0.0
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:4 jinmei]:
 > First, I made some trivial editorial changes and pushed them.
 > Having/removing a white space, e.g. from "{ }" may be purely matter of
 > taste (and the coding guideline doesn't say anything about this
 > point), but it seems we don't have a space for these cases in the vast
 > majority of the code, so I made them consistent.

 I noticed ‒ I thought we were using camelCase for C++ code and underscores
 in python. Was I mistaken?

 > - why not using boost::noncopyable to prohibit copy?  we agreed on
 >   using instead of in-house privatization of the copy constructor and
 >   operator= for that purpose.

 I simply forgot about that one. In my own code I usually don't even bother
 to document such class is not supposed to be copied and rely on common
 sense O:-).

 > - execute(): is it reasonable to return an Action "object"?  This
 >   means it must be copyable, and, depending on the internal detail of
 >   the actual Action type, the copy could be very expensive.  On a
 >   related note, maybe it would be useful to describe the minimal
 >   requirement (such as copyability) that the Action template parameter
 >   must meet, and, if it's basically intended to be integral types,
 >   mention that, too.

 I'm returning reference to it now (I somehow implicitly expected to be
 something small, but you're right, when we allow it to be quite anything,
 it may be worth it). But it still needs to be copyable, since I need to
 place it into the vector.

 > - execute(): this type of for loop is a bit more expensive because it
 >   involves a call to entries_.end() in each iteration.  I'd not
 >   necessarily request it be addressed right now, but since this could
 >   be a performance sensitive path, I believe we should be careful.

 Well, considering that the vector is template, therefore completely
 available to compiler and the function is inlined, if it is combined with
 the fact that vector guarantees being really an array and iterator being
 wrapped pointer, this should optimise to single pointer comparison with
 the end pointer being fetched from constant memory location. Would there
 be any way of avoiding even that?

 > The use of reference of pointer seems to be abuse in the apparent
 > sense of Check class:

 Yes, it's abuse for tests (and now even documented). I don't expect the
 match to modify the context (maybe with some exception of a scratchpad
 area for optimising them, but that one would be mutable and not created by
 the user).

 I used mutable in the test, as it sounds a lot cleaner than const-cast to
 me (knowing how many optimisations compiler can base on constness of
 something and mutable is only for the part of the object that changes,
 while const-cast is both hidden, therefore possibly unsafe, and on the
 whole object).

 > that, I'd introduce a non-const version of matches() and let the
 > application use that with an explicit care.

 That wouldn't work, because we would need to have two separate
 hierarchies, one with const and one with non-const or every check would
 need to provide both or something.

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