BIND 10 #1697: extend MessageRenderer so we can replace the output buffer

BIND 10 Development do-not-reply at isc.org
Tue Feb 28 13:21:27 UTC 2012


#1697: extend MessageRenderer so we can replace the output buffer
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20120306
                  Component:         |            Resolution:
  libdns++                           |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  9
Feature Depending on Ticket:  auth   |           Total Hours:  0
  performance                        |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 One question first. Maybe I'm missing something, but this way we are
 creating two buffers instead of one each time, right? One private of the
 renderer and one it is set to later. What is the plan about this? How
 should this help to speed things up?

 Regarding the code, I've only few minor things:

 What buffer (leftover \param)?
 {{{#!c++
     ///
     /// \param buffer The buffer where the data should be rendered into.
     AbstractMessageRenderer();
 }}}

 The same message seems to be rendered to two different buffers. What is
 the purpose?
 {{{#!c++
         // Construct the data buffer for question we expect to receive.
         Message msg(Message::RENDER);
         msg.setQid(0);
         msg.setOpcode(Opcode::QUERY());
         msg.setRcode(Rcode::NOERROR());
         msg.setHeaderFlag(Message::HEADERFLAG_RD);
         msg.addQuestion(question_);
         EDNSPtr msg_edns(new EDNS());
         msg_edns->setUDPSize(Message::DEFAULT_MAX_EDNS0_UDPSIZE);
         msg.setEDNS(msg_edns);

         MessageRenderer renderer;
         renderer.setBuffer(msgbuf_.get());
         msg.toWire(renderer);
         renderer.setBuffer(NULL);

         renderer.setBuffer(expected_buffer_.get());
         msg.toWire(renderer);
         renderer.setBuffer(NULL);
 }}}

 This should be \param, not \brief:
 {{{#!c++
    /// \throw isc::InvalidParameter A restrictions of the method usage
 isn't
    /// met.
    ///
    /// \brief buffer A pointer to a temporary output buffer or NULL for
 reset
    /// it.
    void setBuffer(isc::util::OutputBuffer* buffer);
 }}}

 Typo, rest→reset.
 {{{#!c++
     TEST_F(MessageRendererTest, setBufferErrors) {
     OutputBuffer new_buffer(0);

     // Buffer cannot be rest when the renderer is in use.
     renderer.writeUint32(10);
 }}}

 Thank you

 With regards

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


More information about the bind10-tickets mailing list