Commit fb9060e5 authored by JINMEI Tatuya's avatar JINMEI Tatuya
Browse files

[1697] 1st update: make MessageRenderer constructors parameter-free.

AbstractMessageRenderer now creates and holds its own buffer internally.
adjusted unittests and the python wrapper in a straightforward manner.
parent 464cada6
......@@ -42,7 +42,7 @@ template <typename T>
class RdataRenderBenchMark {
public:
RdataRenderBenchMark(const vector<T>& dataset) :
dataset_(dataset), buffer_(4096), renderer_(buffer_)
dataset_(dataset)
{}
unsigned int run() {
typename vector<T>::const_iterator data;
......@@ -55,7 +55,6 @@ public:
}
private:
const vector<T>& dataset_;
OutputBuffer buffer_;
MessageRenderer renderer_;
};
......
......@@ -171,8 +171,8 @@ struct MessageRenderer::MessageRendererImpl {
CompressMode compress_mode_;
};
MessageRenderer::MessageRenderer(OutputBuffer& buffer) :
AbstractMessageRenderer(buffer),
MessageRenderer::MessageRenderer() :
AbstractMessageRenderer(),
impl_(new MessageRendererImpl)
{}
......@@ -273,9 +273,14 @@ MessageRenderer::writeName(const Name& name, const bool compress) {
}
}
AbstractMessageRenderer::AbstractMessageRenderer() :
local_buffer_(0), buffer_(&local_buffer_)
{
}
void
AbstractMessageRenderer::clear() {
buffer_.clear();
buffer_->clear();
}
}
......
......@@ -102,29 +102,32 @@ protected:
/// This is intentionally defined as \c protected as this base class should
/// never be instantiated (except as part of a derived class).
/// \param buffer The buffer where the data should be rendered into.
///
/// We are now doing this:
/// \todo We might want to revisit this API at some point and remove the
/// buffer parameter. In that case it would create it's own buffer and
/// a function to extract the data would be available instead. It seems
/// like a cleaner design, but it's left undone until we would actually
/// benefit from the change.
AbstractMessageRenderer(isc::util::OutputBuffer& buffer) :
buffer_(buffer)
{}
AbstractMessageRenderer();
public:
/// \brief The destructor.
virtual ~AbstractMessageRenderer() {}
//@}
protected:
/// \brief Return the output buffer we render into.
const isc::util::OutputBuffer& getBuffer() const { return (buffer_); }
isc::util::OutputBuffer& getBuffer() { return (buffer_); }
const isc::util::OutputBuffer& getBuffer() const { return (*buffer_); }
isc::util::OutputBuffer& getBuffer() { return (*buffer_); }
private:
/// \short Local (default) buffer to store data.
isc::util::OutputBuffer local_buffer_;
/// \short Buffer to store data
///
/// It was decided that there's no need to have this in every subclass,
/// at least not now, and this reduces code size and gives compiler a better
/// chance to optimise.
isc::util::OutputBuffer& buffer_;
isc::util::OutputBuffer* buffer_;
public:
///
/// \name Getter Methods
......@@ -136,12 +139,12 @@ public:
/// This method works exactly same as the same method of the \c OutputBuffer
/// class; all notes for \c OutputBuffer apply.
const void* getData() const {
return (buffer_.getData());
return (buffer_->getData());
}
/// \brief Return the length of data written in the internal buffer.
size_t getLength() const {
return (buffer_.getLength());
return (buffer_->getLength());
}
/// \brief Return whether truncation has occurred while rendering.
......@@ -209,7 +212,7 @@ public:
///
/// \param len The length of the gap to be inserted in bytes.
void skip(size_t len) {
buffer_.skip(len);
buffer_->skip(len);
}
/// \brief Trim the specified length of data from the end of the internal
......@@ -223,7 +226,7 @@ public:
///
/// \param len The length of data that should be trimmed.
void trim(size_t len) {
buffer_.trim(len);
buffer_->trim(len);
}
/// \brief Clear the internal buffer and other internal resources.
......@@ -236,7 +239,7 @@ public:
///
/// \param data The 8-bit integer to be written into the internal buffer.
void writeUint8(const uint8_t data) {
buffer_.writeUint8(data);
buffer_->writeUint8(data);
}
/// \brief Write an unsigned 16-bit integer in host byte order into the
......@@ -244,7 +247,7 @@ public:
///
/// \param data The 16-bit integer to be written into the buffer.
void writeUint16(uint16_t data) {
buffer_.writeUint16(data);
buffer_->writeUint16(data);
}
/// \brief Write an unsigned 16-bit integer in host byte order at the
......@@ -259,7 +262,7 @@ public:
/// \param data The 16-bit integer to be written into the internal buffer.
/// \param pos The beginning position in the buffer to write the data.
void writeUint16At(uint16_t data, size_t pos) {
buffer_.writeUint16At(data, pos);
buffer_->writeUint16At(data, pos);
}
/// \brief Write an unsigned 32-bit integer in host byte order into the
......@@ -267,7 +270,7 @@ public:
///
/// \param data The 32-bit integer to be written into the buffer.
void writeUint32(uint32_t data) {
buffer_.writeUint32(data);
buffer_->writeUint32(data);
}
/// \brief Copy an arbitrary length of data into the internal buffer
......@@ -278,7 +281,7 @@ public:
/// \param data A pointer to the data to be copied into the internal buffer.
/// \param len The length of the data in bytes.
void writeData(const void *data, size_t len) {
buffer_.writeData(data, len);
buffer_->writeData(data, len);
}
/// \brief Write a \c Name object into the internal buffer in wire format,
......@@ -316,10 +319,7 @@ public:
using AbstractMessageRenderer::CASE_SENSITIVE;
/// \brief Constructor from an output buffer.
///
/// \param buffer An \c OutputBuffer object to which wire format data is
/// written.
MessageRenderer(isc::util::OutputBuffer& buffer);
MessageRenderer();
virtual ~MessageRenderer();
virtual bool isTruncated() const;
......
......@@ -36,7 +36,6 @@ namespace {
class s_MessageRenderer : public PyObject {
public:
s_MessageRenderer();
isc::util::OutputBuffer* outputbuffer;
MessageRenderer* cppobj;
};
......@@ -78,17 +77,14 @@ PyMethodDef MessageRenderer_methods[] = {
int
MessageRenderer_init(s_MessageRenderer* self) {
self->outputbuffer = new OutputBuffer(4096);
self->cppobj = new MessageRenderer(*self->outputbuffer);
self->cppobj = new MessageRenderer;
return (0);
}
void
MessageRenderer_destroy(s_MessageRenderer* self) {
delete self->cppobj;
delete self->outputbuffer;
self->cppobj = NULL;
self->outputbuffer = NULL;
Py_TYPE(self)->tp_free(self);
}
......
......@@ -70,8 +70,7 @@ namespace {
// it's hopefully an acceptable practice.
class RdataFieldComposer : public AbstractMessageRenderer {
public:
RdataFieldComposer(OutputBuffer& buffer) :
AbstractMessageRenderer(buffer),
RdataFieldComposer() :
truncated_(false), length_limit_(65535),
mode_(CASE_INSENSITIVE), last_data_pos_(0)
{}
......@@ -128,8 +127,7 @@ public:
}
RdataFields::RdataFields(const Rdata& rdata) {
OutputBuffer buffer(0);
RdataFieldComposer field_composer(buffer);
RdataFieldComposer field_composer;
rdata.toWire(field_composer);
nfields_ = field_composer.getFields().size();
data_length_ = field_composer.getLength();
......
......@@ -43,9 +43,7 @@ const uint8_t EDNS::SUPPORTED_VERSION;
namespace {
class EDNSTest : public ::testing::Test {
protected:
EDNSTest() : rrtype(RRType::OPT()), buffer(NULL, 0), obuffer(0),
renderer(obuffer), rcode(0)
{
EDNSTest() : rrtype(RRType::OPT()), buffer(NULL, 0), obuffer(0), rcode(0) {
opt_rdata = ConstRdataPtr(new generic::OPT());
edns_base.setUDPSize(4096);
}
......
......@@ -79,7 +79,6 @@ namespace {
class MessageTest : public ::testing::Test {
protected:
MessageTest() : test_name("test.example.com"), obuffer(0),
renderer(obuffer),
message_parse(Message::PARSE),
message_render(Message::RENDER),
bogus_section(static_cast<Message::Section>(
......@@ -731,8 +730,8 @@ TEST_F(MessageTest, toWire) {
message_render.toWire(renderer);
vector<unsigned char> data;
UnitTestUtil::readWireData("message_toWire1", data);
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, obuffer.getData(),
obuffer.getLength(), &data[0], data.size());
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, renderer.getData(),
renderer.getLength(), &data[0], data.size());
}
TEST_F(MessageTest, toWireInParseMode) {
......@@ -961,9 +960,6 @@ TEST_F(MessageTest, toWireTSIGNoTruncation) {
// rendering fail unexpectedly in the test that follows.
class BadRenderer : public MessageRenderer {
public:
BadRenderer(isc::util::OutputBuffer& buffer) :
MessageRenderer(buffer)
{}
virtual void setLengthLimit(size_t len) {
if (getLength() > 0) {
MessageRenderer::setLengthLimit(getLength());
......@@ -994,8 +990,7 @@ TEST_F(MessageTest, toWireTSIGLengthErrors) {
InvalidParameter);
// Trying to render a message with TSIG using a buggy renderer.
obuffer.clear();
BadRenderer bad_renderer(obuffer);
BadRenderer bad_renderer;
bad_renderer.setLengthLimit(512);
message_render.clear(Message::RENDER);
EXPECT_THROW(commonTSIGToWireCheck(message_render, bad_renderer, tsig_ctx,
......
......@@ -30,15 +30,13 @@ using isc::util::OutputBuffer;
namespace {
class MessageRendererTest : public ::testing::Test {
protected:
MessageRendererTest() : expected_size(0), buffer(0), renderer(buffer)
{
MessageRendererTest() : expected_size(0) {
data16 = (2 << 8) | 3;
data32 = (4 << 24) | (5 << 16) | (6 << 8) | 7;
}
size_t expected_size;
uint16_t data16;
uint32_t data32;
OutputBuffer buffer;
MessageRenderer renderer;
std::vector<unsigned char> data;
static const uint8_t testdata[5];
......@@ -60,21 +58,22 @@ TEST_F(MessageRendererTest, writeName) {
renderer.writeName(Name("a.example.com."));
renderer.writeName(Name("b.example.com."));
renderer.writeName(Name("a.example.org."));
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, buffer.getData(),
buffer.getLength(), &data[0], data.size());
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, renderer.getData(),
renderer.getLength(), &data[0], data.size());
}
TEST_F(MessageRendererTest, writeNameInLargeBuffer) {
size_t offset = 0x3fff;
buffer.skip(offset);
renderer.skip(offset);
UnitTestUtil::readWireData("name_toWire2", data);
renderer.writeName(Name("a.example.com."));
renderer.writeName(Name("a.example.com."));
renderer.writeName(Name("b.example.com."));
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
static_cast<const uint8_t*>(buffer.getData()) + offset,
buffer.getLength() - offset,
static_cast<const uint8_t*>(renderer.getData()) +
offset,
renderer.getLength() - offset,
&data[0], data.size());
}
......@@ -83,8 +82,8 @@ TEST_F(MessageRendererTest, writeNameWithUncompressed) {
renderer.writeName(Name("a.example.com."));
renderer.writeName(Name("b.example.com."), false);
renderer.writeName(Name("b.example.com."));
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, buffer.getData(),
buffer.getLength(), &data[0], data.size());
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, renderer.getData(),
renderer.getLength(), &data[0], data.size());
}
TEST_F(MessageRendererTest, writeNamePointerChain) {
......@@ -92,8 +91,8 @@ TEST_F(MessageRendererTest, writeNamePointerChain) {
renderer.writeName(Name("a.example.com."));
renderer.writeName(Name("b.example.com."));
renderer.writeName(Name("b.example.com."));
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, buffer.getData(),
buffer.getLength(), &data[0], data.size());
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, renderer.getData(),
renderer.getLength(), &data[0], data.size());
}
TEST_F(MessageRendererTest, compressMode) {
......@@ -120,8 +119,8 @@ TEST_F(MessageRendererTest, writeNameCaseCompress) {
// this should match the first name in terms of compression:
renderer.writeName(Name("b.exAmple.CoM."));
renderer.writeName(Name("a.example.org."));
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, buffer.getData(),
buffer.getLength(), &data[0], data.size());
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, renderer.getData(),
renderer.getLength(), &data[0], data.size());
}
TEST_F(MessageRendererTest, writeNameCaseSensitiveCompress) {
......@@ -132,8 +131,8 @@ TEST_F(MessageRendererTest, writeNameCaseSensitiveCompress) {
renderer.writeName(Name("a.example.com."));
renderer.writeName(Name("b.eXample.com."));
renderer.writeName(Name("c.eXample.com."));
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, buffer.getData(),
buffer.getLength(), &data[0], data.size());
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, renderer.getData(),
renderer.getLength(), &data[0], data.size());
}
TEST_F(MessageRendererTest, writeNameMixedCaseCompress) {
......@@ -147,8 +146,8 @@ TEST_F(MessageRendererTest, writeNameMixedCaseCompress) {
// allowed in this API.
renderer.setCompressMode(MessageRenderer::CASE_INSENSITIVE);
renderer.writeName(Name("c.b.EXAMPLE.com."));
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, buffer.getData(),
buffer.getLength(), &data[0], data.size());
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, renderer.getData(),
renderer.getLength(), &data[0], data.size());
}
TEST_F(MessageRendererTest, writeRootName) {
......@@ -164,8 +163,8 @@ TEST_F(MessageRendererTest, writeRootName) {
renderer.writeName(Name("."));
renderer.writeName(example_name);
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
static_cast<const uint8_t*>(buffer.getData()),
buffer.getLength(),
static_cast<const uint8_t*>(renderer.getData()),
renderer.getLength(),
static_cast<const uint8_t*>(expected.getData()),
expected.getLength());
}
......
......@@ -357,13 +357,12 @@ TEST_F(NameTest, toWireBuffer) {
//
TEST_F(NameTest, toWireRenderer) {
vector<unsigned char> data;
OutputBuffer buffer(0);
MessageRenderer renderer(buffer);
MessageRenderer renderer;
UnitTestUtil::readWireData(string("01610376697803636f6d00"), data);
Name("a.vix.com.").toWire(renderer);
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, &data[0], data.size(),
buffer.getData(), buffer.getLength());
renderer.getData(), renderer.getLength());
}
//
......@@ -524,8 +523,8 @@ TEST_F(NameTest, at) {
example_name.toWire(buffer_expected);
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
&data[0], data.size(),
buffer_expected.getData(), buffer_expected.getLength());
&data[0], data.size(), buffer_expected.getData(),
buffer_expected.getLength());
// Out-of-range access: should trigger an exception.
EXPECT_THROW(example_name.at(example_name.getLength()), OutOfRange);
......
......@@ -37,7 +37,7 @@ using namespace isc::util;
namespace {
class QuestionTest : public ::testing::Test {
protected:
QuestionTest() : obuffer(0), renderer(obuffer),
QuestionTest() : obuffer(0),
example_name1(Name("foo.example.com")),
example_name2(Name("bar.example.com")),
test_question1(example_name1, RRClass::IN(),
......@@ -102,8 +102,8 @@ TEST_F(QuestionTest, toWireRenderer) {
test_question1.toWire(renderer);
test_question2.toWire(renderer);
UnitTestUtil::readWireData("question_toWire2", wiredata);
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, obuffer.getData(),
obuffer.getLength(), &wiredata[0], wiredata.size());
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, renderer.getData(),
renderer.getLength(), &wiredata[0], wiredata.size());
}
TEST_F(QuestionTest, toWireTruncated) {
......
......@@ -162,7 +162,7 @@ TEST_F(Rdata_AFSDB_Test, toWireRenderer) {
renderer.clear();
// construct actual data
Name("example.com.").toWire(obuffer);
renderer.writeName(Name("example.com."));
rdata_afsdb2.toWire(renderer);
// construct expected data
......
......@@ -97,11 +97,11 @@ TEST_F(Rdata_CNAME_Test, toWireBuffer) {
TEST_F(Rdata_CNAME_Test, toWireRenderer) {
rdata_cname.toWire(renderer);
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
obuffer.getData(), obuffer.getLength(),
renderer.getData(), renderer.getLength(),
wiredata_cname, sizeof(wiredata_cname));
rdata_cname2.toWire(renderer);
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
obuffer.getData(), obuffer.getLength(),
renderer.getData(), renderer.getLength(),
wiredata_cname2, sizeof(wiredata_cname2));
}
......
......@@ -97,11 +97,11 @@ TEST_F(Rdata_DNAME_Test, toWireBuffer) {
TEST_F(Rdata_DNAME_Test, toWireRenderer) {
rdata_dname.toWire(renderer);
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
obuffer.getData(), obuffer.getLength(),
renderer.getData(), renderer.getLength(),
wiredata_dname, sizeof(wiredata_dname));
rdata_dname2.toWire(renderer);
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
obuffer.getData(), obuffer.getLength(),
renderer.getData(), renderer.getLength(),
wiredata_dname2, sizeof(wiredata_dname2));
}
......
......@@ -90,8 +90,8 @@ TEST_F(Rdata_DNSKEY_Test, toWireRenderer) {
vector<unsigned char> data;
UnitTestUtil::readWireData("rdata_dnskey_fromWire", data);
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
static_cast<const uint8_t *>(obuffer.getData()) + 2,
obuffer.getLength() - 2, &data[2], data.size() - 2);
static_cast<const uint8_t *>(renderer.getData()) + 2,
renderer.getLength() - 2, &data[2], data.size() - 2);
}
TEST_F(Rdata_DNSKEY_Test, toWireBuffer) {
......
......@@ -115,8 +115,8 @@ TYPED_TEST(Rdata_DS_LIKE_Test, toWireRenderer) {
UnitTestUtil::readWireData("rdata_ds_fromWire", data);
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
static_cast<const uint8_t*>
(this->obuffer.getData()) + 2,
this->obuffer.getLength() - 2,
(this->renderer.getData()) + 2,
this->renderer.getLength() - 2,
&data[2], data.size() - 2);
}
......
......@@ -94,8 +94,9 @@ TEST_F(Rdata_HINFO_Test, toWireRenderer) {
HINFO hinfo(hinfo_str);
hinfo.toWire(renderer);
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, obuffer.getData(),
obuffer.getLength(), hinfo_rdata, sizeof(hinfo_rdata));
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, renderer.getData(),
renderer.getLength(), hinfo_rdata,
sizeof(hinfo_rdata));
}
TEST_F(Rdata_HINFO_Test, compare) {
......
......@@ -78,7 +78,7 @@ TEST_F(Rdata_IN_A_Test, toWireBuffer) {
TEST_F(Rdata_IN_A_Test, toWireRenderer) {
rdata_in_a.toWire(renderer);
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
obuffer.getData(), obuffer.getLength(),
renderer.getData(), renderer.getLength(),
wiredata_in_a, sizeof(wiredata_in_a));
}
......
......@@ -76,7 +76,7 @@ TEST_F(Rdata_IN_AAAA_Test, toWireBuffer) {
TEST_F(Rdata_IN_AAAA_Test, toWireRenderer) {
rdata_in_aaaa.toWire(renderer);
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
obuffer.getData(), obuffer.getLength(),
renderer.getData(), renderer.getLength(),
wiredata_in_aaaa, sizeof(wiredata_in_aaaa));
}
......
......@@ -142,15 +142,15 @@ TEST_F(Rdata_MINFO_Test, toWireRenderer) {
vector<unsigned char> data;
UnitTestUtil::readWireData("rdata_minfo_toWire1.wire", data);
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
static_cast<const uint8_t *>(obuffer.getData()),
obuffer.getLength(), &data[0], data.size());
static_cast<const uint8_t *>(renderer.getData()),
renderer.getLength(), &data[0], data.size());
renderer.clear();
rdata_minfo2.toWire(renderer);
vector<unsigned char> data2;
UnitTestUtil::readWireData("rdata_minfo_toWire2.wire", data2);
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
static_cast<const uint8_t *>(obuffer.getData()),
obuffer.getLength(), &data2[0], data2.size());
static_cast<const uint8_t *>(renderer.getData()),
renderer.getLength(), &data2[0], data2.size());
}
TEST_F(Rdata_MINFO_Test, toText) {
......
......@@ -68,12 +68,12 @@ TEST_F(Rdata_MX_Test, toWireRenderer) {
vector<unsigned char> data;
UnitTestUtil::readWireData("rdata_mx_toWire1", data);
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, obuffer.getData(),
obuffer.getLength(), &data[0], data.size());
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, renderer.getData(),
renderer.getLength(), &data[0], data.size());
}
TEST_F(Rdata_MX_Test, toWireBuffer) {
renderer.writeName(Name("example.com"));
Name("example.com").toWire(obuffer);
rdata_mx.toWire(obuffer);
vector<unsigned char> data;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment