[2893] Addressed most of the review comments, except for doc update.

diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h
index 28f680bca0092cfd99c2b7b33141cc708f8edf04..d3218f36bd6125fcc8b6bbff045e53ba1a710707 100644 (file)
--- a/src/lib/dhcp/iface_mgr.h
+++ b/src/lib/dhcp/iface_mgr.h
@@ -353,7 +353,7 @@ public:
     ///
     /// @return Pointer to the first element of the read buffer or
     /// NULL if the buffer is empty.
-    uint8_t* getReadBufferPtr() const {
+    uint8_t* getReadBuffer() const {
         return (read_buffer_);
     }
 

diff --git a/src/lib/dhcp/pkt_filter_bpf.cc b/src/lib/dhcp/pkt_filter_bpf.cc
index ba8dcf6e0750ca779f9d9b5d126b248c3d0f3723..864cc37a3e2be81f78f7e828e03a7ceb9a225d65 100644 (file)
--- a/src/lib/dhcp/pkt_filter_bpf.cc
+++ b/src/lib/dhcp/pkt_filter_bpf.cc
@@ -177,7 +177,7 @@ PktFilterBPF::openSocket(Iface& iface,
     // the BPF device below.
     int fallback = openFallbackSocket(addr, port);
 
-    // Fallback has opened so, let's open the BPF device that we will be
+    // Fallback has opened, so let's open the BPF device that we will be
     // using for receiving and sending packets. The BPF device is opened
     // by opening a file /dev/bpf%d where %d is a number. There may be
     // devices already open so we will try them one by one and open the
@@ -190,7 +190,7 @@ PktFilterBPF::openSocket(Iface& iface,
         s << "/dev/bpf" << bpf_dev;
         sock = open(s.str().c_str(), O_RDWR, 0);
         if (sock < 0) {
-            // If device busy, try another one.
+            // If device is busy, try another one.
             if (errno == EBUSY) {
                 continue;
             }
@@ -231,7 +231,11 @@ PktFilterBPF::openSocket(Iface& iface,
         (ver.bv_minor < BPF_MINOR_VERSION)) {
         close(fallback);
         close(sock);
-        isc_throw(SocketConfigError, "Invalid BPF version");
+        isc_throw(SocketConfigError, "Invalid BPF version: "
+                  << ver.bv_major << "." << ver.bv_minor
+                  << " Expected at least version:"
+                  << BPF_MAJOR_VERSION << "."
+                  << BPF_MINOR_VERSION);;
     }
 
     // Get the size of the read buffer for this device. We will need to
@@ -323,14 +327,11 @@ PktFilterBPF::receive(const Iface& iface, const SocketInfo& socket_info) {
     // when the DHCP server is idle.
     int datalen;
     do {
-        datalen = recv(socket_info.fallbackfd_, iface.getReadBufferPtr(),
+        datalen = recv(socket_info.fallbackfd_, iface.getReadBuffer(),
                        iface.getReadBufferSize(), 0);
     } while (datalen > 0);
 
-    // Now that we finished getting data from the fallback socket, we
-    // have to get the data from the raw socket too.
-    memset(iface.getReadBufferPtr(), 0, iface.getReadBufferSize());
-    datalen = read(socket_info.sockfd_, iface.getReadBufferPtr(),
+    datalen = read(socket_info.sockfd_, iface.getReadBuffer(),
                    iface.getReadBufferSize());
     // If negative value is returned by read(), it indicates that an
     // error occured. If returned value is 0, no data was read from the
@@ -363,11 +364,11 @@ PktFilterBPF::receive(const Iface& iface, const SocketInfo& socket_info) {
 
         // Copy the BPF header.
         memcpy(static_cast<void*>(&bpfh),
-               static_cast<void*>(iface.getReadBufferPtr()),
+               static_cast<void*>(iface.getReadBuffer()),
                sizeof(bpfh));
 
         // Check if the captured data fit into the reminder of the buffer.
-        // Again, something is really wrong here.
+        // Again, something is really wrong here if it doesn't fit.
         if (offset + bpfh.bh_hdrlen + bpfh.bh_caplen > datalen) {
             isc_throw(SocketReadError, "packet received from the BPF device"
                       << " attached to interface " << iface.getName()
@@ -392,7 +393,7 @@ PktFilterBPF::receive(const Iface& iface, const SocketInfo& socket_info) {
     }
 
     // Skip the BPF header and create the buffer holding a frame.
-    InputBuffer buf(iface.getReadBufferPtr() + offset + bpfh.bh_hdrlen,
+    InputBuffer buf(iface.getReadBuffer() + offset + bpfh.bh_hdrlen,
                     datalen - bpfh.bh_hdrlen - offset);
 
 
@@ -468,7 +469,7 @@ PktFilterBPF::send(const Iface& iface, uint16_t sockfd, const Pkt4Ptr& pkt) {
     // Some interfaces may have no HW address - e.g. loopback interface.
     // For these interfaces the HW address length is 0. If this is the case,
     // then we will rely on the functions which construct the IP/UDP headers
-    // to provide a default HW addres. Otherwise, create the HW address
+    // to provide a default HW address. Otherwise, create the HW address
     // object using the HW address of the interface.
     if (iface.getMacLen() > 0) {
         HWAddrPtr hwaddr(new HWAddr(iface.getMac(), iface.getMacLen(),
@@ -476,7 +477,7 @@ PktFilterBPF::send(const Iface& iface, uint16_t sockfd, const Pkt4Ptr& pkt) {
         pkt->setLocalHWAddr(hwaddr);
     }
 
-    /// Local loopback interface requires special treatment. It doesn't
+    /// Loopback interface requires special treatment. It doesn't
     /// use the ethernet header but rather a 4-bytes long pseudo header
     /// holding an address family type (see bpf.c in OS sources).
     if (iface.flag_loopback_) {
@@ -497,9 +498,8 @@ PktFilterBPF::send(const Iface& iface, uint16_t sockfd, const Pkt4Ptr& pkt) {
 
     int result = write(sockfd, buf.getData(), buf.getLength());
     if (result < 0) {
-        std::cout << strerror(errno) << std::endl;
-        isc_throw(SocketWriteError, "failed to send DHCPv4 packet, errno="
-                  << errno << " (check errno.h)");
+        isc_throw(SocketWriteError, "failed to send DHCPv4 packet: "
+                  << strerror(errno));
     }
 
     return (0);

diff --git a/src/lib/dhcp/pkt_filter_bpf.h b/src/lib/dhcp/pkt_filter_bpf.h
index 04435b3770cb5c9d5d151a2aa819fee630d8aa35..bc9d80719ccf24ab844b82271a98c45822c05530 100644 (file)
--- a/src/lib/dhcp/pkt_filter_bpf.h
+++ b/src/lib/dhcp/pkt_filter_bpf.h
@@ -25,7 +25,7 @@ namespace dhcp {
 /// @brief Packet handling class using Berkeley Packet Filtering (BPF)
 ///
 /// The BPF is supported on the BSD-like operating systems. It allows for access
-/// to low level layers of the inbound and outbound packets. This is specifially
+/// to low level layers of the inbound and outbound packets. This is specifically
 /// useful when the DHCP server is allocating new address to the client.
 ///
 /// The response being sent to the client must include the HW address in the

diff --git a/src/lib/dhcp/tests/iface_mgr_unittest.cc b/src/lib/dhcp/tests/iface_mgr_unittest.cc
index d43e462b32c54d75dcf286b70683b5355b0150aa..641b87586237647a4a848879ba92e0c8142984b2 100644 (file)
--- a/src/lib/dhcp/tests/iface_mgr_unittest.cc
+++ b/src/lib/dhcp/tests/iface_mgr_unittest.cc
@@ -65,14 +65,14 @@ TEST(IfaceTest, readBuffer) {
     // The size of read buffer should initially be 0 and the returned
     // pointer should be NULL.
     ASSERT_EQ(0, iface.getReadBufferSize());
-    EXPECT_EQ(NULL, iface.getReadBufferPtr());
+    EXPECT_EQ(NULL, iface.getReadBuffer());
 
     // Let's resize the buffer.
     iface.resizeReadBuffer(256);
     // Check that the buffer has expected size.
     ASSERT_EQ(256, iface.getReadBufferSize());
     // The returned pointer should now be non-NULL.
-    uint8_t* buf_ptr = iface.getReadBufferPtr();
+    uint8_t* buf_ptr = iface.getReadBuffer();
     ASSERT_FALSE(buf_ptr == NULL);
 
     // Use the pointer to set some data.
@@ -81,7 +81,7 @@ TEST(IfaceTest, readBuffer) {
     }
 
     // Get the pointer again and validate the data.
-    buf_ptr = iface.getReadBufferPtr();
+    buf_ptr = iface.getReadBuffer();
     ASSERT_EQ(256, iface.getReadBufferSize());
     for (int i = 0; i < iface.getReadBufferSize(); ++i) {
         // Use assert so as it fails on the first failure, no need
@@ -1324,7 +1324,7 @@ TEST_F(IfaceMgrTest, setMatchingPacketFilter) {
 // The fallback socket should fail to open when there is another IP/UDP
 // socket bound to the same address and port. Failing to open the fallback
 // socket should preclude the raw socket from being open.
-TEST_F(IfaceMgrTest, checkPacketFilterLPFSocket) {
+TEST_F(IfaceMgrTest, checkPacketFilterRawSocket) {
     IOAddress loAddr("127.0.0.1");
     int socket1 = -1, socket2 = -1;
     // Create two instances of IfaceMgr.
@@ -1368,15 +1368,16 @@ TEST_F(IfaceMgrTest, checkPacketFilterLPFSocket) {
 
 #else
 
-// This non-Linux specific test checks whether it is possible to use
-// IfaceMgr to figure out which Pakcket Filter object should be
-// used when direct responses to hosts, having no address assigned
-// are desired or not desired. Since direct responses aren't supported
-// on systems other than Linux the function under test should always
-// set object of PktFilterInet type as current Packet Filter. This
-// object does not support direct responses. Once implementation is
-// added on systems other than BSD and Linux the OS specific version
-// of the test will be removed.
+// Note: This test will only run on non-Linux and non-BSD systems.
+// This test checks whether it is possible to use IfaceMgr to figure
+// out which Pakcket Filter object should be used when direct responses
+// to hosts, having no address assigned are desired or not desired.
+// Since direct responses aren't supported on systems other than Linux
+// and BSD the function under test should always set object of
+// PktFilterInet type as current Packet Filter. This object does not 
+//support direct responses. Once implementation is added on systems
+// other than BSD and Linux the OS specific version of the test will
+// be removed.
 TEST_F(IfaceMgrTest, setMatchingPacketFilter) {
 
     // Create an instance of IfaceMgr.