[trac998] Tidy-up

src/lib/acl/ip_check.h

@@ -16,6 +16,7 @@
 #define __IP_CHECK_H
 
 #include <cassert>
+#include <functional>
 #include <iterator>
 #include <utility>
 #include <vector>
@@ -31,17 +32,26 @@
 #include <util/strutil.h>
 #include <exceptions/exceptions.h>
 
+// The contents of this file are:
+//
+// 1. Free functions used by other code in the file.
+// 2. Ipv4Check, the class handling the checking of IPV4 addresses.
+// 3. Ipv6Check, the class handling the checking of IPV6 addresses.
+// 4. IpCheck, the "external" interface and the one that should be used in
+//    code implementing ACL checks.  Detailed documentation of use can be
+//    found in this header for that class.
+
 namespace isc {
 namespace acl {
 
 // Free functions
 
-/// \brief Convert Mask Size to Mask
+/// \brief Convert mask size to mask
 ///
 /// Given a mask size and a data type, return a value of that data type with the
-/// most significant masksize bits set.  For example, if the data type is an
-/// unsigned eight-bit byte and the masksize is 3, the function would return
-/// an unsigned eight-bit byte with the binary value 11100000.
+/// most significant "masksize" bits set.  For example, if the data type is an
+/// uint8_t and the masksize is 3, the function would return a uint8_t holding
+/// the binary value 11100000.
 ///
 /// The function is templated on the data type of the mask.
 ///
@@ -53,36 +63,41 @@ namespace acl {
 template <typename T>
 T createNetmask(size_t masksize) {
 
-    if ((masksize > 0) && (masksize <= 8 * sizeof(T))) {
+    if (masksize == 0) {
+
+        // Although a mask size of zero is invalid for the IP ACL check
+        // specification, it simplifies logic elsewhere if this function is
+        // allowed to handle a mask size of 0.
+        return (0);
+
+    } else if (masksize <= 8 * sizeof(T)) {
 
         // In the following discussion:
         //
-        // w is the width of the data type T in bits
+        // w is the width of the data type T in bits.
         // m is the value of masksize, the number of most signifcant bits we
         // want to set.
+        // ** is exponentiation (i.e. 2**n is 2 raised to the power of n).
         //
-        // We note that the value of 2**m - 1 gives a result with the least
-        // significant m bits set and the most signficant (w-m) bits clear.
+        // We note that the value of 2**m - 1 gives a value with the least
+        // significant m bits set.  For a data type of width w, this means that
+        // the most signficant (w-m) bits clear.
         //
         // Hence the value 2**(w-m) - 1 gives a result with the least signficant
-        // w-m bits set and the most significant m bits clear.
-        //
-        // The 1's complement of this value gives is the result we want.
+        // w-m bits set and the most significant m bits clear.  The 1's
+        // complement of this value gives is the result we want.
         //
-        // Final note:  masksize is non-zero, so we are assured that no term in 
-        // the expression below will overflow.
-
+        // Final note: at this point in the logic, m is non-zero, so w-m < m.
+        // This means 1<<(w-m) will fit into a variable of width w bits.  In
+        // other words, in the expression below, no term will cause an integer
+        // overflow.
         return (~((1 << (8 * sizeof(T) - masksize)) - 1));
 
-    } else if (masksize == 0) {
-
-        // Simplifies logic elsewhere we we are allowed to cope with a mask
-        // size of 0.
-
-        return (0);
     }
 
-    isc_throw(isc::OutOfRange, "mask size must be between 0 and " <<
+    // Mask size is too large. (Note that masksize is unsigned, so can't be
+    // negative.)
+    isc_throw(isc::OutOfRange, "masksize argument must be between 0 and " <<
                                8 * sizeof(T));
 }
 
@@ -90,24 +105,29 @@ T createNetmask(size_t masksize) {
 ///
 /// Splits an IP address (given in the form of "xxxxxx/n" or "xxxxx" into a
 /// string representing the IP address and a number giving the size of the
-/// network mask in bits.
+/// network mask in bits. (In the latter case, the size of the network mask is
+/// equal to the width of the data type holding the address.) An exception will
+/// be thrown if the string format is invalid or if the network mask is larger
+/// than a given maximum value.
 ///
-/// An exception will be thrown if the string format is invalid.  N.B. This
-/// does NOT check that the address component is a valid IP address - only that
-/// some string is present.
+/// N.B. This function does NOT check that the address component is a valid IP
+/// address; this is done elsewhere in the address parsing process.
 ///
 /// \param addrmask Address and/or address/mask.  The string should be passed
 ///                 without leading or trailing spaces.
 /// \param maxmask  Default value of the mask size, used if no mask is given.
-///                 It is also the maximum permitted value of the mask.
+///                 It is also the maximum permitted value of the mask.  An
+///                 OutOfRange exception is thrown if the mask size value is
+///                 greater than this.
 ///
-/// \return Pair of (string, uint32) holding the address string and the mask
+/// \return Pair of (string, size_t) holding the address string and the mask
 ///         size value.
 
-std::pair<std::string, uint32_t>
-splitIpAddress(const std::string& addrmask, uint32_t maxmask) {
+std::pair<std::string, size_t>
+splitIpAddress(const std::string& addrmask, size_t maxmask) {
 
-    uint32_t masksize = maxmask;
+    // Set the default value for the mask size
+    size_t masksize = maxmask;
 
     // See if a mask size was given
     std::vector<std::string> components = isc::util::str::tokens(addrmask, "/");
@@ -138,28 +158,6 @@ splitIpAddress(const std::string& addrmask, uint32_t maxmask) {
 }
 
 
-/// \brief IP Base
-///
-/// Base class for both the Ipv4 and Ipv4 check classes.  The only thing this
-/// supplies over and above the Check class is a clone() method, to return
-/// a pointer to a newly-allocated copy of itself.
-
-template <typename Context>
-class IpBaseCheck : public Check<Context> {
-public:
-    /// \brief Clone Object
-    ///
-    /// Implemented by derived classes, this causes a class to create a copy
-    /// of itself and return a pointer to the object.
-    ///
-    /// \return Pointer to the cloned object.  It is the caller's responsibility
-    ///         to delete this object.
-    virtual IpBaseCheck* clone() const {
-        return (0);
-    }
-};
-
-
 
 /// \brief IPV4 Check
 ///
@@ -167,10 +165,21 @@ public:
 /// (IP address, network mask and a flag indicating whether the check should
 /// be for a match or for a non-match) and a given IP address.
 ///
-/// \param Context Structure holding address to be matched.
+/// The base Check class is templated on a context structure which is passed
+/// to the matches() method.  In Check, the method is declared abstract.  In
+/// this class, the method is concrete, but no declaration is provided.  To
+/// use the class for a given data type X say, a template specialisation of the
+/// method must be provided, i.e.
+/// \code
+/// template <> bool Ipv4Check<X>::matches(const X& context) const {
+/// :
+/// }
+/// \endcode
+///
+/// The Ipv4Check class should not be directly.  Instead, use IpCheck.
 
 template <typename Context>
-class Ipv4Check : public IpBaseCheck<Context> {
+class Ipv4Check : public Check<Context> {
 public:
     /// \brief IPV4 Constructor
     ///
@@ -198,23 +207,23 @@ public:
     /// given as a string of the form "a.b.c.d/n", where the "/n" part is
     /// optional.
     ///
-    /// \param address IP address and netmask in the form "a.b.c.d/n" (where
+    /// \param addrmask IP address and netmask in the form "a.b.c.d/n" (where
     ///        the "/n" part is optional).
     /// \param inverse If false (the default), matches() returns true if the
     ///        condition matches.  If true, matches() returns true if the
     ///        condition does not match.
-    Ipv4Check(const std::string& address, bool inverse = false) :
+    Ipv4Check(const std::string& addrmask, bool inverse = false) :
         address_(1), masksize_(32), inverse_(inverse), netmask_(0)
     {
-        // Split the address into address part and mask.
-        std::pair<std::string, uint32_t> result =
-            splitIpAddress(address, 8 * sizeof(uint32_t));
+        // Split into address part and mask.
+        std::pair<std::string, size_t> result =
+            splitIpAddress(addrmask, 8 * sizeof(uint32_t));
 
-        // Try to convert the address.
+        // Try to convert the address.  The result is in network-byte order.
         int status = inet_pton(AF_INET, result.first.c_str(), &address_);
         if (status == 0) {
             isc_throw(isc::InvalidParameter, "address/masksize of " <<
-                      address << " is not valid IPV4 address");
+                      addrmask << " is not valid IPV4 address");
         }
 
         // All done, so finish initialization.
@@ -222,8 +231,8 @@ public:
         setNetmask();
     }
 
-    // Default copy constructor and assignment operator are correct for this
-    // class.
+    // Default copy constructor and assignment operator is correct for this
+    // class (all member variables have appropriate operations defined).
 
     /// \brief Destructor
     virtual ~Ipv4Check() {}
@@ -232,7 +241,8 @@ public:
     ///
     /// Matches the passed argument to the condition stored here.  Different
     /// specialisations are provided for different argument types, so the
-    /// link will fail if used for a type for which no match is provided.
+    /// program link will fail if used for a type for which no match is
+    /// provided.
     ///
     /// \param context Information to be matched
     virtual bool matches(const Context& context) const;
@@ -269,38 +279,17 @@ public:
     }
     ///@}
 
-    /// \brief Set Network Mask
-    ///
-    /// Sets up the network mask from the mask size.
-    void setNetmask() {
-        // Validate that the mask is valid.
-        if ((masksize_ >= 1) && (masksize_ <=  8 * sizeof(uint32_t))) {
-
-            // Calculate the bitmask given by the number of bits.
-            netmask_ = isc::acl::createNetmask<uint32_t>(masksize_);
-
-            // ... and convert to network byte order.
-            netmask_ = htonl(netmask_);
-
-        } else {
-            isc_throw(isc::OutOfRange,
-                      "mask size of " << masksize_ << " is invalid " <<
-                      "for the data type which is " << sizeof(uint32_t) <<
-                      " bytes long");
-        }
-    }
-
     /// \brief Comparison
     ///
     /// This is the actual comparison function that checks the IP address passed
     /// to this class with the matching information in the class itself.  It is
     /// expected to be called from matches().
     ///
-    /// \param address Address (in network byte order) to match against the
-    ///                check condition in the class.
+    /// \param addr Address (in network byte order) to match against the
+    ///             check condition in the class.
     ///
     /// \return true if the address matches, false if it does not.
-    virtual bool compare(uint32_t address) const {
+    virtual bool compare(uint32_t addr) const {
 
         // To check that the address given matches the stored network address
         // and netmask, we check the simple condition that:
@@ -308,7 +297,7 @@ public:
         //     address_given & netmask_ == stored_address & netmask_
         //
         // However, we must return the negation of the result if inverse_ is
-        // set.  This leads to the truth table:
+        // set.  The appropriate truth table is:
         //
         // Result inverse_ Return
         // false  false    false
@@ -316,10 +305,35 @@ public:
         // true   false    true
         // true   true     false
         //
-        // ... which is an XOR function.  Although there is no explicit logical
-        /// XOR operator, with two bool arguments, "!=" serves that function.
+        // ... which is an XOR operation.  Although there is no explicit logical
+        /// XOR operator, with two bool arguments "!=" serves that function.
 
-        return (((address & netmask_) == (address_ & netmask_)) != inverse_);
+        return (((addr & netmask_) == (address_ & netmask_)) != inverse_);
+    }
+
+private:
+    /// \brief Set Network Mask
+    ///
+    /// Sets up the network mask from the mask size.  A value with the most
+    /// significant "mask size" bits is created and then stored in network-byte
+    /// order, the same way as addresses are presented and stored.
+    void setNetmask() {
+
+        // Validate that the mask is valid.
+        if ((masksize_ >= 1) && (masksize_ <=  8 * sizeof(uint32_t))) {
+
+            // Calculate the bitmask given by the number of bits.
+            netmask_ = isc::acl::createNetmask<uint32_t>(masksize_);
+
+            // ... and convert to network byte order.
+            netmask_ = htonl(netmask_);
+
+        } else {
+            isc_throw(isc::OutOfRange,
+                      "mask size of " << masksize_ << " is invalid " <<
+                      "for the data type which is " << sizeof(uint32_t) <<
+                      " bytes long");
+        }
     }
 
     // Member variables
@@ -332,32 +346,32 @@ public:
 
 
 
-// Used for an assertion check
-namespace {
-bool isNonZero(uint8_t i) {
-    return (i != 0);
-}
-} // Anonymous namespace
-
 /// \brief IPV6 Check
 ///
 /// This class performs a match between an IPv6 address specified in an ACL
 /// (IP address, network mask and a flag indicating whether the check should
 /// be for a match or for a non-match) and a given IP address.
 ///
-/// \param Context Structure holding address to be matched.
+/// Like Ipv4Check, the class is templated on the type of a context structure
+/// passed to the matches() method, and a template specialisation for that
+/// method must be supplied for the class to be used.  Also like Ipv4Check,
+/// the class should not be used directly, Ipcheck should be used instead.
 
 template <typename Context>
-class Ipv6Check : public IpBaseCheck<Context> {
-public:
-
-    // Size of array to old IPV6 address.
+class Ipv6Check : public Check<Context> {
+private:
+    // Size of array to hold and IPV6 address.
     static const size_t IPV6_SIZE = sizeof(struct in6_addr);
 
+public:
     /// \brief Default Constructor
+    ///
+    /// Only provided for use as a placeholder in array initializations etc.
+    /// There are no setXxxx() methods; an object of this class can be updated
+    /// by assignment.
     Ipv6Check() :
-        address_(IPV6_SIZE, 0), masksize_(0),
-        inverse_(false), netmask_(IPV6_SIZE, 0)
+        address_(IPV6_SIZE, 0), masksize_(0), inverse_(false),
+        netmask_(IPV6_SIZE, 0)
     {}
 
     /// \brief IPV6 Constructor
@@ -385,8 +399,7 @@ public:
     /// \brief String Constructor
     ///
     /// Constructs an IPv6 Check object from a network address and size of mask
-    /// given as a string of the form "a.b.c.d/n", where the "/n" part is
-    /// optional.
+    /// given as a string of the form <ip6-address>/n".
     ///
     /// \param address IP address and netmask in the form "<v6-address>/n"
     ///        (where the "/n" part is optional).
@@ -394,14 +407,15 @@ public:
     ///        condition matches.  If true, matches() returns true if the
     ///        condition does not match.
     Ipv6Check(const std::string& address, bool inverse = false) :
-        address_(IPV6_SIZE, 0), masksize_(8 * IPV6_SIZE),
-        inverse_(inverse), netmask_(IPV6_SIZE, 0)
+        address_(IPV6_SIZE, 0), masksize_(8 * IPV6_SIZE), inverse_(inverse),
+        netmask_(IPV6_SIZE, 0)
     {
         // Split the address into address part and mask.
-        std::pair<std::string, uint32_t> result =
+        std::pair<std::string, size_t> result =
             splitIpAddress(address, 8 * IPV6_SIZE);
 
-        // Try to convert the address.
+        // Try to convert the address.  If successful, the result is in
+        // network-byte order (most significant components at lower addresses).
         int status = inet_pton(AF_INET6, result.first.c_str(), &address_[0]);
         if (status == 0) {
             isc_throw(isc::InvalidParameter, "address/masksize of " <<
@@ -413,31 +427,8 @@ public:
         setNetmask();
     }
 
-    /// \brief Copy constructor
-    Ipv6Check(const Ipv6Check& other) : IpBaseCheck<Context>(other),
-        address_(other.address_.begin(), other.address_.end()),
-        masksize_(other.masksize_), inverse_(other.inverse_),
-        netmask_(other.netmask_.begin(), other.netmask_.end())
-    {}
-
-    /// \brief Assignment operator
-    Ipv6Check& operator=(const Ipv6Check& other) {
-        if (this != &other) {
-            // Copy parent
-            IpBaseCheck<Context>::operator=(other);
-
-            // Copy this object
-            address_.clear();
-            std::copy(other.address_.begin(), other.address_.end(),
-                      std::back_inserter(address_));
-            masksize_ = other.masksize_;
-            inverse_ = other.inverse_;
-            netmask_.clear();
-            std::copy(other.netmask_.begin(), other.netmask_.end(),
-                      std::back_inserter(netmask_));
-        }
-        return (*this);
-    }
+    // Default copy constructor and assignment operator is correct for this
+    // class (all member variables have equivalent operations).
 
     /// \brief Destructor
     virtual ~Ipv6Check() {}
@@ -446,7 +437,8 @@ public:
     ///
     /// Matches the passed argument to the condition stored here.  Different
     /// specialisations are provided for different argument types, so the
-    /// link will fail if used for a type for which no match is provided.
+    /// program will fail to compile if a required specialisation is not
+    /// provided.
     ///
     /// \param context Information to be matched
     virtual bool matches(const Context& context) const;
@@ -483,42 +475,6 @@ public:
     }
     ///@}
 
-    /// \brief Set Network Mask
-    ///
-    /// Sets up the network mask from the mask size.  This involves setting
-    /// mask in each byte of the network mask.
-    void setNetmask() {
-
-        // Validate that the mask is valid.
-        if ((masksize_ >= 1) && (masksize_ <=  8 * IPV6_SIZE)) {
-
-            // Loop, setting the bits in the set of mask bytes until all the
-            // specified bits have been used up.  The netmask array was
-            // initialized to zero in the constructor.
-            assert(std::find_if(netmask_.begin(), netmask_.end(), isNonZero) ==
-                   netmask_.end());
-
-            size_t bits_left = masksize_;   // Bits remaining to set
-            int i = -1;                     // To allow pre-incrementing
-            while (bits_left > 0) {
-                if (bits_left >= 8) {
-                    netmask_[++i] = ~0;  // All bits set
-                    bits_left -= 8;
-
-                } else if (bits_left > 0) {
-                    netmask_[++i] = createNetmask<uint8_t>(bits_left);
-                    bits_left = 0;
-
-                }
-            }
-        } else {
-            isc_throw(isc::OutOfRange,
-                      "mask size of " << masksize_ << " is invalid " <<
-                      "for the data type which is " << sizeof(uint32_t) <<
-                      " bytes long");
-        }
-    }
-
     /// \brief Comparison
     ///
     /// This is the actual comparison function that checks the IP address passed
@@ -541,11 +497,12 @@ public:
         // the network mask.  We stop at the first non-match (or when we run
         // out of bits in the network mask). (Note that the network mask
         // represents a contiguous set of bits.  As such, as soon as we find
-        // a netmask byte of zeroes, we have run part the part of the address
+        // a netmask byte of zeroes, we have run past the part of the address
         // where we need to match.
 
         bool match = true;
-        for (int i = 0; match && (i < IPV6_SIZE) && (netmask_[i] != 0); ++i){
+        for (int i = 0; match && (i < netmask_.size()) && (netmask_[i] != 0);
+             ++i) {
              match = ((testaddr[i] & netmask_[i]) ==
                       (address_[i] & netmask_[i]));
         }
@@ -554,6 +511,45 @@ public:
         return (match != inverse_);
     }
 
+private:
+    /// \brief Set Network Mask
+    ///
+    /// Sets up the network mask from the mask size.  This involves setting
+    /// an individual mask in each byte of the network mask.
+    void setNetmask() {
+
+        // Validate that the mask is valid.
+        if ((masksize_ >= 1) && (masksize_ <=  8 * IPV6_SIZE)) {
+
+            // Loop, setting the bits in the set of mask bytes until all the
+            // specified bits have been used up.  The netmask array was
+            // initialized to zero in the constructor, but in debug mode we
+            // can check that.
+            assert(std::find_if(netmask_.begin(), netmask_.end(),
+                   std::bind1st(std::not_equal_to<uint8_t>(), 0)) ==
+                   netmask_.end());
+
+            size_t bits_left = masksize_;   // Bits remaining to set
+            int i = -1;
+            while (bits_left > 0) {
+                if (bits_left >= 8) {
+                    netmask_[++i] = ~0;  // All bits set
+                    bits_left -= 8;
+
+                } else if (bits_left > 0) {
+                    netmask_[++i] = createNetmask<uint8_t>(bits_left);
+                    bits_left = 0;
+
+                }
+            }
+        } else {
+            isc_throw(isc::OutOfRange,
+                      "mask size of " << masksize_ << " is invalid " <<
+                      "for the data type which is " << IPV6_SIZE <<
+                      " bytes long");
+        }
+    }
+
     // Member variables
 
     std::vector<uint8_t>    address_;   ///< IPv6 address
@@ -570,10 +566,13 @@ public:
 /// (IP address (either V4 of V6), network mask and a flag indicating whether
 /// the check should be for a match or for a non-match) and a given IP address.
 ///
-/// \param Context Structure holding address to be matched.
+/// Derived from the base Check class, the class is templated on the Context
+/// parameter.  Internally, it uses the Ipv4Check and Ipv6Check classes and as a
+/// result requires that specializations for their matches() methods be provided
+/// for the template parameter.
 
 template <typename Context>
-class IpCheck : public IpBaseCheck<Context> {
+class IpCheck : public Check<Context> {
 public:
 
     /// \brief String Constructor
@@ -582,37 +581,46 @@ public:
     /// given as a string of the form "<address>/n", where the "/n" part is
     /// optional.
     ///
-    /// \param address IP address and netmask in the form "<address>/n"
-    ///        (where the "/n" part is optional).
+    /// \param address IP address and netmask in the form "<address>/n".  The
+    ///        "/n" part is optional but, if provided, must match the type
+    ///        of address specified.
     /// \param inverse If false (the default), matches() returns true if the
     ///        condition matches.  If true, matches() returns true if the
     ///        condition does not match.
-    IpCheck(const std::string& address, bool inverse = false) {
-        try {
-            check_.reset(new Ipv4Check<Context>(address, inverse));
-            isv4_ = true;
-        } catch (isc::Exception&) {
-            check_.reset(new Ipv6Check<Context>(address, inverse));
-            isv4_ = false;
-        }
+    IpCheck(const std::string& address, bool inverse = false) :
+        address_(address), inverse_(inverse)
+    {
+        createCheck();
     }
 
     /// \brief Copy constructor
-    IpCheck(const IpCheck<Context>& other) : IpBaseCheck<Context>(other),
-                                             check_(other.check_->clone())
-    {}
+    ///
+    /// \param other Object to be copied
+    IpCheck(const IpCheck<Context>& other) : Check<Context>(other),
+        address_(other.address_), inverse_(other.inverse_)
+    {
+        // The address_ and inverse_ objects are copied but, rather than copy
+        // the stored "check_" object (the type of which is indeterminate, as
+        // only a pointer to a base class is stored), it recreates the object
+        // from the stored constructor arguments.
+        createCheck();
+    }
 
     /// \brief Assignment operator
+    ///
+    /// \param other Object to be copied
+    ///
+    /// \return reference to current object
     IpCheck& operator=(const IpCheck& other) {
         if (this != &other) {
-            IpBaseCheck<Context>::operator=(other);
-            check_.reset(other.check_->clone());
+            // Similar to the copy constructor, this copies the constructor
+            // arguments from the source of the assignment and recreates the
+            // check object from them.
+            address_ = other.address_;
+            inverse_ = other.inverse_;
+            createCheck();
         }
-    }
-
-    /// \brief Clone
-    IpBaseCheck<Context>* clone() const {
-        return (new IpCheck<Context>(*this));
+        return (*this);
     }
 
     /// \brief Destructor
@@ -620,9 +628,10 @@ public:
 
     /// \brief The check itself
     ///
-    /// Matches the passed argument to the condition stored here.  Different
-    /// specialisations are provided for different argument types, so the
-    /// link will fail if used for a type for which no match is provided.
+    /// Matches the passed argument to the condition stored here by devolving
+    /// the check to the internal Ipv4Check or Ipv6Check object.  Appropriate
+    /// specialisations for the matches() method in those classes must be
+    /// provided for the program to link successfully.
     ///
     /// \param context Information to be matched
     virtual bool matches(const Context& context) const {
@@ -632,16 +641,30 @@ public:
     /// \brief Estimated cost
     ///
     /// Assume that the cost of the match is linear and depends on the number
-    /// of comparison operations.
+    /// of comparison operations.  The cost is obtained from the type of
+    /// check object stored.
     virtual unsigned cost() const {
         return (check_->cost());
     }
 
 private:
-    // Member variables
+    /// \brief Create check object
+    ///
+    /// Creates an instance of the actual check object (Ipv4Check or Ipv6Check)
+    /// that will be used to perform the checking.  It assumes that the member
+    /// variables are set appropriately.
+    void createCheck() {
+        try {
+            check_.reset(new Ipv4Check<Context>(address_, inverse_));
+        } catch (isc::Exception&) {
+            check_.reset(new Ipv6Check<Context>(address_, inverse_));
+        }
+    }
 
-    bool                                      isv4_;  ///< true if V4 address
-    boost::scoped_ptr<IpBaseCheck<Context> >  check_; ///< Check object
+    // Member variables
+    std::string         address_;   ///< Copy of constructor address string
+    bool                inverse_;   ///< Copy of constructor inverse flag
+    boost::scoped_ptr<Check<Context> >  check_; ///< Check object
 };
 
 } // namespace acl

src/lib/acl/tests/ip_check_unittest.cc

@@ -20,14 +20,40 @@
 using namespace isc::acl;
 using namespace std;
 
-// Provide specializations for a simple joint struct holding both an
-// IPV4 address and an IPV6 address
+// Simple struct holding either an IPV4 or IPV6 address.  This is the "Context"
+// used for the tests.
 
-typedef struct {
-    bool        isv4;
+struct GeneralAddress {
+    bool        isv4;           // true if it holds a v4 address
     uint32_t    v4addr;
     uint8_t     v6addr[16];
-} GeneralAddress;
+
+    GeneralAddress()
+    {}
+
+    // Convenience constructor for V4 address.  As it is not marked as
+    // explicit, it allows the automatic promotion of a uint32_t to a
+    // GeneralAddress data type in calls to matches().
+    GeneralAddress(uint32_t address) : isv4(true), v4addr(address)
+    {
+        fill(v6addr, v6addr + sizeof(v6addr), 0);   // Explicitly zero
+    }
+
+    // Convenience constructor for V6 address.  As it is not marked as
+    // explicit, it allows the automatic promotion of a vector<uint8_t> to a
+    // GeneralAddress data type in calls to matches().
+    GeneralAddress(const vector<uint8_t>& address) : isv4(false), v4addr(0) {
+        if (address.size() != sizeof(v6addr)) {
+            isc_throw(isc::InvalidParameter, "vector passed to GeneralAddress "
+                      "constructor is " << address.size() << " bytes long - it "
+                      "should be " << sizeof(v6addr) << " instead");
+        }
+        copy(address.begin(), address.end(), v6addr);
+    }
+};
+
+// Provide specializations of the Ipv4check and Ipv6Check matches() methods for
+// the GeneralAddress structure.
 
 namespace isc  {
 namespace acl {
@@ -45,14 +71,15 @@ bool Ipv6Check<GeneralAddress>::matches(const GeneralAddress& addr) const {
 
 /// *** Free Function Tests ***
 
+// Test the createNetmask() function.
 TEST(IpFunctionCheck, CreateNetmask) {
 
     // 8-bit tests: invalid arguments should throw.
     EXPECT_THROW(createNetmask<uint8_t>(9), isc::OutOfRange);
 
     // Check on all possible 8-bit values.  Use a signed type to generate a
-    // variable with the most significant bits set, as right-shifting it is
-    // guaranteed to introduce additional bits.
+    // variable with the most significant bits set (right-shifting will
+    // introduce additional bits).
     int8_t expected8 = 0x80;
     for (size_t i = 1; i <= 8; ++i, expected8 >>= 1) {
         EXPECT_EQ(static_cast<uint8_t>(expected8), createNetmask<uint8_t>(i));
@@ -62,7 +89,6 @@ TEST(IpFunctionCheck, CreateNetmask) {
     // Do the same for 32 bits.
     EXPECT_THROW(createNetmask<int32_t>(33), isc::OutOfRange);
 
-    // Check on all possible 8-bit values
     int32_t expected32 = 0x80000000;
     for (size_t i = 1; i <= 32; ++i, expected32 >>= 1) {
         EXPECT_EQ(static_cast<uint32_t>(expected32),
@@ -71,15 +97,16 @@ TEST(IpFunctionCheck, CreateNetmask) {
     EXPECT_EQ(0, createNetmask<uint32_t>(0));
 }
 
-TEST(IpFunctionCheck, SplitIp) {
+// Test the splitIpAddress() function.
+TEST(IpFunctionCheck, SplitIpAddress) {
     pair<string, uint32_t> result;
 
     result = splitIpAddress("192.0.2.1/24", 32);
     EXPECT_EQ(string("192.0.2.1"), result.first);
     EXPECT_EQ(24, result.second);
 
-    result = splitIpAddress("192.0.2.1/32", 32);
-    EXPECT_EQ(string("192.0.2.1"), result.first);
+    result = splitIpAddress("192.0.2.2/32", 32);
+    EXPECT_EQ(string("192.0.2.2"), result.first);
     EXPECT_EQ(32, result.second);
 
     result = splitIpAddress("2001:db8::/128", 128);
@@ -92,79 +119,29 @@ TEST(IpFunctionCheck, SplitIp) {
 }
 
 // *** IPV4 Tests ***
-
-// Declare a derived class to allow a definition of the match() method to be
-// provided.
-//
-// In this case, the match will check a uint32_t variable representing an IPV4
-// address.
-
-class DerivedV4Check : public Ipv4Check<GeneralAddress> {
-public:
-    // Basic (and default) constructor
-    DerivedV4Check(uint32_t address = 1, size_t masksize = 32,
-                   bool inverse = false) :
-                   Ipv4Check<GeneralAddress>(address, masksize, inverse)
-    {}
-
-    // String constructor
-    DerivedV4Check(const string& address, bool inverse = false) :
-        Ipv4Check<GeneralAddress>(address, inverse)
-    {}
-
-    // Copy constructor
-    DerivedV4Check(const DerivedV4Check& other) :
-        Ipv4Check<GeneralAddress>(other)
-    {}
-
-    // Assignment operator
-    DerivedV4Check& operator=(const DerivedV4Check& other) {
-        if (this != &other) {
-            Ipv4Check<GeneralAddress>::operator=(other);
-        }
-        return (*this);
-    }
-
-    // Clone method
-    virtual IpBaseCheck<GeneralAddress>* clone() const {
-        return (new DerivedV4Check(*this));
-    }
-
-    // Destructor
-    virtual ~DerivedV4Check()
-    {}
-
-    // Concrete implementation of abstract method
-    virtual bool matches(const uint32_t& context) const {
-        GeneralAddress gen;
-        gen.isv4 = true;
-        gen.v4addr = context;
-        return (Ipv4Check<GeneralAddress>::matches(gen));  // Call parent method
-    }
-};
-
 // Check that a default constructor can be instantiated.
 
 TEST(Ipv4Check, V4DefaultConstructor) {
-    DerivedV4Check acl1;
+    Ipv4Check<GeneralAddress> acl1;
 
     // The test is needed to avoid the unused variable causing a warning or
     // getting optimised away.
     EXPECT_EQ(1, acl1.getAddress());
 }
 
-// Check that the constructor stores the elements correctly.  As the address
-// is provided in the correct (network-byte order) format, the real test is
-// checking that the network mask is stored in the correct byte order.
+// Check that the constructor stores the elements correctly and, for the
+// address and mask, in network-byte order.
 
 TEST(Ipv4Check, V4ConstructorAddress) {
-    DerivedV4Check acl1(0x12345678);
+    // Address is presented in network byte order in constructor, so not
+    // conversion is needed for this test.
+    Ipv4Check<GeneralAddress> acl1(0x12345678);
     EXPECT_EQ(0x12345678, acl1.getAddress());
 }
 
 TEST(Ipv4Check, V4ConstructorMask) {
     // Valid values. Address of "1" is used as a placeholder
-    DerivedV4Check acl1(1, 1);
+    Ipv4Check<GeneralAddress> acl1(1, 1);
 
     // The mask is stored in network byte order, so the pattern expected must
     // also be converted to network byte order for the comparison to succeed.
@@ -172,49 +149,52 @@ TEST(Ipv4Check, V4ConstructorMask) {
     EXPECT_EQ(expected, acl1.getNetmask());
     EXPECT_EQ(1, acl1.getMasksize());
 
-    DerivedV4Check acl2(1, 24);
+    Ipv4Check<GeneralAddress> acl2(1, 24);
     expected = htonl(0xffffff00);
     EXPECT_EQ(expected, acl2.getNetmask());
     EXPECT_EQ(24, acl2.getMasksize());
 
     // ... and some invalid network masks
-    EXPECT_THROW(DerivedV4Check(1, 0), isc::OutOfRange);
-    EXPECT_THROW(DerivedV4Check(1, 33), isc::OutOfRange);
+    EXPECT_THROW(Ipv4Check<GeneralAddress>(1, 0), isc::OutOfRange);
+    EXPECT_THROW(Ipv4Check<GeneralAddress>(1, 33), isc::OutOfRange);
 }
 
 TEST(Ipv4Check, V4ConstructorInverse) {
     // Valid values. Address/mask of "1" is used as a placeholder
-    DerivedV4Check acl1(1, 1);
+    Ipv4Check<GeneralAddress> acl1(1, 1);
     EXPECT_FALSE(acl1.getInverse());
 
-    DerivedV4Check acl2(1, 1, true);
+    Ipv4Check<GeneralAddress> acl2(1, 1, true);
     EXPECT_TRUE(acl2.getInverse());
 
-    DerivedV4Check acl3(1, 1, false);
+    Ipv4Check<GeneralAddress> acl3(1, 1, false);
     EXPECT_FALSE(acl3.getInverse());
 }
 
 TEST(Ipv4Check, V4StringConstructor) {
-    DerivedV4Check acl1("192.168.132.255");
-    uint32_t expected = htonl(0xc0a884ff);
+    Ipv4Check<GeneralAddress> acl1("192.0.2.255");
+    uint32_t expected = htonl(0xc00002ff);
     EXPECT_EQ(expected, acl1.getAddress());
     EXPECT_EQ(32, acl1.getMasksize());
 
-    DerivedV4Check acl2("192.0.2.0/24");
+    Ipv4Check<GeneralAddress> acl2("192.0.2.0/24");
     expected = htonl(0xc0000200);
     EXPECT_EQ(expected, acl2.getAddress());
     EXPECT_EQ(24, acl2.getMasksize());
 
-    EXPECT_THROW(DerivedV4Check("192.0.2.0/0"), isc::OutOfRange);
-    EXPECT_THROW(DerivedV4Check("192.0.2.0/33"), isc::OutOfRange);
-    EXPECT_THROW(DerivedV4Check("192.0.2.0/24/3"), isc::InvalidParameter);
-    EXPECT_THROW(DerivedV4Check("192.0.2.0/ww"), isc::InvalidParameter);
-    EXPECT_THROW(DerivedV4Check("aa.255.255.0/ww"), isc::InvalidParameter);
+    EXPECT_THROW(Ipv4Check<GeneralAddress>("192.0.2.0/0"), isc::OutOfRange);
+    EXPECT_THROW(Ipv4Check<GeneralAddress>("192.0.2.0/33"), isc::OutOfRange);
+    EXPECT_THROW(Ipv4Check<GeneralAddress>("192.0.2.0/24/3"),
+                 isc::InvalidParameter);
+    EXPECT_THROW(Ipv4Check<GeneralAddress>("192.0.2.0/ww"),
+                 isc::InvalidParameter);
+    EXPECT_THROW(Ipv4Check<GeneralAddress>("aa.255.255.0/ww"),
+                 isc::InvalidParameter);
 }
 
 TEST(Ipv4Check, V4CopyConstructor) {
-    DerivedV4Check acl1("127.0.0.1/16", true);
-    DerivedV4Check acl2(acl1);
+    Ipv4Check<GeneralAddress> acl1("192.0.2.1/24", true);
+    Ipv4Check<GeneralAddress> acl2(acl1);
 
     EXPECT_EQ(acl1.getAddress(), acl2.getAddress());
     EXPECT_EQ(acl1.getMasksize(), acl2.getMasksize());
@@ -223,8 +203,8 @@ TEST(Ipv4Check, V4CopyConstructor) {
 }
 
 TEST(Ipv4Check, V4AssignmentOperator) {
-    DerivedV4Check acl1("127.0.0.1/16", true);
-    DerivedV4Check acl2("192.1.2.3/12", false);
+    Ipv4Check<GeneralAddress> acl1("192.0.2.0/24", true);
+    Ipv4Check<GeneralAddress> acl2("192.0.2.128/25", false);
 
     acl2 = acl1;
     EXPECT_EQ(acl1.getAddress(), acl2.getAddress());
@@ -233,19 +213,10 @@ TEST(Ipv4Check, V4AssignmentOperator) {
     EXPECT_EQ(acl1.getInverse(), acl2.getInverse());
 }
 
-TEST(IPv4Check, V4Clone) {
-    DerivedV4Check acl1("127.0.0.1/16", true);
-
-    boost::scoped_ptr<DerivedV4Check> acl2(
-            static_cast<DerivedV4Check*>(acl1.clone()));
-    EXPECT_EQ(acl1.getAddress(), acl2->getAddress());
-    EXPECT_EQ(acl1.getMasksize(), acl2->getMasksize());
-    EXPECT_EQ(acl1.getNetmask(), acl2->getNetmask());
-    EXPECT_EQ(acl1.getInverse(), acl2->getInverse());
-}
-
 // Check that the comparison works - note that "matches" just calls the
-// internal compare() code.
+// internal compare() code. (Also note that the argument to matches() will be
+// automatically converted to the GeneralAddress data type used for the tests
+// because of its constructor taking a uint32_t argument.
 //
 // As before, note that addresses passed to the class are expected to be in
 // network-byte order.  Therefore for the comparisons to work as expected, we
@@ -253,19 +224,19 @@ TEST(IPv4Check, V4Clone) {
 
 TEST(Ipv4Check, V4Compare) {
     // Exact address - match if given address matches stored address.
-    DerivedV4Check acl1(htonl(0x23457f13), 32);
+    Ipv4Check<GeneralAddress> acl1(htonl(0x23457f13), 32);
     EXPECT_TRUE(acl1.matches(htonl(0x23457f13)));
     EXPECT_FALSE(acl1.matches(htonl(0x23457f12)));
     EXPECT_FALSE(acl1.matches(htonl(0x13457f13)));
 
     // Exact address - match if address does not match stored address
-    DerivedV4Check acl2(htonl(0x23457f13), 32, true);
+    Ipv4Check<GeneralAddress> acl2(htonl(0x23457f13), 32, true);
     EXPECT_FALSE(acl2.matches(htonl(0x23457f13)));
     EXPECT_TRUE(acl2.matches(htonl(0x23457f12)));
     EXPECT_TRUE(acl2.matches(htonl(0x13457f13)));
 
     // Match if the address matches a mask
-    DerivedV4Check acl3(htonl(0x23450000), 16);
+    Ipv4Check<GeneralAddress> acl3(htonl(0x23450000), 16);
     EXPECT_TRUE(acl3.matches(htonl(0x23450000)));
     EXPECT_TRUE(acl3.matches(htonl(0x23450001)));
     EXPECT_TRUE(acl3.matches(htonl(0x2345ffff)));
@@ -273,7 +244,7 @@ TEST(Ipv4Check, V4Compare) {
     EXPECT_FALSE(acl3.matches(htonl(0x2346ffff)));
 
     // Match if the address does not match a mask
-    DerivedV4Check acl4(htonl(0x23450000), 16, true);
+    Ipv4Check<GeneralAddress> acl4(htonl(0x23450000), 16, true);
     EXPECT_FALSE(acl4.matches(htonl(0x23450000)));
     EXPECT_FALSE(acl4.matches(htonl(0x23450001)));
     EXPECT_FALSE(acl4.matches(htonl(0x2345ffff)));
@@ -284,49 +255,9 @@ TEST(Ipv4Check, V4Compare) {
 
 
 // *** IPV6 Tests ***
-//
-// Declare a derived class to allow a definition of the match() method to be
-// provided.
-//
-// In this case, the match will check a vector of uint8_t s representing an IPV6
-// address.
-
-class DerivedV6Check : public Ipv6Check<GeneralAddress> {
-public:
-    // default constructor
-    DerivedV6Check() : Ipv6Check<GeneralAddress>()
-    {}
-
-    // Basic constructor
-    DerivedV6Check(const uint8_t* address, size_t masksize = 128,
-                   bool inverse = false) :
-                   Ipv6Check<GeneralAddress>(address, masksize, inverse)
-    {}
-
-    // String constructor
-    DerivedV6Check(const string& address, bool inverse = false) :
-        Ipv6Check<GeneralAddress>(address, inverse)
-    {}
-
-    // Clone method
-    virtual IpBaseCheck<GeneralAddress>* clone() const {
-        return (new DerivedV6Check(*this));
-    }
-
-    // Destructor
-    virtual ~DerivedV6Check()
-    {}
-
-    // Concrete implementation of abstract method
-    virtual bool matches(const vector<uint8_t>& context) const {
-        GeneralAddress gen;
-        gen.isv4 = false;
-        copy(context.begin(), context.end(), gen.v6addr);
-        return (Ipv6Check<GeneralAddress>::matches(gen));  // Parent method
-    }
-};
 
 // Some constants used in the tests
+
 static const char* V6ADDR_1_STRING = "2001:0db8:1122:3344:5566:7788:99aa:bbcc";
 static const uint8_t V6ADDR_1[] = {
     0x20, 0x01, 0x0d, 0xb8, 0x11, 0x22, 0x33, 0x44,
@@ -387,7 +318,7 @@ static const uint8_t MASK_128[] = {
 // Check that a default constructor can be instantiated.
 
 TEST(Ipv6Check, V6DefaultConstructor) {
-    DerivedV6Check acl1;
+    Ipv6Check<GeneralAddress> acl1;
 
     // The test is needed to avoid the unused variable causing a warning or
     // getting optimised away.
@@ -395,7 +326,7 @@ TEST(Ipv6Check, V6DefaultConstructor) {
 }
 
 TEST(Ipv6Check, V6ConstructorAddress) {
-    DerivedV6Check acl1(V6ADDR_1);
+    Ipv6Check<GeneralAddress> acl1(V6ADDR_1);
     vector<uint8_t> stored = acl1.getAddress();
     EXPECT_EQ(sizeof(V6ADDR_1), stored.size());
     EXPECT_TRUE(equal(stored.begin(), stored.end(), V6ADDR_1));
@@ -404,70 +335,72 @@ TEST(Ipv6Check, V6ConstructorAddress) {
 TEST(Ipv6Check, V6ConstructorMask) {
 
     // Valid masks...
-    DerivedV6Check acl1(V6ADDR_1, 1);
+    Ipv6Check<GeneralAddress> acl1(V6ADDR_1, 1);
     vector<uint8_t> stored = acl1.getNetmask();
     EXPECT_EQ(sizeof(MASK_1), stored.size());
     EXPECT_TRUE(equal(stored.begin(), stored.end(), MASK_1));
 
-    DerivedV6Check acl2(V6ADDR_1, 8);
+    Ipv6Check<GeneralAddress> acl2(V6ADDR_1, 8);
     stored = acl2.getNetmask();
     EXPECT_TRUE(equal(stored.begin(), stored.end(), MASK_8));
 
-    DerivedV6Check acl3(V6ADDR_1, 48);
+    Ipv6Check<GeneralAddress> acl3(V6ADDR_1, 48);
     stored = acl3.getNetmask();
     EXPECT_TRUE(equal(stored.begin(), stored.end(), MASK_48));
 
-    DerivedV6Check acl4(V6ADDR_1, 51);
+    Ipv6Check<GeneralAddress> acl4(V6ADDR_1, 51);
     stored = acl4.getNetmask();
     EXPECT_TRUE(equal(stored.begin(), stored.end(), MASK_51));
 
-    DerivedV6Check acl5(V6ADDR_1, 128);
+    Ipv6Check<GeneralAddress> acl5(V6ADDR_1, 128);
     stored = acl5.getNetmask();
     EXPECT_TRUE(equal(stored.begin(), stored.end(), MASK_128));
 
     // ... and some invalid network masks
-    EXPECT_THROW(DerivedV6Check(V6ADDR_1, 0), isc::OutOfRange);
-    EXPECT_THROW(DerivedV6Check(V6ADDR_1, 129), isc::OutOfRange);
+    EXPECT_THROW(Ipv6Check<GeneralAddress>(V6ADDR_1, 0), isc::OutOfRange);
+    EXPECT_THROW(Ipv6Check<GeneralAddress>(V6ADDR_1, 129), isc::OutOfRange);
 }
 
 TEST(Ipv6Check, V6ConstructorInverse) {
     // Valid values. Address/mask of "1" is used as a placeholder
-    DerivedV6Check acl1(V6ADDR_1, 1);
+    Ipv6Check<GeneralAddress> acl1(V6ADDR_1, 1);
     EXPECT_FALSE(acl1.getInverse());
 
-    DerivedV6Check acl2(V6ADDR_1, 1, true);
+    Ipv6Check<GeneralAddress> acl2(V6ADDR_1, 1, true);
     EXPECT_TRUE(acl2.getInverse());
 
-    DerivedV6Check acl3(V6ADDR_1, 1, false);
+    Ipv6Check<GeneralAddress> acl3(V6ADDR_1, 1, false);
     EXPECT_FALSE(acl3.getInverse());
 }
 
 TEST(Ipv6Check, V6StringConstructor) {
-    DerivedV6Check acl1(V6ADDR_1_STRING);
+    Ipv6Check<GeneralAddress> acl1(V6ADDR_1_STRING);
     vector<uint8_t> address = acl1.getAddress();
     EXPECT_EQ(128, acl1.getMasksize());
     EXPECT_TRUE(equal(address.begin(), address.end(), V6ADDR_1));
 
-    DerivedV6Check acl2(string(V6ADDR_2_STRING) + string("/48"));
+    Ipv6Check<GeneralAddress> acl2(string(V6ADDR_2_STRING) + string("/48"));
     address = acl2.getAddress();
     EXPECT_EQ(48, acl2.getMasksize());
     EXPECT_TRUE(equal(address.begin(), address.end(), V6ADDR_2));
 
-    DerivedV6Check acl3("::1");
+    Ipv6Check<GeneralAddress> acl3("::1");
     address = acl3.getAddress();
     EXPECT_EQ(128, acl3.getMasksize());
     EXPECT_TRUE(equal(address.begin(), address.end(), V6ADDR_3));
 
-    EXPECT_THROW(DerivedV6Check("::1/0"), isc::OutOfRange);
-    EXPECT_THROW(DerivedV6Check("::1/129"), isc::OutOfRange);
-    EXPECT_THROW(DerivedV6Check("::1/24/3"), isc::InvalidParameter);
-    EXPECT_THROW(DerivedV6Check("2001:0db8::abcd/ww"), isc::InvalidParameter);
-    EXPECT_THROW(DerivedV6Check("2xx1:0db8::abcd/32"), isc::InvalidParameter);
+    EXPECT_THROW(Ipv6Check<GeneralAddress>("::1/0"), isc::OutOfRange);
+    EXPECT_THROW(Ipv6Check<GeneralAddress>("::1/129"), isc::OutOfRange);
+    EXPECT_THROW(Ipv6Check<GeneralAddress>("::1/24/3"), isc::InvalidParameter);
+    EXPECT_THROW(Ipv6Check<GeneralAddress>("2001:0db8::abcd/ww"),
+                 isc::InvalidParameter);
+    EXPECT_THROW(Ipv6Check<GeneralAddress>("2xx1:0db8::abcd/32"),
+                 isc::InvalidParameter);
 }
 
 TEST(Ipv6Check, V6CopyConstructor) {
-    DerivedV6Check acl1(string(V6ADDR_2_STRING) + string("/52"));
-    DerivedV6Check acl2(acl1);
+    Ipv6Check<GeneralAddress> acl1(string(V6ADDR_2_STRING) + string("/52"));
+    Ipv6Check<GeneralAddress> acl2(acl1);
 
     vector<uint8_t> acl1_address = acl1.getAddress();
     vector<uint8_t> acl2_address = acl1.getAddress();
@@ -489,8 +422,8 @@ TEST(Ipv6Check, V6CopyConstructor) {
 }
 
 TEST(Ipv6Check, V6AssignmentOperator) {
-    DerivedV6Check acl1(string(V6ADDR_2_STRING) + string("/52"));
-    DerivedV6Check acl2(string(V6ADDR_1_STRING) + string("/48"));
+    Ipv6Check<GeneralAddress> acl1(string(V6ADDR_2_STRING) + string("/52"));
+    Ipv6Check<GeneralAddress> acl2(string(V6ADDR_1_STRING) + string("/48"));
 
     acl2 = acl1;
 
@@ -513,63 +446,38 @@ TEST(Ipv6Check, V6AssignmentOperator) {
     EXPECT_EQ(acl1.getInverse(), acl2.getInverse());
 }
 
-TEST(Ipv6Check, V6Clone) {
-    DerivedV6Check acl1(string(V6ADDR_2_STRING) + string("/52"));
-    boost::scoped_ptr<DerivedV6Check> acl2(
-        static_cast<DerivedV6Check*>(acl1.clone()));
-
-    vector<uint8_t> acl1_address = acl1.getAddress();
-    vector<uint8_t> acl2_address = acl2->getAddress();
-    EXPECT_EQ(sizeof(V6ADDR_1), acl1_address.size());
-    EXPECT_EQ(acl1_address.size(), acl2_address.size());
-    EXPECT_TRUE(equal(acl1_address.begin(), acl1_address.end(),
-                acl2_address.begin()));
-
-    EXPECT_EQ(acl1.getMasksize(), acl2->getMasksize());
-
-    vector<uint8_t> acl1_netmask = acl1.getNetmask();
-    vector<uint8_t> acl2_netmask = acl2->getNetmask();
-    EXPECT_EQ(sizeof(V6ADDR_1), acl1_netmask.size());
-    EXPECT_EQ(acl1_netmask.size(), acl2_netmask.size());
-    EXPECT_TRUE(equal(acl1_netmask.begin(), acl1_netmask.end(),
-                acl2_netmask.begin()));
-
-    EXPECT_EQ(acl1.getInverse(), acl2->getInverse());
-}
-
 TEST(Ipv6Check, V6Compare) {
-    // Set up some data.  The constant doesn't depend on the template parameter,
-    // so use a type name that's short.
-    vector<uint8_t> v6addr_2(V6ADDR_2, V6ADDR_2 + DerivedV6Check::IPV6_SIZE);
-    vector<uint8_t> v6addr_2_48(V6ADDR_2_48,
-                                V6ADDR_2_48 + DerivedV6Check::IPV6_SIZE);
-    vector<uint8_t> v6addr_2_52(V6ADDR_2_52,
-                                V6ADDR_2_52 + DerivedV6Check::IPV6_SIZE);
-    vector<uint8_t> v6addr_3(V6ADDR_3, V6ADDR_3 + DerivedV6Check::IPV6_SIZE);
+    // Set up some data.  
+    vector<uint8_t> v6addr_2(V6ADDR_2, V6ADDR_2 + sizeof(V6ADDR_2));
+    vector<uint8_t> v6addr_2_48(V6ADDR_2_48, V6ADDR_2_48 + sizeof(V6ADDR_2_48));
+    vector<uint8_t> v6addr_2_52(V6ADDR_2_52, V6ADDR_2_52 + sizeof(V6ADDR_2_52));
+    vector<uint8_t> v6addr_3(V6ADDR_3, V6ADDR_3 + sizeof(V6ADDR_3));
 
     // Exact address - match if given address matches stored address.
-    DerivedV6Check acl1(string(V6ADDR_2_STRING) + string("/128"));
+    Ipv6Check<GeneralAddress> acl1(string(V6ADDR_2_STRING) + string("/128"));
     EXPECT_TRUE(acl1.matches(v6addr_2));
     EXPECT_FALSE(acl1.matches(v6addr_2_52));
     EXPECT_FALSE(acl1.matches(v6addr_2_48));
     EXPECT_FALSE(acl1.matches(v6addr_3));
 
     // Exact address - match if address does not match stored address
-    DerivedV6Check acl2(string(V6ADDR_2_STRING) + string("/128"), true);
+    Ipv6Check<GeneralAddress> acl2(string(V6ADDR_2_STRING) + string("/128"),
+                                   true);
     EXPECT_FALSE(acl2.matches(v6addr_2));
     EXPECT_TRUE(acl2.matches(v6addr_2_52));
     EXPECT_TRUE(acl2.matches(v6addr_2_48));
     EXPECT_TRUE(acl2.matches(v6addr_3));
 
     // Match if the address matches a mask
-    DerivedV6Check acl3(string(V6ADDR_2_STRING) + string("/52"));
+    Ipv6Check<GeneralAddress> acl3(string(V6ADDR_2_STRING) + string("/52"));
     EXPECT_TRUE(acl3.matches(v6addr_2));
     EXPECT_TRUE(acl3.matches(v6addr_2_52));
     EXPECT_FALSE(acl3.matches(v6addr_2_48));
     EXPECT_FALSE(acl3.matches(v6addr_3));
 
     // Match if the address does not match a mask
-    DerivedV6Check acl4(string(V6ADDR_2_STRING) + string("/52"), true);
+    Ipv6Check<GeneralAddress> acl4(string(V6ADDR_2_STRING) + string("/52"),
+                              true);
     EXPECT_FALSE(acl4.matches(v6addr_2));
     EXPECT_FALSE(acl4.matches(v6addr_2_52));
     EXPECT_TRUE(acl4.matches(v6addr_2_48));
@@ -632,3 +540,35 @@ TEST(IpCheck, V6Test) {
     copy(V6ADDR_2, V6ADDR_2 + sizeof(V6ADDR_2), test.v6addr);
     EXPECT_FALSE(acl2.matches(test));
 }
+
+// Check copy constructor and assignment operator for V4 address
+TEST(IpCheck, V4Copying) {
+    IpCheck<GeneralAddress> acl("192.168.132.255/16");
+    GeneralAddress test;
+    test.isv4 = true;
+    test.v4addr = htonl(0xc0a884ff);
+
+    IpCheck<GeneralAddress> acl1(acl);
+    EXPECT_TRUE(acl1.matches(test));
+
+    IpCheck<GeneralAddress> acl2("192.255.132.255/32");
+    EXPECT_FALSE(acl2.matches(test));
+    acl2 = acl;
+    EXPECT_TRUE(acl2.matches(test));
+}
+
+// Check copy constructor and assignment operator for V6 address
+TEST(IpCheck, V6Copying) {
+    IpCheck<GeneralAddress> acl(string(V6ADDR_2_STRING) + string("/52"));
+    GeneralAddress test;
+    test.isv4 = false;
+    copy(V6ADDR_2, V6ADDR_2 + sizeof(V6ADDR_2), test.v6addr);
+
+    IpCheck<GeneralAddress> acl1(acl);
+    EXPECT_TRUE(acl1.matches(test));
+
+    IpCheck<GeneralAddress> acl2(V6ADDR_3_STRING);
+    EXPECT_FALSE(acl2.matches(test));
+    acl2 = acl;
+    EXPECT_TRUE(acl2.matches(test));
+}