From: Marcin Siodelski Date: Fri, 23 Nov 2012 09:06:15 +0000 (+0100) Subject: [2490] Changes as a result of review. X-Git-Tag: bind10-1.0.0-beta-release~84^2^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=139f66086e6cd02bfbf3b9fd0f55761ae4df3aac;p=thirdparty%2Fkea.git [2490] Changes as a result of review. --- diff --git a/src/lib/dhcp/libdhcp++.cc b/src/lib/dhcp/libdhcp++.cc index 2edd5a543f..a997d314ad 100644 --- a/src/lib/dhcp/libdhcp++.cc +++ b/src/lib/dhcp/libdhcp++.cc @@ -122,7 +122,7 @@ size_t LibDHCP::unpackOptions6(const OptionBuffer& buf, " for the same option code. This will be supported once" " support for option spaces is implemented"); } else if (num_defs == 0) { - // Don't crash if definition does not exist because only a few + // @todo Don't crash if definition does not exist because only a few // option definitions are initialized right now. In the future // we will initialize definitions for all options and we will // remove this elseif. For now, return generic option. @@ -303,11 +303,12 @@ LibDHCP::initStdOptionDefs6() { try { definition->validate(); } catch (const Exception& ex) { - // Return empty set in an unlikely event of a validation error. - // We don't return exception here on error because it should - // not happen to the regular user. It is a programming error. + // This is unlikely event that validation fails and may + // be only caused by programming error. To guarantee the + // data consistency we clear all option definitions that + // have been added so far and pass the exception forward. v6option_defs_.clear(); - return; + throw; } v6option_defs_.push_back(definition); } diff --git a/src/lib/dhcp/libdhcp++.h b/src/lib/dhcp/libdhcp++.h index cf6490cffd..cab79289bb 100644 --- a/src/lib/dhcp/libdhcp++.h +++ b/src/lib/dhcp/libdhcp++.h @@ -137,6 +137,8 @@ private: /// The method creates option definitions for all DHCPv6 options. /// /// @throw std::bad_alloc if system went out of memory. + /// @throw MalformedOptionDefinition if any of the definitions + /// is incorect. This is a programming error. static void initStdOptionDefs6(); /// pointers to factories that produce DHCPv6 options diff --git a/src/lib/dhcp/option_data_types.h b/src/lib/dhcp/option_data_types.h index 2d5d64e6aa..99b42205b0 100644 --- a/src/lib/dhcp/option_data_types.h +++ b/src/lib/dhcp/option_data_types.h @@ -147,7 +147,14 @@ struct OptionDataTypeTraits { template<> struct OptionDataTypeTraits { static const bool valid = true; - static const int len = sizeof(asiolink::IOAddress); + // The len value is used to determine the size of the data + // to be written to an option buffer. IOAddress object may + // either represent an IPv4 or IPv6 addresses which have + // different lengths. Thus we can't put fixed value here. + // The length of a data to be written into an option buffer + // have to be determined in the runtime for a particular + // IOAddress object. Thus setting len to zero. + static const int len = 0; static const bool integer_type = false; static const OptionDataType type = OPT_ANY_ADDRESS_TYPE; }; @@ -156,7 +163,11 @@ struct OptionDataTypeTraits { template<> struct OptionDataTypeTraits { static const bool valid = true; - static const int len = sizeof(std::string); + // The len value is used to determine the size of the data + // to be written to an option buffer. For strings this + // size is unknown until we actually deal with the particular + // string to be written. Thus setting it to zero. + static const int len = 0; static const bool integer_type = false; static const OptionDataType type = OPT_STRING_TYPE; }; diff --git a/src/lib/dhcp/option_definition.cc b/src/lib/dhcp/option_definition.cc index 72f16778c0..62a81c291e 100644 --- a/src/lib/dhcp/option_definition.cc +++ b/src/lib/dhcp/option_definition.cc @@ -157,7 +157,7 @@ OptionDefinition::DataTypeUtil::writeToBuffer(const std::string& value, } case OPT_UINT8_TYPE: { - buf.push_back(lexicalCastWithRangeCheck(value)); + buf.push_back(lexicalCastWithRangeCheck(value)); return; } case OPT_UINT16_TYPE: @@ -212,7 +212,7 @@ OptionDefinition::DataTypeUtil::writeToBuffer(const std::string& value, // Increase the size of the storage by the size of the string. buf.resize(buf.size() + value.size()); // Assuming that the string is already UTF8 encoded. - std::copy_backward(value.c_str(), value.c_str() + value.length(), + std::copy_backward(value.c_str(), value.c_str() + value.size(), buf.end()); return; } @@ -288,40 +288,50 @@ OptionDefinition::optionFactory(Option::Universe u, uint16_t type, try { if (type_ == OPT_BINARY_TYPE) { return (factoryGeneric(u, type, begin, end)); + } else if (type_ == OPT_IPV6_ADDRESS_TYPE && array_type_) { return (factoryAddrList6(type, begin, end)); + } else if (type_ == OPT_IPV4_ADDRESS_TYPE && array_type_) { return (factoryAddrList4(type, begin, end)); + } else if (type_ == OPT_EMPTY_TYPE) { return (factoryEmpty(u, type)); + } else if (u == Option::V6 && code_ == D6O_IA_NA && haveIA6Format()) { return (factoryIA6(type, begin, end)); + } else if (u == Option::V6 && code_ == D6O_IAADDR && haveIAAddr6Format()) { return (factoryIAAddr6(type, begin, end)); + } else if (type_ == OPT_UINT8_TYPE) { if (array_type_) { return (factoryGeneric(u, type, begin, end)); } else { return (factoryInteger(u, type, begin, end)); } + } else if (type_ == OPT_UINT16_TYPE) { if (array_type_) { return (factoryIntegerArray(type, begin, end)); } else { return (factoryInteger(u, type, begin, end)); } + } else if (type_ == OPT_UINT32_TYPE) { if (array_type_) { return (factoryIntegerArray(type, begin, end)); } else { return (factoryInteger(u, type, begin, end)); } + } return (factoryGeneric(u, type, begin, end)); + } catch (const Exception& ex) { isc_throw(InvalidOptionValue, ex.what()); }