[219-allow-an-option-value-to-be-set-from-an-expression] Checkpoint: code and test done, todo docs

src/hooks/dhcp/flex_option/flex_option.cc

@@ -7,6 +7,7 @@
 #include <config.h>
 
 #include <flex_option.h>
+#include <flex_option_log.h>
 #include <cc/simple_parser.h>
 #include <dhcp/dhcp4.h>
 #include <dhcp/libdhcp++.h>
@@ -19,6 +20,7 @@ using namespace isc;
 using namespace isc::data;
 using namespace isc::dhcp;
 using namespace isc::eval;
+using namespace isc::log;
 using namespace std;
 
 namespace isc {
@@ -225,5 +227,46 @@ FlexOptionImpl::parseOptionConfig(ConstElementPtr option) {
     option_config_map_[code] = opt_cfg;
 }
 
+void
+FlexOptionImpl::logAction(Action action, uint16_t code,
+                          const string& value) const {
+    if (action == NONE) {
+        return;
+    }
+    if (action == REMOVE) {
+        LOG_DEBUG(flex_option_logger, DBGLVL_TRACE_BASIC,
+                  FLEX_OPTION_PROCESS_REMOVE)
+            .arg(code);
+        return;
+    }
+    bool printable = true;
+    for (const unsigned char& ch : value) {
+        if (isprint(ch) == 0) {
+            printable = false;
+            break;
+        }
+    }
+    ostringstream repr;
+    if (printable) {
+        repr << "'" << value << "'";
+    } else {
+        repr << "0x" << hex;
+        for (const unsigned char& ch : value) {
+            repr << setw(2) << setfill('0') << static_cast<unsigned>(ch);
+        }
+    }
+    if (action == SUPERSEDE) {
+        LOG_DEBUG(flex_option_logger, DBGLVL_TRACE_BASIC,
+                  FLEX_OPTION_PROCESS_SUPERSEDE)
+            .arg(code)
+            .arg(repr.str());
+    } else {
+        LOG_DEBUG(flex_option_logger, DBGLVL_TRACE_BASIC,
+                  FLEX_OPTION_PROCESS_ADD)
+            .arg(code)
+            .arg(repr.str());
+    }
+}
+
 } // end of namespace flex_option
 } // end of namespace isc

src/hooks/dhcp/flex_option/flex_option.h

@@ -163,6 +163,7 @@ public:
                 opt.reset(new isc::dhcp::Option(universe, opt_cfg->getCode(),
                                                 buffer));
                 response->addOption(opt);
+                logAction(ADD, opt_cfg->getCode(), value);
                 break;
             case SUPERSEDE:
                 value = isc::dhcp::evaluateString(*opt_cfg->getExpr(), *query);
@@ -177,6 +178,7 @@ public:
                 opt.reset(new isc::dhcp::Option(universe, opt_cfg->getCode(),
                                                 buffer));
                 response->addOption(opt);
+                logAction(SUPERSEDE, opt_cfg->getCode(), value);
                 break;
             case REMOVE:
                 if (!opt) {
@@ -189,11 +191,19 @@ public:
                     response->delOption(opt_cfg->getCode());
                     opt = response->getOption(opt_cfg->getCode());
                 }
+                logAction(REMOVE, opt_cfg->getCode(), "");
                 break;
             }
         }
     }
 
+    /// @brief Log the action.
+    ///
+    /// @param action The action.
+    /// @param code The option code.
+    /// @param value The option value ("" for remove).
+    void logAction(Action action, uint16_t code, const std::string& value) const;
+
 protected:
     /// @brief Get a mutable reference to the option config map
     ///

src/hooks/dhcp/flex_option/flex_option_callouts.cc

@@ -41,6 +41,11 @@ extern "C" {
 ///
 /// @return 0 upon success, non-zero otherwise
 int pkt4_send(CalloutHandle& handle) {
+    // Sanity.
+    if (!impl) {
+        return (0);
+    }
+
     // Get the parameters.
     Pkt4Ptr query;
     Pkt4Ptr response;
@@ -68,6 +73,11 @@ int pkt4_send(CalloutHandle& handle) {
 ///
 /// @return 0 upon success, non-zero otherwise
 int pkt6_send(CalloutHandle& handle) {
+    // Sanity.
+    if (!impl) {
+        return (0);
+    }
+
     // Get the parameters.
     Pkt6Ptr query;
     Pkt6Ptr response;

src/hooks/dhcp/flex_option/flex_option_log.h

@@ -9,6 +9,7 @@
 
 #include <log/logger_support.h>
 #include <log/macros.h>
+#include <log/log_dbglevels.h>
 #include <flex_option_messages.h>
 
 namespace isc {

src/hooks/dhcp/flex_option/flex_option_messages.cc

-// File created from ../../../../src/hooks/dhcp/flex_option/flex_option_messages.mes on Tue Oct 01 2019 14:08
+// File created from ../../../../src/hooks/dhcp/flex_option/flex_option_messages.mes on Wed Oct 02 2019 17:43
 
 #include <cstddef>
 #include <log/message_types.h>
 #include <log/message_initializer.h>
 
 extern const isc::log::MessageID FLEX_OPTION_LOAD_ERROR = "FLEX_OPTION_LOAD_ERROR";
+extern const isc::log::MessageID FLEX_OPTION_PROCESS_ADD = "FLEX_OPTION_PROCESS_ADD";
 extern const isc::log::MessageID FLEX_OPTION_PROCESS_ERROR = "FLEX_OPTION_PROCESS_ERROR";
+extern const isc::log::MessageID FLEX_OPTION_PROCESS_REMOVE = "FLEX_OPTION_PROCESS_REMOVE";
+extern const isc::log::MessageID FLEX_OPTION_PROCESS_SUPERSEDE = "FLEX_OPTION_PROCESS_SUPERSEDE";
 extern const isc::log::MessageID FLEX_OPTION_UNLOAD = "FLEX_OPTION_UNLOAD";
 
 namespace {
 
 const char* values[] = {
     "FLEX_OPTION_LOAD_ERROR", "loading Flex Option hooks library failed: %1",
+    "FLEX_OPTION_PROCESS_ADD", "Added the option code %1 value by %2",
     "FLEX_OPTION_PROCESS_ERROR", "An error occurred processing query %1: %2",
+    "FLEX_OPTION_PROCESS_REMOVE", "Removed option code %1",
+    "FLEX_OPTION_PROCESS_SUPERSEDE", "Supersedes the value of option code %1 by %2",
     "FLEX_OPTION_UNLOAD", "Flex Option hooks library has been unloaded",
     NULL
 };

src/hooks/dhcp/flex_option/flex_option_messages.h

-// File created from ../../../../src/hooks/dhcp/flex_option/flex_option_messages.mes on Tue Oct 01 2019 14:08
+// File created from ../../../../src/hooks/dhcp/flex_option/flex_option_messages.mes on Wed Oct 02 2019 17:43
 
 #ifndef FLEX_OPTION_MESSAGES_H
 #define FLEX_OPTION_MESSAGES_H
@@ -6,7 +6,10 @@
 #include <log/message_types.h>
 
 extern const isc::log::MessageID FLEX_OPTION_LOAD_ERROR;
+extern const isc::log::MessageID FLEX_OPTION_PROCESS_ADD;
 extern const isc::log::MessageID FLEX_OPTION_PROCESS_ERROR;
+extern const isc::log::MessageID FLEX_OPTION_PROCESS_REMOVE;
+extern const isc::log::MessageID FLEX_OPTION_PROCESS_SUPERSEDE;
 extern const isc::log::MessageID FLEX_OPTION_UNLOAD;
 
 #endif // FLEX_OPTION_MESSAGES_H

src/hooks/dhcp/flex_option/flex_option_messages.mes

@@ -5,12 +5,26 @@ This error message indicates an error during loading the Flex Option
 hooks library. The details of the error are provided as argument of
 the log message.
 
+% FLEX_OPTION_PROCESS_ADD Added the option code %1 value by %2
+This debug message is printed when an option was added into the response
+packet. The option code and the value (between quotes if printable, in
+hexadecimal is not) are provided.
+
 % FLEX_OPTION_PROCESS_ERROR An error occurred processing query %1: %2
 This error message indicates an error during processing of a query
 by the Flex Option hooks library. The client identification information
 from the query and the details of the error are provided as arguments
 of the log message.
 
+% FLEX_OPTION_PROCESS_REMOVE Removed option code %1
+This debug message is printed when an option was removed from the response
+packet. The option code is provided.
+
+% FLEX_OPTION_PROCESS_SUPERSEDE Supersedes the value of option code %1 by %2
+This debug message is printed when an option was superseded into the response
+packet. The option code and the value (between quotes if printable, in
+hexadecimal is not) are provided.
+
 % FLEX_OPTION_UNLOAD Flex Option hooks library has been unloaded
 This info message indicates that the Flex Option hooks library has been
 unloaded.

src/hooks/dhcp/flex_option/tests/flex_option_unittests.cc

@@ -674,7 +674,7 @@ TEST_F(FlexOptionTest, optionConfigMultipleAction) {
     errmsg.str("");
     errmsg << "multiple actions: " << option->str();
     EXPECT_EQ(errmsg.str(), impl_->getErrMsg());
-    
+
     // add and remove.
     option->remove("supersede");
     option->set("add", add);
@@ -860,7 +860,7 @@ TEST_F(FlexOptionTest, processSupersedeExisting) {
     options->add(option);
     ElementPtr code = Element::create(D6O_BOOTFILE_URL);
     option->set("code", code);
-    ElementPtr supersede = Element::create(string("'abc'"));
+    ElementPtr supersede = Element::create(string("0xabcdef"));
     option->set("supersede", supersede);
     EXPECT_NO_THROW(impl_->testConfigure(options));
     EXPECT_TRUE(impl_->getErrMsg().empty());
@@ -877,7 +877,8 @@ TEST_F(FlexOptionTest, processSupersedeExisting) {
     EXPECT_EQ(D6O_BOOTFILE_URL, opt->getType());
     const OptionBuffer& buffer = opt->getData();
     ASSERT_EQ(3, buffer.size());
-    EXPECT_EQ(0, memcmp(&buffer[0], "abc", 3));
+    uint8_t expected[] = { 0xab, 0xcd, 0xef };
+    EXPECT_EQ(0, memcmp(&buffer[0], expected, 3));
 }
 
 // Verify that SUPERSEDE action does not supersede an empty value.