BIND 10 #999: Integrate ACLs into b10-resolver

[BIND 10 Development]

#999: Integrate ACLs into b10-resolver
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  vorner                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20110628
                  Component:         |            Resolution:
  Unclassified                       |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  4.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:15 jinmei]:
 > auto_ptr is not very friendly with STL containers (which often
 > internally involve object copies).  So, in general, I'd avoid using
 > auto_ptr with STLs.  It might happen to be safe in this specific case
 > (I've not examined that possibility), but even if so, as you said I
 > don't think it worth taking the risk in this context.

 OK, no problem.

 > >  * Should the `IPCheck<Client>::matches` really be in
 `server_common/client.{cc,h}`?
 >
 > Where else do you think it should belong?  As long as Client is
 > defined in server_common, it should be at least somewhere we can
 > reasonably refer to the Client's definition.

 I don't know. I find it strange and hard to follow, when the class lives
 somewhere and one of it's methods lives somewhere completely else. Anyway,
 that isn't a problem of this changeset, but I believe we don't want such
 things in the code generally and should do something about it. If you say
 this problem will disappear soon, it's OK.

 > >  * Should we test localhost addresses as well? (and in the following
 test) They are default, so to check that they are default at the right
 place.
 >
 > I don't understand these comments...in the first sentence are you
 > suggesting we should explicitly test 127.0.0.1 and ::1 instead of
 > 192.0.2.1, etc?  We could do so, but in this context I don't see
 > an essential difference between these cases.

 Well, I think we should do it to test that the default (allowing
 localhost) comes from the spec file, not the code itself. Because, if the
 code loaded the currently-spec-file default without the spec file, the
 localhost would distinguish it, while the current 192.0.2.1 test wouldn't.

 > >  * You enable debug logging in tests. But should we spam the outputs
 with the debug logging?
 >
 > I believe we should, because otherwise we could overlook regression in
 > producing log messages (we in fact had such a bug, and since it was at
 > the DEBUG level we didn't notice that until after merging it to
 > master).  Actually, I believe we should do this for other cases, too,
 > and created a general ticket for that (don't remember the number right
 > now).  The increased noise level could be an issue as you hinted, so
 > we may have to address that part while enabling debug logging in
 > tests.  But I still believe it's important to enable as many logs as
 > possible in our tests.

 And isn't it better to enable it using the environment variables,
 globally? The problem I see with this is, I have stderr red-colored while
 running tests. And with the logging I see a lot of red, which makes it
 harder to spot real failures and complaints. Maybe I'll just turn the
 logging off using the variables. Anyway, I see it inconsistent with the
 rest of the tests. Maybe we want to discuss it and put the DEBUG level as
 the default?

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