BIND 10 #658: Check qid of responses

BIND 10 Development do-not-reply at isc.org
Fri Mar 11 12:01:05 UTC 2011


#658: Check qid of responses
-------------------------------------+-------------------------------------
                 Reporter:  jelte    |                Owner:  jelte
                     Type:  defect   |               Status:  reviewing
                 Priority:  major    |            Milestone:  R-Team-
                Component:           |  Sprint-20110316
  resolver                           |           Resolution:
                 Keywords:           |            Sensitive:  0
Estimated Number of Hours:  0.0      |  Add Hours to Ticket:  0
                Billable?:  1        |          Total Hours:  0
                Internal?:  0        |
-------------------------------------+-------------------------------------

Comment (by jelte):

 Replying to [comment:6 stephen]:
 > '''src/lib/dns/buffer_unittest.cc'''[[BR]]
 > Should add a EXPECT_THROW test to check that the exception is thrown if
 the position specified is invalid.

 ack, done

 >
 > '''src/lib/asiolink/io_fetch.cc'''[[BR]]
 > The recently merged ticket #499 includes "asiolink_utilities", which
 provides a readUint16 function.
 >

 ah, i'll use that one when i merge (but see below)

 > I think it would be more elegant to put the QID check in a separate
 method which does the check and prints the error message (instead of an
 explicit test inlie and a "break" to get out of the loop), i.e.
 > {{{
 > do {
 >    do {
 >       asyncReceive
 >    } while (!receiveComplete)
 > } while (!qidMatches)
 > }}}
 >

 ok. In fact, the address check could go in there as well, so i've made it
 responseOK()

 > We do need the address check.  The endpoint passed to open() and
 asyncSend() is is the address to where the data is being sent.  The
 endpoint returned from asyncReceive() is the address from which the
 response was received.  (These may be different when UDP is used.)  At
 present, IOFetch is incorrect in that it uses the same variable for both:
 it should use a separate one for the returned address.

 right. Changed remote to remote_snd and added a remote_rcv (which is
 initially a copy of remote_snd)

 For the merge i'll also change that shared_ptr for remote_rcv to a
 scoped_ptr. But I just quickly looked, and 499 changed exactly that much
 that nearly every change here ended up a conflict. So over lunch i will
 contemplate 'replaying' this branch manually on top of the current master
 (in a new branch). You may choose whether to review this current change,
 the merge result, or both :)

-- 
Ticket URL: <https://bind10.isc.org/ticket/658#comment:7>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development


More information about the bind10-tickets mailing list