[2096] Merge development and review branch for trac2096

Fixing conflicts in
	src/lib/datasrc/memory/rdata_encoder.h
	src/lib/datasrc/memory/rdata_reader.cc
	src/lib/datasrc/memory/rdata_reader.h
	src/lib/datasrc/memory/tests/rdata_serialization_unittest.cc

src/lib/datasrc/memory/rdata_encoder.h

@@ -29,71 +29,6 @@
 
 #include <vector>
 
-/// \file rdata_encoder.h
-/// \brief Set of utility classes for encoding RDATA in memory efficient way.
-///
-/// This file defines a set of interfaces (classes, types, constants) to
-/// manipulate a given set of RDATA of the same type (normally associated with
-/// an RRset) that may be accompanied with RRSIGs in a memory efficient way.
-///
-/// The entire set of RDATA is stored in a packed form in a contiguous
-/// memory region.  It's opaque data, without containing non trivial
-/// data structures, so it can be located anywhere in the memory or even
-/// dumped to a file.
-///
-/// Two main classes are provided: one is
-/// \c isc::datasrc::memory::RdataEncoder, which allows
-/// the application to create encoded data for a set of RDATA;
-/// the other is \c isc::datasrc::memory::RdataReader, which provides an
-/// interface to iterate over encoded set of
-/// RDATA for purposes such as data lookups or rendering the data into the
-/// wire format to create a DNS message.
-///
-/// The actual encoding detail is private information to the implementation,
-/// and the application shouldn't assume anything about that except that
-/// each RDATA is considered to consist of one or more generic fields,
-/// and each field is typed as either opaque data or a domain name.
-/// A domain name field has additional attributes
-/// (see \c isc::datasrc::memory::RdataNameAttributes)
-/// so the application can change how the name should be handled in terms
-/// of the DNS protocol (e.g., whether it's subject to name compression).
-///
-/// The following are the current implementation of internal encoding, shown
-/// only for reference.  Applications must not assume this particular form
-/// for the encoded data; in fact, it can change in a future version of the
-/// implementation.
-/// \verbatim
-// The encoded data begin with a series of 16-bit length fields (values are
-// stored in the host byte order).  The sequence may be empty.
-// uint16_t n1_1: size of 1st variable len field (if any) of 1st RDATA
-// uint16_t n1_2: size of 2nd variable len field of 1st RDATA
-// ...
-// uint16_t nN_M: size of last (Mth) variable len field of last (Nth) RDATA
-// uint16_t ns1: size of 1st RRSIG (if any) data
-// ...
-// uint16_t nsL: size of last (Lth) RRSIG data
-// A sequence of packed data fields follows:
-// uint8_t[]: data field value, length specified by nI_J (in case it's
-//            variable-length) or by the per type field spec (in case it's
-//            fixed-length).
-// or
-// opaque data, LabelSequence::getSerializedLength() bytes: data for a name
-// uint8_t[ns1]: 1st RRSIG data
-// ...
-// uint8_t[nsL]: last RRSIG data
-// \endverbatim
-///
-/// As described above, this implementation treats RRSIGs as opaque data
-/// that don't contain any domain names.  Technically, it has a "signer"
-/// domain name field in the sense of RFC4034.  In practice, however, this
-/// field is essentially mere data; it's not subject to name compression,
-/// and since it's very likely to be a subdomain of (or equal to) the
-/// owner name of the corresponding RR (or, if used in a DNS message,
-/// some domain name that already appears before this field), so it won't
-/// be a target of name compression either.  By treating the entire RRSIG
-/// as single-field data we can make the implementation simpler, and probably
-/// make it faster in rendering it into a DNS message.
-
 namespace isc {
 namespace datasrc {
 namespace memory {

src/lib/datasrc/memory/rdata_field.h

@@ -20,9 +20,71 @@
 /// \file rdata_field.h
 ///
 /// This header should be considered private to the implementation and should
-/// not be used from outside.
+/// not be used included directly.
 ///
 /// It is used to share the definition of encoding for RRtypes.
+///
+/// These are types shared by classes in rdata_encoder.h and rdata_reader.h.
+/// The defyne a set of interfaces (classes, types, constants) to
+/// manipulate a given set of RDATA of the same type (normally associated with
+/// an RRset) that may be accompanied with RRSIGs in a memory efficient way.
+///
+/// The entire set of RDATA is stored in a packed form in a contiguous
+/// memory region.  It's opaque data, without containing non trivial
+/// data structures, so it can be located anywhere in the memory or even
+/// dumped to a file.
+///
+/// Two main classes are provided: one is
+/// \c isc::datasrc::memory::RdataEncoder, which allows
+/// the application to create encoded data for a set of RDATA;
+/// the isc::datasrc::memory::RdataReader provides an interface to iterate
+/// over encoded set of RDATA for purposes such as data lookups or rendering
+/// the data into the wire format to create a DNS message.
+///
+/// The actual encoding detail is private information to the implementation,
+/// and the application shouldn't assume anything about that except that
+/// each RDATA is considered to consist of one or more generic fields,
+/// and each field is typed as either opaque data or a domain name.
+/// A domain name field has additional attributes
+/// (see \c isc::datasrc::memory::RdataNameAttributes)
+/// so the application can change how the name should be handled in terms
+/// of the DNS protocol (e.g., whether it's subject to name compression).
+///
+/// The following are the current implementation of internal encoding, shown
+/// only for reference.  Applications must not assume this particular form
+/// for the encoded data; in fact, it can change in a future version of the
+/// implementation.
+/// \verbatim
+// The encoded data begin with a series of 16-bit length fields (values are
+// stored in the host byte order).  The sequence may be empty.
+// uint16_t n1_1: size of 1st variable len field (if any) of 1st RDATA
+// uint16_t n1_2: size of 2nd variable len field of 1st RDATA
+// ...
+// uint16_t nN_M: size of last (Mth) variable len field of last (Nth) RDATA
+// uint16_t ns1: size of 1st RRSIG (if any) data
+// ...
+// uint16_t nsL: size of last (Lth) RRSIG data
+// A sequence of packed data fields follows:
+// uint8_t[]: data field value, length specified by nI_J (in case it's
+//            variable-length) or by the per type field spec (in case it's
+//            fixed-length).
+// or
+// opaque data, LabelSequence::getSerializedLength() bytes: data for a name
+// uint8_t[ns1]: 1st RRSIG data
+// ...
+// uint8_t[nsL]: last RRSIG data
+// \endverbatim
+///
+/// As described above, this implementation treats RRSIGs as opaque data
+/// that don't contain any domain names.  Technically, it has a "signer"
+/// domain name field in the sense of RFC4034.  In practice, however, this
+/// field is essentially mere data; it's not subject to name compression,
+/// and since it's very likely to be a subdomain of (or equal to) the
+/// owner name of the corresponding RR (or, if used in a DNS message,
+/// some domain name that already appears before this field), so it won't
+/// be a target of name compression either.  By treating the entire RRSIG
+/// as single-field data we can make the implementation simpler, and probably
+/// make it faster in rendering it into a DNS message.
 
 namespace isc {
 namespace dns {

src/lib/datasrc/memory/rdata_reader.cc

@@ -20,37 +20,8 @@ namespace isc {
 namespace datasrc {
 namespace memory {
 
-void
-RdataReader::emptyNameAction(const LabelSequence&, unsigned) {
-    // Do nothing here. On purpose, it's not unfinished.
-}
-
-void
-RdataReader::emptyDataAction(const uint8_t*, size_t) {
-    // Do nothing here. On purpose, it's not unfinished.
-}
-
-RdataReader::Result::Result(const LabelSequence& label,
-                            unsigned attributes) :
-    label_(label),
-    data_(NULL),
-    size_(0),
-    type_(NAME),
-    compressible_((attributes & NAMEATTR_COMPRESSIBLE) != 0),
-    additional_((attributes & NAMEATTR_ADDITIONAL) != 0)
-{}
-
-RdataReader::Result::Result(const uint8_t* data, size_t size) :
-    label_(Name::ROOT_NAME()),
-    data_(data),
-    size_(size),
-    type_(DATA),
-    compressible_(false),
-    additional_(false)
-{}
-
 RdataReader::RdataReader(const RRClass& rrclass, const RRType& rrtype,
-                         const uint8_t* data,
+                         const void* data,
                          size_t rdata_count, size_t sig_count,
                          const NameAction& name_action,
                          const DataAction& data_action) :
@@ -60,13 +31,11 @@ RdataReader::RdataReader(const RRClass& rrclass, const RRType& rrtype,
     var_count_total_(spec_.varlen_count * rdata_count),
     sig_count_(sig_count),
     spec_count_(spec_.field_count * rdata_count),
-    // The casts, well, C++ decided it doesn't like completely valid
-    // and explicitly allowed cast in C, so we need to fool it through
-    // void.
-    lengths_(static_cast<const uint16_t*>(
-             static_cast<const void*>(data))), // The lenghts are stored first
+    // The lenghts are stored first
+    lengths_(reinterpret_cast<const uint16_t*>(data)),
     // And the data just after all the lengths
-    data_(data + (var_count_total_ + sig_count_) * sizeof(uint16_t)),
+    data_(reinterpret_cast<const uint8_t*>(data) +
+          (var_count_total_ + sig_count_) * sizeof(uint16_t)),
     sigs_(NULL)
 {
     rewind();
@@ -81,10 +50,11 @@ RdataReader::rewind() {
     sig_pos_ = 0;
 }
 
-RdataReader::Result
+RdataReader::Boundary
 RdataReader::nextInternal(const NameAction& name_action,
                           const DataAction& data_action)
 {
+
     if (spec_pos_ < spec_count_) {
         const RdataFieldSpec& spec(spec_.fields[(spec_pos_++) %
                                                 spec_.field_count]);
@@ -92,27 +62,41 @@ RdataReader::nextInternal(const NameAction& name_action,
             const LabelSequence sequence(data_ + data_pos_);
             data_pos_ += sequence.getSerializedLength();
             name_action(sequence, spec.name_attributes);
-            return (Result(sequence, spec.name_attributes));
         } else {
             const size_t length(spec.type == RdataFieldSpec::FIXEDLEN_DATA ?
-                                spec.fixeddata_len : lengths_[length_pos_++]);
-            const Result result(data_ + data_pos_, length);
+                                spec.fixeddata_len : lengths_[length_pos_ ++]);
+            const uint8_t* const pos = data_ + data_pos_;
             data_pos_ += length;
-            data_action(result.data(), result.size());
-            return (result);
+            data_action(pos, length);
         }
+        return (spec_pos_ % spec_.field_count == 0 ?
+                RDATA_BOUNDARY : NO_BOUNDARY);
     } else {
         sigs_ = data_ + data_pos_;
-        return (Result());
+        return (RRSET_BOUNDARY);
     }
 }
 
-RdataReader::Result
+RdataReader::Boundary
 RdataReader::next() {
     return (nextInternal(name_action_, data_action_));
 }
 
-RdataReader::Result
+namespace {
+
+void
+emptyNameAction(const LabelSequence&, unsigned) {
+    // Do nothing here.
+}
+
+void
+emptyDataAction(const void*, size_t) {
+    // Do nothing here.
+}
+
+}
+
+RdataReader::Boundary
 RdataReader::nextSig() {
     if (sig_pos_ < sig_count_) {
         if (sigs_ == NULL) {
@@ -123,7 +107,8 @@ RdataReader::nextSig() {
             const size_t spec_pos = spec_pos_;
             const size_t length_pos = length_pos_;
             // When the next() gets to the last item, it sets the sigs_
-            while (nextInternal(emptyNameAction, emptyDataAction)) {}
+            while (nextInternal(emptyNameAction, emptyDataAction) !=
+                   RRSET_BOUNDARY) {}
             assert(sigs_ != NULL);
             // Return the state
             data_pos_ = data_pos;
@@ -131,16 +116,16 @@ RdataReader::nextSig() {
             length_pos_ = length_pos;
         }
         // Extract the result
-        const Result result(sigs_ + sig_data_pos_, lengths_[var_count_total_ +
-                                                            sig_pos_]);
+        const size_t length = lengths_[var_count_total_ + sig_pos_];
+        const uint8_t* const pos = sigs_ + sig_data_pos_;
         // Move the position of iterator.
         sig_data_pos_ += lengths_[var_count_total_ + sig_pos_];
         ++sig_pos_;
         // Call the callback
-        data_action_(result.data(), result.size());
-        return (result);
+        data_action_(pos, length);
+        return (RDATA_BOUNDARY);
     } else {
-        return (Result());
+        return (RRSET_BOUNDARY);
     }
 }
 

src/lib/datasrc/memory/rdata_reader.h

@@ -34,74 +34,60 @@ namespace memory {
 
 /// \brief Class to read serialized rdata
 ///
-/// This class allows you to read the data encoded by RdataEncoder.
-/// It is rather low-level -- it provides sequence of data fields
-/// and names. It does not give you convenient Rdata or RRset class.
+/// This class allows you to read the data encoded by RDataEncoder.
+/// It is rather low-level -- it provides sequence of data fields.
+/// Each field is either opaque data, passed as a pointer and length,
+/// or a name, in the form of dns::LabelSequence (which is always
+/// absolute) and attributes.
 ///
-/// It allows two types of operation. First one is by providing callbacks
-/// to the constructor and then iterating by repeatedly calling next(), or
-/// calling iterate() once. The callbacks are then called with each part of
-/// the data.
+/// Conceptually, these fields correspond to consecutive regions in
+/// wire-format representation of the RDATA, varying the type of above
+/// two cases depending on whether the region corresponds to a domain
+/// name or other data.  For example, for an MX RDATA the field
+/// sequence will be
+/// - 2 bytes of opaque data (which corresponds to the MX preference)
+/// - a domain name (which corresponds to the MX name)
+/// If the encoded data contain multiple MX RDATAs, the same type of
+/// sequence continues for the number of RDATAs.  Note that the opaque
+/// data field does not always correspond to a specific RDATA field
+/// as is the 2-byte preference field of MX.  For example, the field
+/// sequence for an SOA RDATA in terms of `RdataEncoder` will be:
+/// - a domain name (which corresponds to the SOA MNAME)
+/// - a domain name (which corresponds to the SOA RNAME)
+/// - 20 bytes of opaque data (for the rest of fields)
+///
+/// So, if you want to construct a general purpose dns::Rdata object
+/// from the field sequence, you'll need to build the complete
+/// wire-format data, and then construct a dns::Rdata object from it.
+///
+/// To use it, contstruct it with the data you got from RDataEncoder,
+/// provide it with callbacks and then iterate through the data.
+/// The callbacks are called with the data fields contained in the
+/// data.
 ///
 /// \code
-/// void handleLabel(const dns::LabelSequence& label, unsigned int flags) {
+/// void handleName(const dns::LabelSequence& labels, unsigned int flags) {
 ///     ...
 /// }
-/// void handleData(const uint8_t* data, size_t size) {
+/// void handleData(const void* data, size_t size) {
 ///     ...
 /// }
 ///
 /// RdataReader reader(RRClass::IN(), RRType::AAAA(), size, data,
-///                    handleLabel, handleData);
+///                    &handleName, &handleData);
 /// reader.iterate();
 /// \endcode
 ///
-/// The other way is to use the return value of next() and loop through
-/// it manually. It has the inconvenience of having the type condition, but
-/// the code is in one place. The used way is matter of personal preferrence,
-/// there's no much difference on the technical side.
-///
-/// \code
-/// RdataReader reader(RRClass::IN(), RRType::AAAA(), size, data,
-///                    &handleLabel, handleData);
-/// RdataReader::Result data;
-/// while (data = reader.next()) {
-///     switch (data.type()) {
-///         case RdataReader::NAME:
-///             ...
-///             break;
-///         case RdataReader::DATA:
-///             ...
-///             break;
-///         default: assert(0); // Can not happen
-///     }
-/// }
-/// \endcode
-///
 /// \note It is caller's responsibility to pass valid data here. This means
 ///     the data returned by RdataEncoder and the corresponding class and type.
 ///     If this is not the case, all the kinds of pointer hell might get loose.
 class RdataReader {
 public:
     /// \brief Function called on each name encountered in the data.
-    typedef boost::function<void(const dns::LabelSequence&, unsigned)>
-        NameAction;
-
+    typedef boost::function<void(const dns::LabelSequence&,
+                                 RdataNameAttributes)> NameAction;
     /// \brief Function called on each data field in the data.
-    typedef boost::function<void(const uint8_t*, size_t)> DataAction;
-
-    /// \brief a NameAction that does nothing.
-    ///
-    /// This is a NameAction function that does nothing. It is used
-    /// as a default in the constructor.
-    static void emptyNameAction(const dns::LabelSequence& label,
-                                unsigned attributes);
-
-    /// \brief a DataAction that does nothing.
-    ///
-    /// This is a DataAction function that does nothing. It is used
-    /// as a default in the constructor.
-    static void emptyDataAction(const uint8_t* data, size_t size);
+    typedef boost::function<void(const void*, size_t)> DataAction;
 
     /// \brief Constructor
     ///
@@ -118,116 +104,33 @@ public:
     /// \param name_action The callback to be called on each encountered name.
     /// \param data_action The callback to be called on each data chunk.
     RdataReader(const dns::RRClass& rrclass, const dns::RRType& rrtype,
-                const uint8_t* data, size_t rdata_count, size_t sig_count,
-                const NameAction& name_action = &emptyNameAction,
-                const DataAction& data_action = &emptyDataAction);
-
-    /// \brief The type of data returned from this iteration.
-    enum DataType {
-        NAME, ///< This iteration returns domain label
-        DATA, ///< This iteration returns unstructuder data
-        END   ///< No more data to return
+                const void* data, size_t rdata_count, size_t sig_count,
+                const NameAction& name_action, const DataAction& data_action);
+
+    /// \brief Result of next() and nextSig()
+    ///
+    /// This specifies if there's any boundary in the data at the
+    /// place where the corresponding call to next() or nextSig()
+    /// finished.
+    enum Boundary {
+        NO_BOUNDARY,    ///< It is in the middle of Rdata
+        RDATA_BOUNDARY, ///< At the end of single Rdata
+        RRSET_BOUNDARY  ///< At the end of the RRset (past the end)
     };
 
-    /// \brief Data from one iteration
+    /// \brief Step to next data field.
     ///
-    /// Each time you call next() or nextSig(), it returns some data.
-    /// This holds the data.
+    /// Iterate over the next field and call appropriate hook (name_action
+    /// or data_action, depending on the type) as passed to the constructor.
     ///
-    /// It is valid only for as long as the RdataReader that returned it.
-    ///
-    /// All the methods can be called under any circumstances. However,
-    /// if the required property is not valid for the given type (eg.
-    /// when calling size() on type() == NAME), it returns some "empty"
-    /// value (0, NULL, or the like).
-    class Result {
-    public:
-        /// \brief Default constructor
-        ///
-        /// It creates an empty result (with no data) of type END.
-        Result() :
-            // TODO: Do we maybe want to have a static one to copy
-            // instead of constructing new one from the root Name?
-            label_(dns::Name::ROOT_NAME()),
-            data_(NULL),
-            size_(0),
-            type_(END),
-            compressible_(false),
-            additional_(false)
-        {}
-
-        /// \brief Constructor from a domain label
-        ///
-        /// Creates the NAME type result. Used internally from RdataReader.
-        ///
-        /// \param label The label to hold
-        /// \param attributes The attributes, as stored by the serialized
-        ///     data.
-        Result(const dns::LabelSequence& label, unsigned attributes);
-
-        /// \brief Constructor from data
-        ///
-        /// Creates the DATA type result. Used internally from RdataReader.
-        ///
-        /// \param data The data pointer to hold.
-        /// \param size The size to hold.
-        Result(const uint8_t* data, size_t size);
-
-        /// \brief The type of data returned.
-        DataType type() const { return (type_); }
-
-        /// \brief The raw data.
-        ///
-        /// This is only valid if type() == DATA.
-        const uint8_t* data() const { return (data_); }
-
-        /// \brief The size of the raw data.
-        ///
-        /// This is the number of bytes the data takes. It is valid only
-        /// if type() == DATA.
-        size_t size() const { return (size_); }
-
-        /// \brief The domain label.
-        ///
-        /// This holds the domain label. It is only valid if type() == NAME.
-        const dns::LabelSequence& label() const { return (label_); }
-
-        /// \brief Is the name in label() compressible?
-        ///
-        /// This is valid only if type() == NAME.
-        bool compressible() const { return (compressible_); }
-
-        /// \brief Does the name expect additional processing?
-        ///
-        /// This is valid only if type() == NAME.
-        bool additional() const { return (additional_); }
-
-        /// \brief If there are data returned.
-        ///
-        /// This returns if there are any data at all returned. This is
-        /// equivalent to action != END, but it allows for more convenient
-        /// code of a loop through the data.
-        operator bool() const {
-            return (type() != END);
-        }
-    private:
-        dns::LabelSequence label_;
-        const uint8_t* data_;
-        size_t size_;
-        DataType type_;
-        bool compressible_;
-        bool additional_;
-    };
-
-    /// \brief Step to next piece of data.
-    ///
-    /// This returns the next available data. Also, the apropriate hook
-    /// (name_action or data_action, depending on the data type) as passed
-    /// to the constructor is called.
-    ///
-    /// If there are no more data, a Result with type END is returned and
-    /// no callback is called.
-    Result next();
+    /// \return It returns NO_BOUNDARY if the next call to next() will process
+    ///     data of the same rdata as this one. RDATA_BOUNDARY is returned when
+    ///     this field is the last of the current rdata. If there are no more
+    ///     data to process, no hook is called and RRSET_BOUNDARY is returned.
+    ///     Therefore, at the end of the whole data, once it processes the last
+    ///     field and returns RDATA_BOUNDARY and then it returns RRSET_BOUNDARY
+    ///     on the next call.
+    Boundary next();
 
     /// \brief Call next() until the end.
     ///
@@ -235,24 +138,52 @@ public:
     /// It calls next until it reaches the end (it does not rewind before,
     /// therefore if you already called next() yourself, it does not start
     /// at the beginning).
-    ///
-    /// The method only makes sense if you set the callbacks in constructor.
     void iterate() {
-        while (next()) {}
+        while (next() != RRSET_BOUNDARY) { }
     }
-
-    /// \brief Step to next piece of RRSig data.
+    /// \brief Call next() until the end of current rdata.
+    ///
+    /// This is a convenience method to iterate until the end of current
+    /// rdata. Notice this may cause more than one field being processed,
+    /// as some rrtypes are more complex.
+    ///
+    /// \return If there was Rdata to iterate through.
+    bool iterateRdata() {
+        while (true) {
+            switch(next()) {
+                case NO_BOUNDARY: break;
+                case RDATA_BOUNDARY: return (true);
+                case RRSET_BOUNDARY: return (false);
+            }
+        }
+    }
+    /// \brief Step to next field of RRSig data.
     ///
     /// This is almost the same as next(), but it iterates through the
     /// associated RRSig data, not the data for the given RRType.
-    Result nextSig();
+    Boundary nextSig();
 
     /// \brief Iterate through all RRSig data.
     ///
     /// This is almost the same as iterate(), but it iterates through the
     /// RRSig data instead.
-    void iterateSig() {
-        while (nextSig()) {}
+    void iterateAllSigs() {
+        while (nextSig() != RRSET_BOUNDARY) { }
+    }
+    /// \brief Iterate through the current RRSig Rdata.
+    ///
+    /// This is almote the same as iterateRdata, except it is for single
+    /// signature Rdata.
+    ///
+    /// In practice, this should process one DATA field.
+    bool iterateSingleSig() {
+        while (true) {
+            switch(nextSig()) {
+                case NO_BOUNDARY: break;
+                case RDATA_BOUNDARY: return (true);
+                case RRSET_BOUNDARY: return (false);
+            }
+        }
     }
 
     /// \brief Rewind the iterator to the beginnig of data.
@@ -291,8 +222,8 @@ private:
     // The positions in data.
     size_t data_pos_, spec_pos_, length_pos_;
     size_t sig_pos_, sig_data_pos_;
-    Result nextInternal(const NameAction& name_action,
-                        const DataAction& data_action);
+    Boundary nextInternal(const NameAction& name_action,
+                          const DataAction& data_action);
 };
 
 }

src/lib/datasrc/memory/tests/rdata_serialization_unittest.cc

@@ -109,7 +109,7 @@ renderNameField(MessageRenderer* renderer, bool additional_required,
 }
 
 void
-renderDataField(MessageRenderer* renderer, const uint8_t* data,
+renderDataField(MessageRenderer* renderer, const void* data,
                 size_t data_len)
 {
     renderer->writeData(data, data_len);
@@ -167,13 +167,16 @@ public:
 
 // Used across more classes and scopes. But it's just uninteresting
 // constant.
-const Name dummy_name2("example.com");
+const Name &dummyName2() {
+    static Name result("example.com");
+    return (result);
+}
 
 bool
 additionalRequired(const RRType& type) {
     // The set of RR types that require additional section processing.
-    // We'll pass it to renderNameField to check the stored attribute matches
-    // our expectation.
+    // We'll use it to determine what value should the renderNameField get
+    // and, if the stored attributes are as expected.
     static std::set<RRType> need_additionals;
     if (need_additionals.empty()) {
         need_additionals.insert(RRType::NS());
@@ -305,15 +308,15 @@ public:
                           boost::bind(renderDataField, &renderer, _1, _2));
 
         // 2nd dummy name
-        renderer.writeName(dummy_name2);
+        renderer.writeName(dummyName2());
         // Finally, dump any RRSIGs in wire format.
         foreachRRSig(encoded_data, rrsiglen_list,
                      boost::bind(renderDataField, &renderer, _1, _2));
     }
 };
 
-// Decode using reader with the return value of next
-class NextDecoder {
+// Check using callbacks and calling next until the end.
+class CallbackDecoder {
 public:
     static void decode(const isc::dns::RRClass& rrclass,
                        const isc::dns::RRType& rrtype,
@@ -322,38 +325,18 @@ public:
                        MessageRenderer& renderer)
     {
         RdataReader reader(rrclass, rrtype, &encoded_data[0], rdata_count,
-                           sig_count);
-        RdataReader::Result field;
-        while ((field = reader.next())) {
-            switch (field.type()) {
-                case RdataReader::DATA:
-                    renderer.writeData(field.data(), field.size());
-                    break;
-                case RdataReader::NAME:
-                    renderer.writeName(field.label(), field.compressible());
-                    break;
-                default:
-                    FAIL();
-            }
-        }
-
-        renderer.writeName(dummy_name2);
-
-        while ((field = reader.nextSig())) {
-            switch (field.type()) {
-                case RdataReader::DATA:
-                    renderer.writeData(field.data(), field.size());
-                    break;
-                default: // There are also no NAME fields in RRSigs
-                    FAIL();
-            }
-        }
+                           sig_count,
+                           boost::bind(renderNameField, &renderer,
+                                       additionalRequired(rrtype), _1, _2),
+                           boost::bind(renderDataField, &renderer, _1, _2));
+        while (reader.next() != RdataReader::RRSET_BOUNDARY) { }
+        renderer.writeName(dummyName2());
+        while (reader.nextSig() != RdataReader::RRSET_BOUNDARY) { }
     }
 };
 
-// Decode using reader with the return value of next, but after the reader
-// was used and rewund.
-class RewundDecoder {
+// Check using callbacks and calling iterate.
+class IterateDecoder {
 public:
     static void decode(const isc::dns::RRClass& rrclass,
                        const isc::dns::RRType& rrtype,
@@ -361,42 +344,45 @@ public:
                        const vector<uint8_t>& encoded_data, size_t,
                        MessageRenderer& renderer)
     {
-        RdataReader reader(rrclass, rrtype, &encoded_data[0], rdata_count,
-                           sig_count);
-        // Use the reader first and rewind it
-        reader.iterateSig();
+        RdataReader reader(rrclass, rrtype, &encoded_data[0],
+                           rdata_count, sig_count,
+                           boost::bind(renderNameField, &renderer,
+                                       additionalRequired(rrtype), _1, _2),
+                           boost::bind(renderDataField, &renderer, _1, _2));
         reader.iterate();
-        reader.rewind();
-        RdataReader::Result field;
-        while ((field = reader.next())) {
-            switch (field.type()) {
-                case RdataReader::DATA:
-                    renderer.writeData(field.data(), field.size());
-                    break;
-                case RdataReader::NAME:
-                    renderer.writeName(field.label(), field.compressible());
-                    break;
-                default:
-                    FAIL();
-            }
-        }
+        renderer.writeName(dummyName2());
+        reader.iterateAllSigs();
+    }
+};
 
-        renderer.writeName(dummy_name2);
+namespace {
 
-        while ((field = reader.nextSig())) {
-            switch (field.type()) {
-                case RdataReader::DATA:
-                    renderer.writeData(field.data(), field.size());
-                    break;
-                default: // There are also no NAME fields in RRSigs
-                    FAIL();
-            }
-        }
+// Render the data to renderer, if one is set, or put it inside
+// a data buffer.
+void
+appendOrRenderData(vector<uint8_t>* where, MessageRenderer** renderer,
+                   const void* data, size_t size)
+{
+    if (*renderer != NULL) {
+        (*renderer)->writeData(data, size);
+    } else {
+        where->insert(where->end(), reinterpret_cast<const uint8_t*>(data),
+                      reinterpret_cast<const uint8_t*>(data) + size);
     }
-};
+}
 
-// Check using callbacks and calling next until the end.
-class CallbackDecoder {
+}
+
+// Similar to IterateDecoder, but it first iterates a little and rewinds
+// before actual rendering.
+class RewindAndDecode {
+private:
+    static void writeName(MessageRenderer** renderer,
+                          const LabelSequence& labels, unsigned attributes)
+    {
+        (*renderer)->writeName(labels,
+                               (attributes & NAMEATTR_COMPRESSIBLE) != 0);
+    }
 public:
     static void decode(const isc::dns::RRClass& rrclass,
                        const isc::dns::RRType& rrtype,
@@ -404,19 +390,30 @@ public:
                        const vector<uint8_t>& encoded_data, size_t,
                        MessageRenderer& renderer)
     {
-        RdataReader reader(rrclass, rrtype, &encoded_data[0], rdata_count,
-                           sig_count,
-                           boost::bind(renderNameField, &renderer,
-                                       additionalRequired(rrtype), _1, _2),
-                           boost::bind(renderDataField, &renderer, _1, _2));
-        while (reader.next()) {}
-        renderer.writeName(dummy_name2);
-        while (reader.nextSig()) {}
+        MessageRenderer dump; // A place to dump the extra data from before
+                              // actual rendering.
+        MessageRenderer* current = &dump;
+        vector<uint8_t> placeholder; // boost::bind does not like NULL
+        RdataReader reader(rrclass, rrtype, &encoded_data[0],
+                           rdata_count, sig_count,
+                           boost::bind(writeName, &current, _1, _2),
+                           boost::bind(appendOrRenderData, &placeholder,
+                                       &current, _1, _2));
+        // Iterate a little and rewind
+        reader.next();
+        reader.nextSig();
+        reader.rewind();
+        // Do the actual rendering
+        current = &renderer;
+        reader.iterate();
+        renderer.writeName(dummyName2());
+        reader.iterateAllSigs();
     }
 };
 
-// Check using callbacks and calling iterate.
-class IterateDecoder {
+// Decode using the iteration over one rdata each time.
+// We also count there's the correct count of Rdatas.
+class SingleIterateDecoder {
 public:
     static void decode(const isc::dns::RRClass& rrclass,
                        const isc::dns::RRType& rrtype,
@@ -429,9 +426,17 @@ public:
                            boost::bind(renderNameField, &renderer,
                                        additionalRequired(rrtype), _1, _2),
                            boost::bind(renderDataField, &renderer, _1, _2));
-        reader.iterate();
-        renderer.writeName(dummy_name2);
-        reader.iterateSig();
+        size_t actual_count = 0;
+        while (reader.iterateRdata()) {
+            actual_count ++;
+        }
+        EXPECT_EQ(rdata_count, actual_count);
+        actual_count = 0;
+        renderer.writeName(dummyName2());
+        while (reader.iterateSingleSig()) {
+            actual_count ++;
+        }
+        EXPECT_EQ(sig_count, actual_count);
     }
 };
 
@@ -441,19 +446,6 @@ public:
 // of rendering.
 template<bool start_data, bool start_sig>
 class HybridDecoder {
-private:
-    // Append data either to the renderer or to the vector passed.
-    // The double pointer is there so we can change the renderer to NULL
-    // and back during the test.
-    static void appendData(vector<uint8_t>* where, MessageRenderer** renderer,
-                           const uint8_t* data, size_t size)
-    {
-        if (*renderer != NULL) {
-            (*renderer)->writeData(data, size);
-        } else {
-            where->insert(where->end(), data, data + size);
-        }
-    }
 public:
     static void decode(const isc::dns::RRClass& rrclass,
                        const isc::dns::RRType& rrtype,
@@ -468,7 +460,8 @@ public:
                            rdata_count, sig_count,
                            boost::bind(renderNameField, &renderer,
                                        additionalRequired(rrtype), _1, _2),
-                           boost::bind(appendData, &data, &current, _1, _2));
+                           boost::bind(appendOrRenderData, &data, &current, _1,
+                                       _2));
         // The size matches
         EXPECT_EQ(encoded_data_len, reader.getSize());
         if (start_sig) {
@@ -484,28 +477,35 @@ public:
         // Now, we let all sigs to be copied to data. We disable the
         // renderer for this.
         current = NULL;
-        reader.iterateSig();
+        reader.iterateAllSigs();
         // Now return the renderer and render the rest of the data
         current = &renderer;
         reader.iterate();
         // Now, this should not break anything and should be valid, but should
         // return ends.
-        EXPECT_FALSE(reader.next());
-        EXPECT_FALSE(reader.nextSig());
+        EXPECT_EQ(RdataReader::RRSET_BOUNDARY, reader.next());
+        EXPECT_EQ(RdataReader::RRSET_BOUNDARY, reader.nextSig());
         // Render the name and the sigs
-        renderer.writeName(dummy_name2);
+        renderer.writeName(dummyName2());
         renderer.writeData(&data[0], data.size());
         // The size matches even after use
         EXPECT_EQ(encoded_data_len, reader.getSize());
     }
 };
 
-typedef ::testing::Types<ManualDecoderStyle, NextDecoder, RewundDecoder,
-                         CallbackDecoder, IterateDecoder,
+typedef ::testing::Types<ManualDecoderStyle,
+                         CallbackDecoder, IterateDecoder, SingleIterateDecoder,
                          HybridDecoder<true, true>, HybridDecoder<true, false>,
                          HybridDecoder<false, true>,
                          HybridDecoder<false, false> >
     DecoderStyles;
+// Each decoder style must contain a decode() method. Such method is expected
+// to decode the passed data, first render the Rdata into the passed renderer,
+// then write the dummyName2() there and write the RRSig data after that. It may
+// do other checks too.
+//
+// There are some slight differences to how to do the decoding, that's why we
+// have the typed test.
 TYPED_TEST_CASE(RdataEncodeDecodeTest, DecoderStyles);
 
 void
@@ -535,8 +535,8 @@ checkEncode(RRClass rrclass, RRType rrtype,
     // These two names will be rendered before and after the test RDATA,
     // to check in case the RDATA contain a domain name whether it's
     // compressed or not correctly.  The names in the RDATA should basically
-    // a subdomain of example.com, so it can be compressed due to dummy_name.
-    // Likewise, dummy_name2 should be able to be fully compressed due to
+    // a subdomain of example.com, so it can be compressed due to dummyName2().
+    // Likewise, dummyName2() should be able to be fully compressed due to
     // the name in the RDATA.
     const Name dummy_name("com");
 
@@ -549,7 +549,7 @@ checkEncode(RRClass rrclass, RRType rrtype,
     BOOST_FOREACH(const ConstRdataPtr& rdata, rdata_list) {
         rdata->toWire(expected_renderer_);
     }
-    expected_renderer_.writeName(dummy_name2);
+    expected_renderer_.writeName(dummyName2());
     BOOST_FOREACH(const ConstRdataPtr& rdata, rrsig_list) {
         rdata->toWire(expected_renderer_);
     }
@@ -653,6 +653,22 @@ addRdataMultiCommon(const vector<ConstRdataPtr>& rrsigs) {
     checkEncode(RRClass::IN(), RRType::NAPTR(), rdata_list_, 1, rrsigs);
 }
 
+void ignoreName(const LabelSequence&, unsigned) {
+}
+
+void
+checkLargeData(const in::DHCID* decoded, bool* called, const void* encoded,
+               size_t length)
+{
+    EXPECT_FALSE(*called); // Called exactly once
+    *called = true;
+
+    // Reconstruct the Rdata and check it.
+    isc::util::InputBuffer ib(encoded, length);
+    const in::DHCID reconstructed(ib, ib.getLength());
+    EXPECT_EQ(0, reconstructed.compare(*decoded));
+}
+
 TEST_F(RdataSerializationTest, encodeLargeRdata) {
     // There should be no reason for a large RDATA to fail in encoding,
     // but we check such a case explicitly.
@@ -666,10 +682,15 @@ TEST_F(RdataSerializationTest, encodeLargeRdata) {
     encodeWrapper(encoder_.getStorageLength());
 
     // The encoded data should be identical to the original one.
-    ASSERT_LT(sizeof(uint16_t), encoder_.getStorageLength());
-    isc::util::InputBuffer ib(&encoded_data_[2], encoded_data_.size() - 2);
-    const in::DHCID encoded_dhcid(ib, ib.getLength());
-    EXPECT_EQ(0, encoded_dhcid.compare(large_dhcid));
+    bool called = false;
+    RdataReader reader(RRClass::IN(), RRType::DHCID(), &encoded_data_[0], 1, 0,
+                       ignoreName, boost::bind(checkLargeData, &large_dhcid,
+                                               &called, _1, _2));
+    reader.iterate();
+    EXPECT_TRUE(called);
+    called = false;
+    reader.iterateAllSigs();
+    EXPECT_FALSE(called);
 }
 
 TYPED_TEST(RdataEncodeDecodeTest, addRdataMulti) {
@@ -762,6 +783,19 @@ TEST_F(RdataSerializationTest, badAddRdata) {
                  isc::BadValue);
 }
 
+void
+checkSigData(const ConstRdataPtr& decoded, bool* called, const void* encoded,
+             size_t length)
+{
+    EXPECT_FALSE(*called); // Called exactly once
+    *called = true;
+
+    // Reconstruct the RRSig and check it.
+    isc::util::InputBuffer ib(encoded, length);
+    const generic::RRSIG reconstructed(ib, ib.getLength());
+    EXPECT_EQ(0, reconstructed.compare(*decoded));
+}
+
 TEST_F(RdataSerializationTest, addSIGRdataOnly) {
     // Encoded data that only contain RRSIGs.  Mostly useless, but can happen
     // (in a partially broken zone) and it's accepted.
@@ -770,10 +804,14 @@ TEST_F(RdataSerializationTest, addSIGRdataOnly) {
     encodeWrapper(encoder_.getStorageLength());
     ASSERT_LT(sizeof(uint16_t), encoder_.getStorageLength());
 
-    // The encoded data should be identical to the given one.
-    isc::util::InputBuffer ib(&encoded_data_[2], encoded_data_.size() - 2);
-    const generic::RRSIG encoded_sig(ib, ib.getLength());
-    EXPECT_EQ(0, encoded_sig.compare(*rrsig_rdata_));
+    bool called = false;
+    RdataReader reader(RRClass::IN(), RRType::A(), &encoded_data_[0], 0, 1,
+                       ignoreName, boost::bind(checkSigData, rrsig_rdata_,
+                                               &called, _1, _2));
+    reader.iterate();
+    EXPECT_FALSE(called);
+    reader.iterateAllSigs();
+    EXPECT_TRUE(called);
 }
 
 TEST_F(RdataSerializationTest, badAddSIGRdata) {
@@ -793,47 +831,4 @@ TEST_F(RdataSerializationTest, badAddSIGRdata) {
     encoder_.start(RRClass::IN(), RRType::A());
     EXPECT_THROW(encoder_.addSIGRdata(big_sigrdata), RdataEncodingError);
 }
-
-// Test the result returns what it was constructed with.
-TEST_F(RdataSerializationTest, readerResult) {
-    // Default constructor
-    const RdataReader::Result empty;
-    // Everything should be at the "empty" values, type END
-    EXPECT_EQ(RdataReader::END, empty.type());
-    EXPECT_EQ(NULL, empty.data());
-    EXPECT_EQ(0, empty.size());
-    EXPECT_TRUE(empty.label().equals(LabelSequence(Name::ROOT_NAME())));
-    EXPECT_FALSE(empty);
-    EXPECT_FALSE(empty.compressible());
-    EXPECT_FALSE(empty.additional());
-    // Constructor from label sequence
-    const Name name("example.org");
-    const LabelSequence seq(name);
-    const RdataReader::Result compressible(seq, NAMEATTR_COMPRESSIBLE);
-    EXPECT_EQ(RdataReader::NAME, compressible.type());
-    EXPECT_EQ(NULL, compressible.data());
-    EXPECT_EQ(0, compressible.size());
-    EXPECT_TRUE(compressible.label().equals(seq));
-    EXPECT_TRUE(compressible);
-    EXPECT_TRUE(compressible.compressible());
-    EXPECT_FALSE(compressible.additional());
-    const RdataReader::Result incompressible(seq, NAMEATTR_ADDITIONAL);
-    EXPECT_EQ(RdataReader::NAME, incompressible.type());
-    EXPECT_EQ(NULL, incompressible.data());
-    EXPECT_EQ(0, incompressible.size());
-    EXPECT_TRUE(incompressible.label().equals(seq));
-    EXPECT_TRUE(incompressible);
-    EXPECT_FALSE(incompressible.compressible());
-    EXPECT_TRUE(incompressible.additional());
-    // Constructor from data
-    const uint8_t byte = 0;
-    const RdataReader::Result data(&byte, 1);
-    EXPECT_EQ(RdataReader::DATA, data.type());
-    EXPECT_EQ(&byte, data.data());
-    EXPECT_EQ(1, data.size());
-    EXPECT_TRUE(data.label().equals(LabelSequence(Name::ROOT_NAME())));
-    EXPECT_TRUE(data);
-    EXPECT_FALSE(data.compressible());
-    EXPECT_FALSE(data.additional());
-}
 }