BIND 10 #296: Timeouts on msgq's cc-channel

[BIND 10 Development]

#296: Timeouts on msgq's cc-channel
-------------------------------+--------------------------------------------
      Reporter:  jelte         |        Owner:  jinmei              
          Type:  enhancement   |       Status:  reviewing           
      Priority:  major         |    Milestone:  y2 6 month milestone
     Component:  Unclassified  |   Resolution:                      
      Keywords:                |    Sensitive:  0                   
Estimatedhours:  0.0           |        Hours:  0                   
      Billable:  1             |   Totalhours:  0                   
      Internal:  0             |  
-------------------------------+--------------------------------------------

Comment(by jinmei):

 '''About r2699'''
  - I'd avoid using a macro.  I'd define it as either a namespace or class
 constant, e.g.:
 {{{
 class SessionImpl {
 private:
     static const size_t MSGQ_DEFAULT_TIMEOUT = 4000;
 ...
 };
 }}}
  - isn't it possible to move setup for 'sess' to the fixture?
 {{{
     Session sess(my_io_service);
 }}}

 '''About r2716'''
  - I have some suggested editorial changes (committed to the branch)
  - setResult() should better be in an anonymous namespace.
  - I'd remove the hardcoded value from:
 {{{
     // timeout for blocking reads (in seconds, defaults to 4000)
 }}}
  - is it okay to do io_service().reset()?  According to the ASIO reference
 "this function must not be called while there are any unfinished calls".
 I'm afraid we cannot assume such a thing in general.
  - I don't understand why we need to check getTimeout() != 0 here:
 {{{
             if (read_result && getTimeout() != 0) {
 }}}
  - The following line isn't necessarily incorrect but looks a bit awkward:
 {{{
         if (read_result->value() != 0) {
 }}}
   why not this?  This seems to be more consistent with the other usage of
 asio::error_code in this code:
 {{{
         if (*read_result) {
 }}}

 Other changes look okay.

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