BIND 10 trac1954, updated. 7e245132c9d7e2e49a23ebf738ca5d9d229dda30 [1954] Code cleanup

BIND 10 source code commits bind10-changes at lists.isc.org
Mon May 21 10:30:41 UTC 2012


The branch, trac1954 has been updated
       via  7e245132c9d7e2e49a23ebf738ca5d9d229dda30 (commit)
      from  ff8a54c66f7c0fadcceb18aa6b6d33c0cfbd615b (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 7e245132c9d7e2e49a23ebf738ca5d9d229dda30
Author: Marcin Siodelski <marcin at isc.org>
Date:   Mon May 21 12:30:20 2012 +0200

    [1954] Code cleanup

-----------------------------------------------------------------------

Summary of changes:
 tests/tools/perfdhcp/command_options.cc |   66 +++++++++++++++++++++---------
 tests/tools/perfdhcp/command_options.h  |   25 +++++++-----
 2 files changed, 61 insertions(+), 30 deletions(-)

-----------------------------------------------------------------------
diff --git a/tests/tools/perfdhcp/command_options.cc b/tests/tools/perfdhcp/command_options.cc
index de57be5..09bc61e 100644
--- a/tests/tools/perfdhcp/command_options.cc
+++ b/tests/tools/perfdhcp/command_options.cc
@@ -17,7 +17,6 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdint.h>
-#include <string.h>
 #include <unistd.h>
 
 #include <boost/algorithm/string.hpp>
@@ -35,7 +34,7 @@ using namespace isc;
 namespace isc {
 namespace perfdhcp {
 
-/// IfaceMgr is a singleton implementation
+/// CommandOptions is a singleton implementation
 CommandOptions* CommandOptions::instance_ = 0;
 
 void
@@ -57,12 +56,13 @@ CommandOptions::instance() {
     return (*instance_);
 }
 
-// Reset stored values to the defaults.
 void
 CommandOptions::reset() {
     uint8_t mac[6] = { 0x0, 0xC, 0x1, 0x2, 0x3, 0x4 };
     double lt[2] = { 1., 1. };
 
+    // We don't use constructor initialization list because we
+    // will need to reset all members many times to perform unit tests
     ipversion_ = 0;
     exchange_mode_ = DORR_SARR;
     rate_ = 0;
@@ -105,7 +105,7 @@ CommandOptions::parse(int argc, char** const argv, bool force_reset /*=false */)
         reset();
     }
     // Reset internal variables used by getopt
-    // to eliminate underfined behavior when
+    // to eliminate undefined behavior when
     // parsing different command lines multiple times
     optind = 1;
     opterr = 0;
@@ -116,21 +116,25 @@ CommandOptions::parse(int argc, char** const argv, bool force_reset /*=false */)
 
 void
 CommandOptions::initialize(int argc, char** const argv) {
-    char opt;
-    char* pc;
-    int nr, of;
+    char opt = 0;
+    char* pc = NULL;
+    int nr = 0, of = 0;
     int di = 0;
     float dp = 0;
-    long long r;
+    long long r = 0;
 
+    // in this section we collect argument values from command line
+    // they will be tuned and validated elsewhere
     while((opt = getopt(argc, argv, "hv46r:t:R:b:n:p:d:D:l:P:a:L:s:iBc1T:X:O:E:S:I:x:w:")) != -1) {
     switch (opt) {
     case 'h':
         usage();
+        break;
 
     case 'v':
-        //        version();
-        ;
+        version();
+        break;
+
     case '4':
         check(ipversion_ == 6, "IP version already set to 6");
         ipversion_ = 4;
@@ -155,15 +159,15 @@ CommandOptions::initialize(int argc, char** const argv) {
         r = boost::lexical_cast<long long>(optarg);
         check(r < 0, "random_range_ must not be a negative integer");
         random_range_ = static_cast<uint32_t>(r);
-        if ((random_range_ != 0) && (random_range_ != UINT32_MAX)) {
+        if ((random_range_ != 0) && (random_range_ != std::numeric_limits<uint32_t>::max())) {
             uint32_t s = random_range_ + 1;
-            uint64_t b = UINT32_MAX + 1, m;
+            uint64_t b = std::numeric_limits<uint32_t>::max() + 1, m;
 
             m = (b / s) * s;
             if (m == b)
                 max_random_ = 0;
             else
-                max_random_ = (uint32_t) m;
+                max_random_ = static_cast<uint32_t> (m);
         }
         break;
 
@@ -226,7 +230,9 @@ CommandOptions::initialize(int argc, char** const argv) {
     case 'L':
         local_port_ = boost::lexical_cast<int>(optarg);
         check(local_port_ < 0, "local-port must not be a negative integer");
-        check(local_port_ > (int) UINT16_MAX, "local-port must be lower than UINT16_MAX");
+        check(local_port_ > static_cast<int>( std::numeric_limits<uint16_t>::max()),
+              "local-port must be lower than " +
+              boost::lexical_cast<std::string>(std::numeric_limits<uint16_t>::max()));
         break;
 
     case 's':
@@ -310,8 +316,12 @@ CommandOptions::initialize(int argc, char** const argv) {
     }
     }
 
+    // Tune ip version as it still might be zero
+    // if not specified in command line was used
     if (ipversion_ == 0)
         ipversion_ = 4;
+
+    // Tune up template file lists
     if (template_file_.size() > 1) {
         if (xid_offset_.size() == 1)
             xid_offset_.push_back(xid_offset_[0]);
@@ -319,11 +329,11 @@ CommandOptions::initialize(int argc, char** const argv) {
             rnd_offset_.push_back(rnd_offset_[0]);
     }
 
-    /* get server argument */
+    // Get server argument
     check(optind < argc -1, "extra arguments?");
     if (optind == argc - 1) {
         server_name_ = argv[optind];
-        /* decode special cases */
+        // Decode special cases
         if ((ipversion_ == 4) &&
             (server_name_.compare("all") == 0)) {
             broadcast_ = 1;
@@ -337,13 +347,15 @@ CommandOptions::initialize(int argc, char** const argv) {
         }
     }
 
-    // TODO USE NON EXISTING IFACE MANAGER TO HANDLE -l option
+    // TODO handle -l option with IfaceManager when it is created
 }
 
 void
 CommandOptions::decodeBase(const std::string& base) {
     std::string b(base);
     boost::algorithm::to_lower(b);
+
+    // Currently we only support MAC and DUID
     if ((b.substr(0, 4) == "mac=") || (b.substr(0, 6) == "ether=")) {
         decodeMac(b);
     } else if (b.substr(0, 5) == "duid=") {
@@ -355,8 +367,11 @@ void
 CommandOptions::decodeMac(const std::string& base) {
     typedef boost::tokenizer<boost::char_separator<char> > tokenizer;
 
+    // Strip string from mac=
     size_t found = base.find('=');
     check(found == std::string::npos, "expected -b<base> format for MAC address is -b MAC=00::0C::01::02::03::04");
+
+    // Tokenize remaining part of the argument to get pieces of MAC address
     boost::char_separator<char> sep(":-");
     tokenizer tokens(base.substr(found + 1), sep);
     std::vector<std::string> stokens(tokens.begin(), tokens.end());
@@ -374,13 +389,17 @@ CommandOptions::decodeMac(const std::string& base) {
 
 void
 CommandOptions::decodeDuid(const std::string& base) {
+    // Strip argument from duid=
     size_t found = base.find('=');
     check(found == std::string::npos, "expected -b<base> format for DUID is -b DUID=<duid>");
     std::string b = base.substr(found + 1);
+
+    // DUID must have even number of digits and must not be longer than 64 bytes
     check(b.length() & 1, "odd number of hexadecimal digits in duid");
     check(b.length() > 128, "duid too large");
     check(b.length() == 0, "no duid specified");
 
+    // Turn pairs of hex digits into vector of bytes
     for (int i = 0; i < b.length(); i += 2) {
         unsigned int ui = 0;
         std::istringstream ss(b.substr(i, 2));
@@ -444,15 +463,16 @@ CommandOptions::validate() const {
 }
 
 void
-CommandOptions::check(bool condition, const std::string errmsg) const {
+CommandOptions::check(bool condition, const std::string& errmsg) const {
+    // The same could have been done with macro or just if statement but
+    // we prefer functions to macros here
     if (condition) {
         isc_throw(isc::InvalidParameter, errmsg);
     }
 }
 
 void
-CommandOptions::usage(void)
-{
+CommandOptions::usage() const {
 	fprintf(stdout, "%s",
 "perfdhcp [-hv] [-4|-6] [-r<rate>] [-t<report>] [-R<range>] [-b<base>]\n"
 "    [-n<num-request>] [-p<test-period>] [-d<drop-time>] [-D<max-drop>]\n"
@@ -576,5 +596,11 @@ CommandOptions::usage(void)
 "  exchanges are not successfully completed.\n");
 }
 
+void
+CommandOptions::version() const {
+	fprintf(stdout, "version 0.01\n");
+}
+
+
 } // namespace perfdhcp
 } // namespace isc
diff --git a/tests/tools/perfdhcp/command_options.h b/tests/tools/perfdhcp/command_options.h
index c44588a..e0e3ea9 100644
--- a/tests/tools/perfdhcp/command_options.h
+++ b/tests/tools/perfdhcp/command_options.h
@@ -53,7 +53,7 @@ public:
     /// \param argc Argument count passed to main().
     /// \param argv Argument value array passed to main().
     /// \param force_reset Force  reset of state variables
-    /// \throw BadValue if fails to parse
+    /// \throws isc::InvalidParameter if parsing fails
     void parse(int argc, char** const argv, bool force_reset = false);
 
     /// \brief Returns IP version
@@ -219,13 +219,18 @@ public:
     /// \brief Print usage
     ///
     /// Prints perfdhcp usage
-    void usage(void);
+    void usage() const;
+
+    /// \brief Print program version
+    ///
+    /// Prints perfdhcp version
+    void version() const;
 
 protected:
 
     /// \brief Default Constructor
     ///
-    /// Protected constructor as this is a singleton class. 
+    /// Protected constructor as this is a singleton class.
     /// Use CommandOptions::instance() to get instance of it.
     CommandOptions() {
         reset();
@@ -241,37 +246,37 @@ private:
     ///
     /// \param argc Argument count passed to main().
     /// \param argv Argument value array passed to main().
-    /// \throw InvalidParameter if bad command line option values
+    /// \throws isc::InvalidParameter if command line options initialization fails
     void initialize(int argc, char** const argv);
 
     /// \brief Validates initialized options
     ///
-    /// \throw InvalidPrameter if validation fails
+    /// \throws isc::InvalidPrameter if command line validation fails
     void validate() const;
 
     /// \brief Checks given condition
     ///
     /// \param condition Condition to be checked
     /// \param errmsg Error message in exception
-    /// \throw InvalidParameter if check fails
-    inline void check(bool condition, const std::string errmsg) const;
+    /// \throws isc::InvalidParameter if check fails
+    inline void check(bool condition, const std::string& errmsg) const;
 
     /// \brief Decodes base provided with -b
     ///
     /// \param base Base in string format
-    /// \throw InvalidParameter if base is invalid
+    /// \throws isc::InvalidParameter if base is invalid
     void decodeBase(const std::string& base);
 
     /// \brief Decodes base MAC address provided with -b
     ///
     /// \param base MAC address in string format
-    /// \throw InvalidParameter if base is invalid
+    /// \throws isc::InvalidParameter if base is invalid
     void decodeMac(const std::string& base);
 
     /// \brief Decodes base DUID provided with -b
     ///
     /// \param base DUID in string format
-    /// \throw InvalidParameter if base is invalid
+    /// \throws isc::InvalidParameter if base is invalid
     void decodeDuid(const std::string& base);
 
     static CommandOptions * instance_;       ///< A pointer to sole instance of this class



More information about the bind10-changes mailing list