[2086] Address review comments

- style fix in return value
- updated header documentation
- added common label count checks to tests

diff --git a/src/lib/dns/labelsequence.cc b/src/lib/dns/labelsequence.cc
index 6e778f42ae27f92688145e6f404f55f9c42ae167..ff7366040443680ec3a9c2636ea3dffd72af5253 100644 (file)
--- a/src/lib/dns/labelsequence.cc
+++ b/src/lib/dns/labelsequence.cc
@@ -26,7 +26,7 @@ namespace dns {
 const uint8_t*
 LabelSequence::getData(size_t *len) const {
     *len = getDataLength();
-    return &(data_[offsets_[first_label_]]);
+    return (&data_[offsets_[first_label_]]);
 }
 
 size_t

diff --git a/src/lib/dns/labelsequence.h b/src/lib/dns/labelsequence.h
index c2149f0ce8b0e2ff386b0ecadd9e17977a763b8c..1a71d4ce23d49cc3a09fd932ba2c091a77eab778 100644 (file)
--- a/src/lib/dns/labelsequence.h
+++ b/src/lib/dns/labelsequence.h
@@ -21,24 +21,25 @@
 namespace isc {
 namespace dns {
 
-/// \brief Light-weight Accessor to Name object
+/// \brief Light-weight Accessor to data of Name object
 ///
 /// The purpose of this class is to easily match Names and parts of Names,
 /// without needing to copy the underlying data on each label strip.
 ///
-/// It can only work on existing Name objects, and the Name object MUST
+/// It can only work on existing Name objects, or data as provided by the
+/// Name object or another LabelSequence, and the data or Name MUST
 /// remain in scope during the entire lifetime of its associated
 /// LabelSequence(s).
 ///
 /// Upon creation of a LabelSequence, it records the offsets of the
 /// labels in the wireformat data of the Name. When stripLeft() or
 /// stripRight() is called on the LabelSequence, no changes in the
-/// Name's data occur, but the internal pointers of the
+/// original data occur, but the internal pointers of the
 /// LabelSequence are modified.
 ///
 /// LabelSequences can be compared to other LabelSequences, and their
 /// data can be requested (which then points to part of the original
-/// data of the associated Name object).
+/// data of the original Name object).
 ///
 class LabelSequence {
     // Name calls the private toText(bool) method of LabelSequence.
@@ -69,9 +70,9 @@ public:
     /// \note No validation is done on the given data upon construction;
     ///       use with care.
     ///
-    /// \param data The Name data, in wire format
-    /// \param offsets The offsets of the labels in the Name, as given
-    ///        by the Name object or another LabelSequence
+    /// \param data The raw data for the domain name, in wire format
+    /// \param offsets The offsets of the labels in the domain name data,
+    ///        as given by a Name object or another LabelSequence
     /// \param offsets_size The size of the offsets data
     LabelSequence(const uint8_t* data,
                   const uint8_t* offsets,
@@ -84,12 +85,13 @@ public:
 
     /// \brief Return the wire-format data for this LabelSequence
     ///
-    /// The data, is returned as a pointer to the original wireformat
-    /// data of the original Name object, and the given len value is
+    /// The data is returned as a pointer to (the part of) the original
+    /// wireformat data, from either the original Name object, or the
+    /// raw data given in the constructor, and the given len value is
     /// set to the number of octets that match this labelsequence.
     ///
     /// \note The data pointed to is only valid if the original Name
-    /// object is still in scope
+    /// object or data is still in scope
     ///
     /// \param len Pointer to a size_t where the length of the data
     ///        will be stored (in number of octets)
@@ -107,7 +109,7 @@ public:
     /// versa.
     ///
     /// \note The data pointed to is only valid if the original Name
-    /// object is still in scope
+    /// object or data is still in scope
     ///
     /// \return The length of the data of the label sequence in octets.
     size_t getDataLength() const;
@@ -149,7 +151,7 @@ public:
     /// \brief Remove labels from the end of this LabelSequence
     ///
     /// \note No actual memory is changed, this operation merely updates the
-    /// internal pointers based on the offsets in the Name object.
+    /// internal pointers based on the offsets originally provided.
     ///
     /// \exception OutOfRange if i is greater than or equal to the number
     ///           of labels currently pointed to by this LabelSequence
@@ -168,13 +170,13 @@ public:
     /// LabelSequence as a string.  The returned string ends with a dot
     /// '.' if the label sequence is absolute.
     ///
-    /// This function assumes the underlying name is in proper
+    /// This function assumes the underlying data is in proper
     /// uncompressed wire format.  If it finds an unexpected label
     /// character including compression pointer, an exception of class
     /// \c BadLabelType will be thrown.  In addition, if resource
     /// allocation for the result string fails, a corresponding standard
     /// exception will be thrown.
-    //
+    ///
     /// \return a string representation of the <code>LabelSequence</code>.
     std::string toText() const;
 
@@ -244,7 +246,7 @@ private:
 ///
 /// \param os A \c std::ostream object on which the insertion operation is
 /// performed.
-/// \param name The \c LabelSequence object output by the operation.
+/// \param label_sequence The \c LabelSequence object output by the operation.
 /// \return A reference to the same \c std::ostream object referenced by
 /// parameter \c os after the insertion operation.
 std::ostream&

diff --git a/src/lib/dns/tests/labelsequence_unittest.cc b/src/lib/dns/tests/labelsequence_unittest.cc
index 6aca1b92cacec08479db5471017c488df10caf99..bb59ee7690a90790c81adf21620228422fb8fd1f 100644 (file)
--- a/src/lib/dns/tests/labelsequence_unittest.cc
+++ b/src/lib/dns/tests/labelsequence_unittest.cc
@@ -707,6 +707,7 @@ TEST(LabelSequence, rawConstruction) {
     result = s1.compare(s3);
     EXPECT_EQ(isc::dns::NameComparisonResult::COMMONANCESTOR,
               result.getRelation());
+    EXPECT_EQ(2, result.getCommonLabels());
 
     s1.stripRight(1);
     s3.stripRight(1);
@@ -714,11 +715,13 @@ TEST(LabelSequence, rawConstruction) {
     result = s1.compare(s3);
     EXPECT_EQ(isc::dns::NameComparisonResult::COMMONANCESTOR,
               result.getRelation());
+    EXPECT_EQ(1, result.getCommonLabels());
 
     data[9] = 'f';
     result = s1.compare(s3);
     EXPECT_EQ(isc::dns::NameComparisonResult::NONE,
               result.getRelation());
+    EXPECT_EQ(0, result.getCommonLabels());
 }
 
 }