[#3025] address review comments

doc/sphinx/arm/ddns.rst

@@ -952,7 +952,7 @@ Currently Kea's statistics management has the following limitations:
 
 .. note::
 
-    Hook libraries, such as the the ISC subscriber-only GSS-TSIG library,
+    Hook libraries, such as the ISC subscriber-only GSS-TSIG library,
     make new statistics available in Kea.
 
 More information about Kea statistics can be found at :ref:`stats`.

src/bin/admin/kea-admin.in

@@ -42,13 +42,13 @@ dump_file=""
 dump_qry=""
 
 # Detect if this is installed or in sources. Check for sources first, so that
-# the the unexpected situations with weird paths fall on the default case of
+# the unexpected situations with weird paths fall on the default case of
 # installed.
 script_path=$(cd "$(dirname "${0}")" && pwd)
 if test "${script_path}" = "@abs_top_builddir@/src/bin/admin"; then
     admin_utils="@abs_top_builddir@/src/bin/admin/admin-utils.sh"
     KEA_LFC="@abs_top_builddir@/src/bin/lfc/kea-lfc"
-    SCRIPTS_DIR="@abs_top_builddir@/src/share/database/scripts"
+    SCRIPTS_DIR="@abs_top_srcdir@/src/share/database/scripts"
 else
     admin_utils="@datarootdir@/@PACKAGE_NAME@/scripts/admin-utils.sh"
     KEA_LFC="@sbindir@/kea-lfc"

src/lib/asiolink/tests/process_spawn_unittest.cc

@@ -302,11 +302,6 @@ TEST_F(ProcessSpawnTest, invalidExecutable) {
 // This test verifies that the full command line for the process is
 // returned.
 TEST_F(ProcessSpawnTest, getCommandLine) {
-    // Note that cases below are enclosed in separate scopes to make
-    // sure that the ProcessSpawn object is destroyed before a new
-    // object is created. Current implementation doesn't allow for
-    // having two ProcessSpawn objects simultaneously as they will
-    // both try to allocate a signal handler for SIGCHLD.
     {
         // Case 1: arguments present.
         ProcessArgs args;

src/lib/database/testutils/Makefile.am

@@ -1,7 +1,7 @@
 SUBDIRS = .
 
 AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib
-TEST_CA_DIR = $(abs_srcdir)/../../asiolink/testutils/ca
+TEST_CA_DIR = $(realpath $(abs_srcdir)/../../asiolink/testutils/ca)
 AM_CPPFLAGS += -DTEST_CA_DIR=\"$(TEST_CA_DIR)\"
 
 AM_CXXFLAGS = $(KEA_CXXFLAGS)

src/lib/dhcpsrv/dhcpsrv_messages.mes

@@ -391,7 +391,7 @@ no lease6 should be assigned. The server will not put that lease in its
 database and the client will get a NoAddrsAvail for that IA_NA option.
 
 % DHCPSRV_HOST_MGR_DB_OPEN_CONNECTION_WITH_RETRY_FAILED Failed to connect to database: %1 with error: %2
-This is an informational message issued when the the server failed to connect to
+This is an informational message issued when the server failed to connect to
 the host database. The operation started a retry to connect procedure.
 The database access string with password redacted is logged, along with the
 error and details for the reconnect procedure.
@@ -438,7 +438,7 @@ This log message variant contains no error text because it is triggered
 by an unknown exception.
 
 % DHCPSRV_LEASE_MGR_DB_OPEN_CONNECTION_WITH_RETRY_FAILED Failed to connect to database: %1 with error: %2
-This is an informational message issued when the the server failed to connect to
+This is an informational message issued when the server failed to connect to
 the lease database. The operation started a retry to connect procedure.
 The database access string with password redacted is logged, along with the
 error and details for the reconnect procedure.

src/lib/hooks/hooks_user.dox

@@ -875,7 +875,7 @@ configuration derived from the root logger.
 @subsection hooksdgBuild Building the Library
 
 Building the code requires building a sharable library.  This requires
-the the code be compiled as position-independent code (using the
+the code be compiled as position-independent code (using the
 compiler's "-fpic" switch) and linked as a shared library (with the
 linker's "-shared" switch).  The build command also needs to point to
 the Kea include directory and link in the appropriate libraries.

src/lib/mysql/mysql_connection.h

@@ -827,6 +827,8 @@ public:
     /// @brief TLS flag (true when TLS was required, false otherwise).
     bool tls_;
 
+    /// @brief Holds location to kea-admin. By default, it points to kea-admin
+    /// from installation. In tests, it points to kea-admin from sources.
     static std::string KEA_ADMIN_;
 };
 

src/lib/mysql/tests/Makefile.am

@@ -1,7 +1,7 @@
 SUBDIRS = .
 
 AM_CPPFLAGS  =
-AM_CPPFLAGS += -DABS_TOP_BUILDDIR="\"$(abs_top_builddir)\""
+AM_CPPFLAGS += -DABS_SRCDIR="\"$(abs_top_builddir)\""
 AM_CPPFLAGS += -DKEA_ADMIN=\"${abs_top_builddir}/src/bin/admin/kea-admin\"
 AM_CPPFLAGS += -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib
 AM_CPPFLAGS += $(BOOST_INCLUDES) $(MYSQL_CPPFLAGS)

src/lib/mysql/tests/mysql_connection_unittest.cc

@@ -1000,18 +1000,16 @@ TEST_F(MySqlConnectionTest, toKeaAdminParameters) {
     parameters = DatabaseConnection::parse(full_mysql_connection_string);
     kea_admin_parameters = MySqlConnection::toKeaAdminParameters(parameters);
     EXPECT_EQ(kea_admin_parameters,
-              vector<string>(
+              vector<string>({"mysql", "--extra",
-                  {"mysql", "--extra",
+                              "--ssl-cert " ABS_SRCDIR "/src/lib/asiolink/testutils/ca/"
-                   "--ssl-cert " ABS_TOP_BUILDDIR "/src/lib/database/testutils/../../asiolink/"
+                              "kea-client.crt",
-                   "testutils/ca/kea-client.crt",
+                              "--extra", "--ssl-cipher AES", "--extra", "--connect_timeout 10",
-                   "--extra", "--ssl-cipher AES", "--extra", "--connect_timeout 10", "--host",
+                              "--host", "127.0.0.1", "--extra",
-                   "127.0.0.1", "--extra",
+                              "--ssl-key " ABS_SRCDIR "/src/lib/asiolink/testutils/ca/"
-                   "--ssl-key " ABS_TOP_BUILDDIR "/src/lib/database/testutils/../../asiolink/"
+                                                      "kea-client.key",
-                   "testutils/ca/kea-client.key",
+                              "--name", "keatest", "--password", "keatest", "--extra",
-                   "--name", "keatest", "--password", "keatest", "--extra",
+                              "--ssl-ca " ABS_SRCDIR "/src/lib/asiolink/testutils/ca/kea-ca.crt",
-                   "--ssl-ca " ABS_TOP_BUILDDIR "/src/lib/database/testutils/../../asiolink/"
+                              "--user", "keatest_secure"}));
-                   "testutils/ca/kea-ca.crt",
-                   "--user", "keatest_secure"}));
 }
 
 }  // namespace

src/lib/pgsql/pgsql_connection.cc

@@ -184,9 +184,6 @@ PgSqlConnection::ensureSchemaVersion(const ParameterMap& parameters,
     IOServiceAccessorPtr ac(new IOServiceAccessor(&DatabaseConnection::getIOService));
     pair<uint32_t, uint32_t> schema_version;
     try {
-        // Attempt to get version without retrying or other sophistries. This
-        // provides the most accurate view of the status of the database and the
-        // most flexibility to reacting to errors.
         schema_version = getVersion(parameters, ac, cb, retry ? timer_name : string());
     } catch (DbOpenError const& exception) {
         throw;

src/lib/pgsql/pgsql_connection.h

@@ -626,6 +626,8 @@ public:
     /// is already in progress.
     int transaction_ref_count_;
 
+    /// @brief Holds location to kea-admin. By default, it points to kea-admin
+    /// from installation. In tests, it points to kea-admin from sources.
     static std::string KEA_ADMIN_;
 };
 

src/lib/pgsql/tests/pgsql_connection_unittest.cc

@@ -197,7 +197,7 @@ public:
         TestRowSet fetched_rows;
 
         // Run the select.  The row consumption lambda should populate
-        // fetched_rows based on the the result set returned by the select.
+        // fetched_rows based on the result set returned by the select.
         conn_->selectQuery(tagged_statements[GET_BY_INT_RANGE], in_bindings,
                            [&](PgSqlResult& r, size_t row) {
             TestRow fetched_row;
@@ -236,7 +236,7 @@ public:
     /// In this test, the input data is a set of rows that describe
     /// which rows in the database to update and how. For each row
     /// in the set we find the record in the database with matching
-    /// int_col value and replace its text_col value with the the
+    /// int_col value and replace its text_col value with the
     /// text value from the input the row.
     ///
     /// @param update_rows Collection of rows of data to update.

src/lib/process/process_messages.mes

@@ -76,7 +76,7 @@ A debug message indicating that the controller has received an
 updated configuration from the Kea configuration system.
 
 % DCTL_DB_OPEN_CONNECTION_WITH_RETRY_FAILED Failed to connect to database: %1 with error: %2
-This is an informational message issued when the the server failed to connect to
+This is an informational message issued when the server failed to connect to
 the configuration database. The operation started a retry to connect procedure.
 The database access string with password redacted is logged, along with the
 error and details for the reconnect procedure.

src/lib/util/file_utilities.cc

@@ -73,7 +73,6 @@ isFile(const string& name) {
     return ((stats.st_mode & S_IFMT) == S_IFREG);
 }
 
-
 } // namespace file
 } // namespace log
 } // namespace isc

src/lib/util/tests/Makefile.am

@@ -2,6 +2,7 @@ SUBDIRS = .
 
 AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib
 AM_CPPFLAGS += $(BOOST_INCLUDES)
+AM_CPPFLAGS += -DABS_SRCDIR=\"$(abs_srcdir)\"
 AM_CPPFLAGS += -DTEST_DATA_BUILDDIR=\"$(abs_builddir)\"
 AM_CXXFLAGS = $(KEA_CXXFLAGS)
 

src/lib/util/tests/file_utilities_unittest.cc

@@ -28,7 +28,7 @@ public:
 };
 
 FileUtilTest::~FileUtilTest() {
-    string test_file_name = string(TEST_DATA_BUILDDIR) + "/fu.test";
+    string test_file_name(TEST_DATA_BUILDDIR "/fu.test");
     static_cast<void>(remove(test_file_name.c_str()));
 }
 
@@ -65,7 +65,7 @@ TEST_F(FileUtilTest, notRegular) {
 
 /// @brief Check getContent works.
 TEST_F(FileUtilTest, basic) {
-    string file_name = string(TEST_DATA_BUILDDIR) + "/fu.test";
+    string file_name(TEST_DATA_BUILDDIR "/fu.test");
     ofstream fs(file_name.c_str(), ofstream::out | ofstream::trunc);
     ASSERT_TRUE(fs.is_open());
     fs << "abdc";
@@ -79,13 +79,13 @@ TEST_F(FileUtilTest, basic) {
 TEST_F(FileUtilTest, isDir) {
     EXPECT_TRUE(isDir("/dev"));
     EXPECT_FALSE(isDir("/dev/null"));
-    EXPECT_FALSE(isDir("/this/does/not/exists"));
+    EXPECT_FALSE(isDir("/this/does/not/exist"));
     EXPECT_FALSE(isDir("/etc/hosts"));
 }
 
 /// @brief Check isFile.
 TEST_F(FileUtilTest, isFile) {
-    EXPECT_TRUE(isFile("file_utilities_unittest.cc"));
+    EXPECT_TRUE(isFile(ABS_SRCDIR "/file_utilities_unittest.cc"));
     EXPECT_FALSE(isFile(TEST_DATA_BUILDDIR));
 }
 

src/lib/yang/translator.h

@@ -205,7 +205,7 @@ public:
     /// in the sysrepo datastore by calling Session::getData(). It should be
     /// used sparingly in production code. It is primarily meant for unit tests.
     ///
-    /// @param xpath the xpath of the root node belonging to the the tree being traversed
+    /// @param xpath the xpath of the root node belonging to the tree being traversed
     /// @param f the function to be called on the node itself and each descendant
     template <typename functor_t>
     void forAll(std::string const& xpath, functor_t f) const {

tools/Makefile.am

@@ -8,7 +8,3 @@ endif
 
 CLEANFILES = *.gcno *.gcda
 DISTCLEANFILES = path_replacer.sh
-
-if GENERATE_DOCS
-
-endif