BIND 10 #275: refactoring CC session class for easier tests
BIND 10 Development
do-not-reply at isc.org
Fri Jul 9 08:16:10 UTC 2010
#275: refactoring CC session class for easier tests
----------------------------------------+-----------------------------------
Reporter: jinmei | Owner: jinmei
Type: enhancement | Status: reviewing
Priority: major | Milestone: 06. 4th Incremental Release
Component: Inter-module communication | Resolution:
Keywords: | Sensitive: 0
----------------------------------------+-----------------------------------
Changes (by jelte):
* owner: jelte => jinmei
Comment:
Replying to [comment:3 jinmei]:
>
> I've updated the description of the constructor. But now that I've read
the comment, I wonder it would be more natural if the caller must
establish the session. As a related matter, maybe we should explicitly
detect and reject an attempt of doubly establishing a session? (I guess
the underlying ASIO implementation detects it, but we should probably
catch the invalid usage at a higher level).
>
Hmm, i think we either need to detect that establish is called twice, or
we need to detect that it hasn't been called at all (and then we can
choose to error or establish ourselves)
> > For the sake of consistency, while not exactly essential, should we
make *ccsession and *cs stack objects in auth's main.cc (and perhaps then
rethink how we can do the same for auth_srv and io_service)?
>
> I'm not sure what this means. Do you mean defining cc_session and cs in
the anonymous namespace of auth/main.cc where auth_server and io_service
are defined? If so, I'd actually rather define the latter set more
locally (because in general it's better to use a narrower scope for
mutable variables). But for now we need to define them in the namespace
scope because they are referenced from free functions (my_config_handler
and my_command_handler). As a middle term plan we'll refactor this
architecture with generalizing the ASIO link module, and then I think we
can solve this in a cleaner way. So I'd live with the inconsistency for
now.
>
no, i mean not putting them on the heap
{{{
Session* cc_session = NULL;
<snip>
cc_session = new Session(io_service->get_io_service());
<snip>
delete cc_session;
}}}
but on the stack
{{{
Session cc_session(io_service->get_io_service());
}}}
And same for ModuleCCSession. Due to the current arch this isn't possible
for auth_srv and io_service.
But since we are in a main() function anyway this isn't really important,
and we can probably defer this to a refactor later.
If you want to so the check-session-for-established-already here, please
do, for the rest i think this is ready for merge.
--
Ticket URL: <http://bind10.isc.org/ticket/275#comment:6>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development
More information about the bind10-tickets
mailing list