BIND 10 trac2546, updated. 52d965040ccb6e919a34abd95240fee3718c0247 [2546] Address further issues pointed out in review
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Dec 12 15:32:24 UTC 2012
The branch, trac2546 has been updated
via 52d965040ccb6e919a34abd95240fee3718c0247 (commit)
from a69ebe7341bfbbc01037e71e66b0c6e7121ca6cd (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 52d965040ccb6e919a34abd95240fee3718c0247
Author: Stephen Morris <stephen at isc.org>
Date: Wed Dec 12 15:30:31 2012 +0000
[2546] Address further issues pointed out in review
-----------------------------------------------------------------------
Summary of changes:
src/lib/dhcp/pkt4.cc | 44 +++++++++++++++++++++++------------
src/lib/dhcp/pkt4.h | 10 ++++----
src/lib/dhcp/tests/pkt4_unittest.cc | 11 ++++++++-
3 files changed, 44 insertions(+), 21 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/dhcp/pkt4.cc b/src/lib/dhcp/pkt4.cc
index 63ab513..15c885c 100644
--- a/src/lib/dhcp/pkt4.cc
+++ b/src/lib/dhcp/pkt4.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2011 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2012 Internet Systems Consortium, Inc. ("ISC")
//
// Permission to use, copy, modify, and/or distribute this software for any
// purpose with or without fee is hereby granted, provided that the above
@@ -79,6 +79,9 @@ Pkt4::Pkt4(const uint8_t* data, size_t len)
isc_throw(OutOfRange, "Truncated DHCPv4 packet (len=" << len
<< ") received, at least " << DHCPV4_PKT_HDR_LEN
<< " is expected.");
+
+ } else if (data == NULL) {
+ isc_throw(InvalidParameter, "data buffer passed to Pkt4 is NULL");
}
data_.resize(len);
@@ -159,7 +162,7 @@ Pkt4::unpack() {
// this is *NOT* DHCP packet. It does not have any DHCPv4 options. In
// particular, it does not have magic cookie, a 4 byte sequence that
// differentiates between DHCP and BOOTP packets.
- isc_throw(InvalidOperation, "Recevied BOOTP packet. BOOTP is not supported.");
+ isc_throw(InvalidOperation, "Received BOOTP packet. BOOTP is not supported.");
}
if (bufferIn.getLength() - bufferIn.getPosition() < 4) {
@@ -179,8 +182,8 @@ Pkt4::unpack() {
bufferIn.readVector(optsBuffer, opts_len);
LibDHCP::unpackOptions4(optsBuffer, options_);
- // TODO: check will need to be called separately, so hooks can be called after
- // packet is parsed, but before its content is verified
+ // @todo check will need to be called separately, so hooks can be called
+ // after the packet is parsed, but before its content is verified
check();
}
@@ -188,8 +191,9 @@ void Pkt4::check() {
boost::shared_ptr<Option> typeOpt = getOption(DHO_DHCP_MESSAGE_TYPE);
if (typeOpt) {
uint8_t msg_type = typeOpt->getUint8();
- if (msg_type>DHCPLEASEACTIVE) {
- isc_throw(BadValue, "Invalid DHCP message type received:" << msg_type);
+ if (msg_type > DHCPLEASEACTIVE) {
+ isc_throw(BadValue, "Invalid DHCP message type received: "
+ << msg_type);
}
msg_type_ = msg_type;
@@ -222,20 +226,20 @@ Pkt4::toText() {
void
Pkt4::setHWAddr(uint8_t hType, uint8_t hlen,
- const std::vector<uint8_t>& macAddr) {
- /// TODO Rewrite this once support for client-identifier option
+ const std::vector<uint8_t>& mac_addr) {
+ /// @todo Rewrite this once support for client-identifier option
/// is implemented (ticket 1228?)
if (hlen > MAX_CHADDR_LEN) {
isc_throw(OutOfRange, "Hardware address (len=" << hlen
<< " too long. Max " << MAX_CHADDR_LEN << " supported.");
- }
- if (macAddr.empty() && (hlen > 0) ) {
+
+ } else if (mac_addr.empty() && (hlen > 0) ) {
isc_throw(OutOfRange, "Invalid HW Address specified");
}
htype_ = hType;
hlen_ = hlen;
- std::copy(&macAddr[0], &macAddr[hlen], &chaddr_[0]);
+ std::copy(&mac_addr[0], &mac_addr[hlen], &chaddr_[0]);
std::fill(&chaddr_[hlen], &chaddr_[MAX_CHADDR_LEN], 0);
}
@@ -244,11 +248,15 @@ Pkt4::setSname(const uint8_t* sname, size_t snameLen /*= MAX_SNAME_LEN*/) {
if (snameLen > MAX_SNAME_LEN) {
isc_throw(OutOfRange, "sname field (len=" << snameLen
<< ") too long, Max " << MAX_SNAME_LEN << " supported.");
+
+ } else if (sname == NULL) {
+ isc_throw(InvalidParameter, "Invalid sname specified");
}
+
std::copy(&sname[0], &sname[snameLen], &sname_[0]);
std::fill(&sname_[snameLen], &sname_[MAX_SNAME_LEN], 0);
- // no need to store snameLen as any empty space is filled with 0s
+ // No need to store snameLen as any empty space is filled with 0s
}
void
@@ -256,11 +264,15 @@ Pkt4::setFile(const uint8_t* file, size_t fileLen /*= MAX_FILE_LEN*/) {
if (fileLen > MAX_FILE_LEN) {
isc_throw(OutOfRange, "file field (len=" << fileLen
<< ") too long, Max " << MAX_FILE_LEN << " supported.");
+
+ } else if (file == NULL) {
+ isc_throw(InvalidParameter, "Invalid file name specified");
}
+
std::copy(&file[0], &file[fileLen], &file_[0]);
std::fill(&file_[fileLen], &file_[MAX_FILE_LEN], 0);
- // no need to store fileLen as any empty space is filled with 0s
+ // No need to store fileLen as any empty space is filled with 0s
}
uint8_t
@@ -273,6 +285,7 @@ Pkt4::DHCPTypeToBootpType(uint8_t dhcpType) {
case DHCPINFORM:
case DHCPLEASEQUERY:
return (BOOTREQUEST);
+
case DHCPACK:
case DHCPNAK:
case DHCPOFFER:
@@ -280,6 +293,7 @@ Pkt4::DHCPTypeToBootpType(uint8_t dhcpType) {
case DHCPLEASEUNKNOWN:
case DHCPLEASEACTIVE:
return (BOOTREPLY);
+
default:
isc_throw(OutOfRange, "Invalid message type: "
<< static_cast<int>(dhcpType) );
@@ -288,7 +302,7 @@ Pkt4::DHCPTypeToBootpType(uint8_t dhcpType) {
void
Pkt4::addOption(boost::shared_ptr<Option> opt) {
- // check for uniqueness (DHCPv4 options must be unique)
+ // Check for uniqueness (DHCPv4 options must be unique)
if (getOption(opt->getType())) {
isc_throw(BadValue, "Option " << opt->getType()
<< " already present in this message.");
@@ -299,7 +313,7 @@ Pkt4::addOption(boost::shared_ptr<Option> opt) {
boost::shared_ptr<isc::dhcp::Option>
Pkt4::getOption(uint8_t type) {
Option::OptionCollection::const_iterator x = options_.find(type);
- if (x!=options_.end()) {
+ if (x != options_.end()) {
return (*x).second;
}
return boost::shared_ptr<isc::dhcp::Option>(); // NULL
diff --git a/src/lib/dhcp/pkt4.h b/src/lib/dhcp/pkt4.h
index b35830f..efeaf62 100644
--- a/src/lib/dhcp/pkt4.h
+++ b/src/lib/dhcp/pkt4.h
@@ -89,7 +89,7 @@ public:
/// reasonable value. This method is expected to grow significantly.
/// It makes sense to separate unpack() and check() for testing purposes.
///
- /// TODO: It is called from unpack() directly. It should be separated.
+ /// @todo It is called from unpack() directly. It should be separated.
///
/// Method will throw exception if anomaly is found.
void check();
@@ -262,16 +262,16 @@ public:
/// @brief Sets hardware address.
///
/// Sets parameters of hardware address. hlen specifies
- /// length of macAddr buffer. Content of macAddr buffer
+ /// length of mac_addr buffer. Content of mac_addr buffer
/// will be copied to appropriate field.
///
- /// Note: macAddr must be a buffer of at least hlen bytes.
+ /// Note: mac_addr must be a buffer of at least hlen bytes.
///
/// @param hType hardware type (will be sent in htype field)
/// @param hlen hardware length (will be sent in hlen field)
/// @param macAddr pointer to hardware address
void setHWAddr(uint8_t hType, uint8_t hlen,
- const std::vector<uint8_t>& macAddr);
+ const std::vector<uint8_t>& mac_addr);
/// Returns htype field
///
@@ -519,7 +519,7 @@ protected:
std::vector<uint8_t> data_;
/// message type (e.g. 1=DHCPDISCOVER)
- /// TODO: this will eventually be replaced with DHCP Message Type
+ /// @todo this will eventually be replaced with DHCP Message Type
/// option (option 53)
uint8_t msg_type_;
diff --git a/src/lib/dhcp/tests/pkt4_unittest.cc b/src/lib/dhcp/tests/pkt4_unittest.cc
index 37af9c6..2ef982a 100644
--- a/src/lib/dhcp/tests/pkt4_unittest.cc
+++ b/src/lib/dhcp/tests/pkt4_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2011 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2012 Internet Systems Consortium, Inc. ("ISC")
//
// Permission to use, copy, modify, and/or distribute this software for any
// purpose with or without fee is hereby granted, provided that the above
@@ -416,6 +416,11 @@ TEST(Pkt4Test, sname) {
delete pkt;
}
+
+ // Check that a null argument generates an exception.
+ Pkt4 pkt4(DHCPOFFER, 1234);
+ EXPECT_THROW(pkt4.setSname(NULL, Pkt4::MAX_SNAME_LEN), InvalidParameter);
+ EXPECT_THROW(pkt4.setSname(NULL, 0), InvalidParameter);
}
TEST(Pkt4Test, file) {
@@ -451,6 +456,10 @@ TEST(Pkt4Test, file) {
delete pkt;
}
+ // Check that a null argument generates an exception.
+ Pkt4 pkt4(DHCPOFFER, 1234);
+ EXPECT_THROW(pkt4.setFile(NULL, Pkt4::MAX_FILE_LEN), InvalidParameter);
+ EXPECT_THROW(pkt4.setFile(NULL, 0), InvalidParameter);
}
static uint8_t v4Opts[] = {
More information about the bind10-changes
mailing list