BIND 10 trac404, updated. a140e9b97e208ce30d0c3fb51f14dd248f307b1d [trac404] Unify the interface a little bit
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Apr 13 11:04:04 UTC 2011
The branch, trac404 has been updated
via a140e9b97e208ce30d0c3fb51f14dd248f307b1d (commit)
from 87c6ef9942c9edcb37aa811e1c20357e6bd6bc4b (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit a140e9b97e208ce30d0c3fb51f14dd248f307b1d
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed Apr 13 13:02:44 2011 +0200
[trac404] Unify the interface a little bit
And hint more by a little bit that the user isn't really expected to dig
inside the data.
-----------------------------------------------------------------------
Summary of changes:
src/lib/dns/benchmarks/rdatarender_bench.cc | 22 ++++++-----
src/lib/dns/rdatafields.cc | 13 ++++--
src/lib/dns/rdatafields.h | 56 +++++++++++++++++---------
src/lib/dns/tests/rdatafields_unittest.cc | 30 +++++++-------
4 files changed, 71 insertions(+), 50 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/dns/benchmarks/rdatarender_bench.cc b/src/lib/dns/benchmarks/rdatarender_bench.cc
index 060ab65..9dd2ba1 100644
--- a/src/lib/dns/benchmarks/rdatarender_bench.cc
+++ b/src/lib/dns/benchmarks/rdatarender_bench.cc
@@ -67,28 +67,30 @@ public:
RdataFieldsStore(ConstRdataPtr rdata) {
const RdataFields fields(*rdata);
- nspecs_ = fields.getFieldCount();
- spec_store_.resize(nspecs_ * sizeof(RdataFields::FieldSpec));
+ spec_size_ = fields.getFieldDataSize();
+ spec_store_.resize(spec_size_);
void* cp_spec = &spec_store_[0];
memcpy(cp_spec, fields.getFieldSpecData(), spec_store_.size());
- spec_ptr_ = static_cast<RdataFields::FieldSpec*>(cp_spec);
+ spec_ptr_ = cp_spec;
data_length_ = fields.getDataLength();
- data_store_.assign(fields.getData(), fields.getData() + data_length_);
+ data_store_.resize(data_length_);
+ void* cp_data = &data_store_[0];
+ memcpy(cp_data, fields.getData(), data_store_.size());
// Vector guarantees that the elements are stored in continuous array
// in memory, so this is actually correct by the standard
- data_ptr_ = &data_store_[0];
+ data_ptr_ = cp_data;
}
void toWire(MessageRenderer& renderer) const {
- RdataFields(spec_ptr_, nspecs_,
+ RdataFields(spec_ptr_, spec_size_,
data_ptr_, data_length_).toWire(renderer);
}
private:
vector<unsigned char> spec_store_;
- const RdataFields::FieldSpec* spec_ptr_;
- unsigned int nspecs_;
- vector<uint8_t> data_store_;
- const uint8_t* data_ptr_;
+ vector<unsigned char> data_store_;
+ const void* spec_ptr_;
+ const void* data_ptr_;
+ unsigned int spec_size_;
size_t data_length_;
};
diff --git a/src/lib/dns/rdatafields.cc b/src/lib/dns/rdatafields.cc
index 196f3c7..9013d08 100644
--- a/src/lib/dns/rdatafields.cc
+++ b/src/lib/dns/rdatafields.cc
@@ -158,16 +158,19 @@ RdataFields::RdataFields(const Rdata& rdata) {
}
}
-RdataFields::RdataFields(const FieldSpec* fields, const unsigned int nfields,
- const uint8_t* data, const size_t data_length) :
- fields_(fields), nfields_(nfields), data_(data), data_length_(data_length),
+RdataFields::RdataFields(const void* fields, const unsigned int fields_length,
+ const void* data, const size_t data_length) :
+ fields_(static_cast<const FieldSpec*>(fields)),
+ nfields_(fields_length / sizeof(*fields_)),
+ data_(static_cast<const uint8_t*>(data)),
+ data_length_(data_length),
detail_(NULL)
{
if ((fields_ == NULL && nfields_ > 0) ||
(fields_ != NULL && nfields_ == 0)) {
isc_throw(InvalidParameter,
- "Inconsistent parameters for RdataFields: nfields ("
- << nfields_ << ") and fields conflict each other");
+ "Inconsistent parameters for RdataFields: fields_length ("
+ << fields_length << ") and fields conflict each other");
}
if ((data_ == NULL && data_length_ > 0) ||
(data_ != NULL && data_length_ == 0)) {
diff --git a/src/lib/dns/rdatafields.h b/src/lib/dns/rdatafields.h
index 4108703..f2e407b 100644
--- a/src/lib/dns/rdatafields.h
+++ b/src/lib/dns/rdatafields.h
@@ -107,20 +107,20 @@ getFieldSpecData()-> { compressible name { compressible name { other data
/// scenario:
/// \code // assume "rdata" is a reference type to Rdata
/// const RdataFields fields(rdata);
-/// const unsigned int nfields = fields.getFieldCount();
-/// memcpy(some_place, fields.getFieldSpecData(), nfields*sizeof(FieldSpec));
+/// const unsigned int fields_size = fields.getFieldDataSize();
+/// memcpy(some_place, fields.getFieldSpecData(), fields_size);
/// const size_t data_length = fields.getDataLength();
/// memcpy(other_place, fields.getData(), data_length);
-/// // (nfields and data_length should be stored somewhere, too)
+/// // (fields_size and data_length should be stored somewhere, too)
/// \endcode
///
/// Another typical usage is to render the stored data in the wire format
/// as efficiently as possible. The following code is an example of such
/// usage:
/// \code // assume "renderer" is of type MessageRenderer
-/// // retrieve data_length and nfields from the storage
-/// RdataFields(static_cast<const FieldSpec*>(some_place),
-/// nfields, other_place, data_length).toWire(renderer);
+/// // retrieve data_length and fields_size from the storage
+/// RdataFields(some_place, fields_size, other_place,
+/// data_length).toWire(renderer);
/// \endcode
///
/// <b>Notes to Users</b>
@@ -128,7 +128,7 @@ getFieldSpecData()-> { compressible name { compressible name { other data
/// The main purposes of this class is to help efficient operation
/// for some (limited classes of) performance sensitive application.
/// For this reason the interface and implementation rely on relatively
-/// lower-level, riskier primitives such as passing around bear pointers.
+/// lower-level, riskier primitives such as passing around bare pointers.
///
/// It is therefore discouraged to use this class for general purpose
/// applications that do not need to maximize performance in terms of either
@@ -278,7 +278,9 @@ public:
/// memory region is a valid representation of domain name.
/// Otherwise, a subsequent method call such as
/// <code>toWire(AbstractMessageRenderer&) const</code>
- /// may trigger an unexpected exception.
+ /// may trigger an unexpected exception. It also expects the fields reside
+ /// on address that is valid for them (eg. it has valid alignment), see
+ /// getFieldSpecData() for details.
///
/// It is the caller's responsibility to ensure this assumption.
/// In general, this constructor is expected to be used for serialized data
@@ -296,8 +298,8 @@ public:
/// \param data A pointer to memory region for the entire RDATA. This can
/// be NULL.
/// \param data_length The length of \c data in bytes.
- RdataFields(const FieldSpec* fields, const unsigned int nfields,
- const uint8_t* data, const size_t data_length);
+ RdataFields(const void* fields, const unsigned int fields_length,
+ const void* data, const size_t data_length);
/// The destructor.
~RdataFields();
@@ -315,26 +317,40 @@ public:
/// \brief Return a pointer to the RDATA encoded in the \c RdataFields.
///
+ /// The RdataFields holds ownership of the data.
+ ///
/// This method never throws an exception.
- const uint8_t* getData() const { return (data_); }
+ const void* getData() const { return (data_); }
- /// \brief Return the number of fields encoded in the RdataFields.
+ /// \brief Return the number of bytes the buffer returned by
+ /// getFieldSpecData() will occupy.
///
/// This method never throws an exception.
- unsigned int getFieldCount() const { return (nfields_); }
+ unsigned int getFieldDataSize() const { return (nfields_ *
+ sizeof *fields_); }
/// \brief Return a pointer to a sequence of \c FieldSpec for the
/// \c RdataFields.
///
- /// The data is just opaque internal representation, as a sequence
- /// of bytes (unsigned char * because of C/C++ aliasing rules).
+ /// This should be treated as an opaque internal representation you can
+ /// just store off somewhere and use it to construct a new RdataFields.
+ /// from it. If you are really interested, you can typecast it to
+ /// FieldSpec * (which is what it really is internally).
+ ///
+ /// The RdataFields holds ownership of the data.
+ ///
+ /// \note You should, however, be aware of alignment issues. The pointer
+ /// you pass to the constructor must be an address where the FieldSpec
+ /// can live. If you store it at a wrong address (eg. even one with
+ /// current implementation on most architectures), it might lead bad
+ /// things from slow access to SIGBUS. The easiest way is not to
+ /// interleave the fields with data from getData(). It is OK to place
+ /// all the fields first (even from multiple RdataFields) and then
+ /// place all the data after them.
///
/// This method never throws an exception.
- const uint8_t* getFieldSpecData() const {
- // Nasty cast, because C++ authors didn't read the C specification
- // about pointers. We can't return void* from it, that would break
- // aliasing rules.
- return ((const uint8_t*) fields_);
+ const void* getFieldSpecData() const {
+ return (fields_);
}
/// \brief Return the specification of the field identified by the given
diff --git a/src/lib/dns/tests/rdatafields_unittest.cc b/src/lib/dns/tests/rdatafields_unittest.cc
index 7ab7041..dec0dc4 100644
--- a/src/lib/dns/tests/rdatafields_unittest.cc
+++ b/src/lib/dns/tests/rdatafields_unittest.cc
@@ -71,7 +71,7 @@ RdataFieldsTest::constructCommonTests(const RdataFields& fields,
EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, expected_data,
expected_data_len, fields.getData(),
fields.getDataLength());
- EXPECT_EQ(1, fields.getFieldCount());
+ EXPECT_EQ(sizeof(RdataFields::FieldSpec), fields.getFieldDataSize());
EXPECT_EQ(RdataFields::DATA, fields.getFieldSpec(0).type);
EXPECT_EQ(4, fields.getFieldSpec(0).len);
@@ -93,7 +93,8 @@ TEST_F(RdataFieldsTest, constructFromRdata) {
TEST_F(RdataFieldsTest, constructFromParams) {
const RdataFields::FieldSpec spec(RdataFields::DATA, 4);
- const RdataFields fields(&spec, 1, in_a_data, sizeof(in_a_data));
+ const RdataFields fields(&spec, sizeof(spec), in_a_data,
+ sizeof(in_a_data));
constructCommonTests(fields, in_a_data, sizeof(in_a_data));
}
@@ -102,7 +103,7 @@ TEST_F(RdataFieldsTest, constructFromParams) {
//
void
RdataFieldsTest::constructCommonTestsNS(const RdataFields& fields) {
- EXPECT_EQ(1, fields.getFieldCount());
+ EXPECT_EQ(sizeof(RdataFields::FieldSpec), fields.getFieldDataSize());
EXPECT_EQ(RdataFields::COMPRESSIBLE_NAME, fields.getFieldSpec(0).type);
EXPECT_EQ(ns_name.getLength(), fields.getFieldSpec(0).len);
@@ -131,7 +132,7 @@ TEST_F(RdataFieldsTest, constructFromRdataNS) {
TEST_F(RdataFieldsTest, constructFromParamsNS) {
const RdataFields::FieldSpec spec(RdataFields::COMPRESSIBLE_NAME,
sizeof(ns_data));
- const RdataFields fields_ns(&spec, 1, ns_data, sizeof(ns_data));
+ const RdataFields fields_ns(&spec, sizeof(spec), ns_data, sizeof(ns_data));
constructCommonTestsNS(fields_ns);
}
@@ -142,7 +143,7 @@ void
RdataFieldsTest::constructCommonTestsTXT(const RdataFields& fields) {
// Since all fields are plain data, they are handled as a single data
// field.
- EXPECT_EQ(1, fields.getFieldCount());
+ EXPECT_EQ(sizeof(RdataFields::FieldSpec), fields.getFieldDataSize());
EXPECT_EQ(RdataFields::DATA, fields.getFieldSpec(0).type);
EXPECT_EQ(expected_wire.size(), fields.getFieldSpec(0).len);
@@ -173,8 +174,8 @@ TEST_F(RdataFieldsTest, constructFromParamsTXT) {
UnitTestUtil::readWireData("rdatafields3.wire", expected_wire);
expected_wire.erase(expected_wire.begin(), expected_wire.begin() + 2);
const RdataFields::FieldSpec spec(RdataFields::DATA, expected_wire.size());
- const RdataFields fields(&spec, 1, &expected_wire[0],
- expected_wire.size());
+ const RdataFields fields(&spec, sizeof(spec), &expected_wire[0],
+ expected_wire.size());
constructCommonTestsTXT(fields);
}
@@ -189,7 +190,7 @@ RdataFieldsTest::constructCommonTestsRRSIG(const RdataFields& fields) {
// this is a variable length field. In this test it's a 13-byte field.
// - a variable-length data field for the signature. In this tests
// it's a 15-byte field.
- ASSERT_EQ(3, fields.getFieldCount());
+ ASSERT_EQ(3 * sizeof(RdataFields::FieldSpec), fields.getFieldDataSize());
EXPECT_EQ(RdataFields::DATA, fields.getFieldSpec(0).type);
EXPECT_EQ(18, fields.getFieldSpec(0).len);
EXPECT_EQ(RdataFields::INCOMPRESSIBLE_NAME, fields.getFieldSpec(1).type);
@@ -239,7 +240,8 @@ TEST_F(RdataFieldsTest, constructFromParamsRRSIG) {
RdataFields::FieldSpec(RdataFields::INCOMPRESSIBLE_NAME, 13),
RdataFields::FieldSpec(RdataFields::DATA, 15)
};
- const RdataFields fields(specs, 3, &fields_wire[0], fields_wire.size());
+ const RdataFields fields(specs, sizeof(specs), &fields_wire[0],
+ fields_wire.size());
constructCommonTestsRRSIG(fields);
}
@@ -254,17 +256,15 @@ TEST_F(RdataFieldsTest, convertRdatatoParams) {
expected_wire.erase(expected_wire.begin(), expected_wire.begin() + 2);
// Copy the data in separate storage
- vector<uint8_t> spec_store(fields.getFieldCount() *
- sizeof(RdataFields::FieldSpec));
+ vector<uint8_t> spec_store(fields.getFieldDataSize());
void* cp_spec = &spec_store[0];
memcpy(cp_spec, fields.getFieldSpecData(), spec_store.size());
vector<uint8_t> data_store(fields.getDataLength());
memcpy(&data_store[0], fields.getData(), fields.getDataLength());
// Restore the data in the form of RdataFields
- const RdataFields fields_byparams(
- static_cast<const RdataFields::FieldSpec*>(cp_spec),
- fields.getFieldCount(), &data_store[0], fields.getDataLength());
+ const RdataFields fields_byparams(cp_spec, fields.getFieldDataSize(),
+ &data_store[0], fields.getDataLength());
// Check it's valid
constructCommonTestsRRSIG(fields_byparams);
@@ -275,7 +275,7 @@ TEST_F(RdataFieldsTest, convertRdatatoParams) {
//
void
RdataFieldsTest::constructCommonTestsOPT(const RdataFields& fields) {
- EXPECT_EQ(0, fields.getFieldCount());
+ EXPECT_EQ(0, fields.getFieldDataSize());
EXPECT_EQ(0, fields.getDataLength());
EXPECT_EQ((const uint8_t*) NULL, fields.getData());
fields.toWire(obuffer);
More information about the bind10-changes
mailing list