Commit c75a7c10 authored by Thomas Markwalder's avatar Thomas Markwalder

[#900,!561] Addressed review comments

src/lib/dhcp/libdhcp++.cc
    Cleaned up necessary exception decls

src/lib/dhcp/option.h
    Added commentary for SkipThisOptionError

src/lib/dhcp/option_definition.cc
    Cleaned up unnecessary exception decls

src/lib/dhcp/option_string.cc
    Replaced NULL with nul

src/lib/testutils/gtest_utils.h
    Added emissions of exception type name
parent a917e4ae
......@@ -456,7 +456,7 @@ size_t LibDHCP::unpackOptions6(const OptionBuffer& buf,
opt = def->optionFactory(Option::V6, opt_type,
buf.begin() + offset,
buf.begin() + offset + opt_len);
} catch (const SkipThisOptionError& ex) {
} catch (const SkipThisOptionError&) {
opt.reset();
}
}
......@@ -599,7 +599,7 @@ size_t LibDHCP::unpackOptions4(const OptionBuffer& buf,
opt = def->optionFactory(Option::V4, opt_type,
buf.begin() + offset,
buf.begin() + offset + opt_len);
} catch (const SkipThisOptionError& ex) {
} catch (const SkipThisOptionError&) {
opt.reset();
}
}
......
......@@ -55,6 +55,15 @@ public:
isc::Exception(file, line, what) { };
};
/// @brief Exception thrown during option unpacking
/// This exception is thrown when an error has occurred unpacking
/// an option from a packet and rather than drop the whole packet, we
/// wish to simply skip over the option (i.e. omit it from the unpacked
/// results), and resume unpacking with the next option in the buffer.
/// The intent is to allow us to be liberal with what we receive, and
/// skip nonsensical options rather than drop the whole packet. This
/// exception is thrown, for instance, when string options are found to
/// be empty or to contain only nuls.
class SkipThisOptionError : public Exception {
public:
SkipThisOptionError (const char* file, size_t line, const char* what) :
......
......@@ -264,12 +264,12 @@ OptionDefinition::optionFactory(Option::Universe u, uint16_t type,
;
}
return (OptionPtr(new OptionCustom(*this, u, begin, end)));
} catch (const SkipThisOptionError& ex) {
} catch (const SkipThisOptionError&) {
// We need to throw this one as is.
throw ex;
} catch (const SkipRemainingOptionsError& ex) {
throw;
} catch (const SkipRemainingOptionsError&) {
// We need to throw this one as is.
throw ex;
throw;
} catch (const Exception& ex) {
isc_throw(InvalidOptionValue, ex.what());
}
......
......@@ -51,13 +51,13 @@ OptionString::setValue(const std::string& value) {
<< getType() << "' must not be empty");
}
// Trim off any trailing NULLs.
// Trim off any trailing nuls.
auto begin = value.begin();
auto end = util::str::seekTrimmed(begin, value.end(), 0x0);
if (std::distance(begin, end) == 0) {
isc_throw(isc::OutOfRange, "string value carried by the option '"
<< getType() << "' contained only NULLs");
<< getType() << "' contained only nuls");
}
// Now set the value.
......@@ -85,12 +85,12 @@ OptionString::pack(isc::util::OutputBuffer& buf) const {
void
OptionString::unpack(OptionBufferConstIter begin,
OptionBufferConstIter end) {
// Trim off trailing null(s)
// Trim off trailing nul(s)
end = util::str::seekTrimmed(begin, end, 0x0);
if (std::distance(begin, end) == 0) {
isc_throw(isc::dhcp::SkipThisOptionError, "failed to parse an option '"
<< getType() << "' holding string value"
<< "' was empty or contained only NULLs");
<< "' was empty or contained only nuls");
}
// Now set the data.
......
......@@ -54,33 +54,35 @@ namespace test {
} \
} \
/// @brief Adds a non-fatal failure with exception info, if the given
/// expression throws
///
/// @brief Adds a non-fatal failure with exception info, if the given
/// expression throws. Note the type name emitted may be mangled.
///
/// @param statement - statement block to execute
#define EXPECT_NO_THROW_LOG(statement) \
{ \
try { \
statement; \
} catch (const std::exception& ex) { \
ADD_FAILURE() << #statement << "threw: " << ex.what(); \
ADD_FAILURE() << #statement << " threw type: " << typeid(ex).name() \
<< ", what: " << ex.what(); \
} catch (...) { \
ADD_FAILURE() << #statement << "threw non-std::exception"; \
} \
} \
/// @brief Generates a fatal failure with exception info, if the given
/// expression throws
///
/// @brief Generates a fatal failure with exception info, if the given
/// expression throws. Note the type name emitted may be mangled.
///
/// @param statement - statement block to execute
#define ASSERT_NO_THROW_LOG(statement) \
{ \
try { \
statement; \
} catch (const std::exception& ex) { \
GTEST_FAIL() << #statement << "threw: " << ex.what(); \
GTEST_FAIL() << #statement << " threw type: " << typeid(ex).name() \
<< ", what: " << ex.what(); \
} catch (...) { \
GTEST_FAIL() << #statement << "threw non-std::exception"; \
GTEST_FAIL() << #statement << " threw non-std::exception"; \
} \
} \
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment