From 17e4e76aa289ec483b014d89fccee36819b69783 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 5 Feb 2015 17:01:32 +0100 Subject: [PATCH] [3669] Added some more commentary in various places. --- src/lib/dhcpsrv/memfile_lease_mgr.cc | 10 ++++---- src/lib/dhcpsrv/memfile_lease_mgr.h | 12 +++++----- src/lib/dhcpsrv/tests/lease_file_io.h | 2 +- src/lib/util/process_spawn.cc | 8 +++---- src/lib/util/tests/process_spawn_app.sh | 24 ++++++++++++++++++++ src/lib/util/tests/process_spawn_unittest.cc | 19 ++++++++++++++++ 6 files changed, 59 insertions(+), 16 deletions(-) diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc index 8b138ee28b..5a59462629 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.cc +++ b/src/lib/dhcpsrv/memfile_lease_mgr.cc @@ -809,9 +809,9 @@ void Memfile_LeaseMgr::lfcExecute(boost::shared_ptr& lease_file) // This string will hold a reason for the failure to rote the lease files. std::string error_string = "(no details)"; // Check if the copy of the lease file exists already. If it does, it - // is an indication that another LFC instance may be in progress, in - // which case we don't want to rotate the current lease file to avoid - // overriding the contents of the existing file. + // is an indication that another LFC instance may be in progress or + // may be stalled. In that case we don't want to rotate the current + // lease file to avoid overriding the contents of the existing file. CSVFile lease_file_copy(appendSuffix(lease_file->getFilename(), FILE_INPUT)); if (!lease_file_copy.exists()) { // Close the current file so as we can move it to the copy file. @@ -849,8 +849,8 @@ void Memfile_LeaseMgr::lfcExecute(boost::shared_ptr& lease_file) error_string = ex.what(); } } - // Once we have rotated files as needed, start the new kea-lfc process - // to perform a cleanup. + // Once the files have been rotated, or untouched if another LFC had + // not finished, a new process is started. if (do_lfc) { lfc_setup_->execute(); diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.h b/src/lib/dhcpsrv/memfile_lease_mgr.h index 41e8e3b2ac..75a9089fc5 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.h +++ b/src/lib/dhcpsrv/memfile_lease_mgr.h @@ -339,12 +339,12 @@ public: /// This enumeration is used by a method which appends the appropriate /// suffix to the lease file name. enum LFCFileType { - FILE_CURRENT, - FILE_INPUT, - FILE_PREVIOUS, - FILE_OUTPUT, - FILE_FINISH, - FILE_PID + FILE_CURRENT, ///< %Lease File + FILE_INPUT, ///< %Lease File Copy + FILE_PREVIOUS, ///< Previous %Lease File + FILE_OUTPUT, ///< LFC Output File + FILE_FINISH, ///< LFC Finish File + FILE_PID ///< PID File }; /// @brief Appends appropriate suffix to the file name. diff --git a/src/lib/dhcpsrv/tests/lease_file_io.h b/src/lib/dhcpsrv/tests/lease_file_io.h index 87cfd48590..e7a52765a5 100644 --- a/src/lib/dhcpsrv/tests/lease_file_io.h +++ b/src/lib/dhcpsrv/tests/lease_file_io.h @@ -1,4 +1,4 @@ -// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above diff --git a/src/lib/util/process_spawn.cc b/src/lib/util/process_spawn.cc index b09a898113..c02b924d13 100644 --- a/src/lib/util/process_spawn.cc +++ b/src/lib/util/process_spawn.cc @@ -48,11 +48,11 @@ public: /// @brief Spawn the new process. /// - /// This method forks the current process and execues the specified + /// This method forks the current process and executes the specified /// binary with arguments within the child process. /// /// The child process will return EXIT_FAILURE if the method was unable - /// to start the exuctable, e.g. as a result of insufficient permissions + /// to start the executable, e.g. as a result of insufficient permissions /// or when the executable does not exist. If the process ends successfully /// the EXIT_SUCCESS is returned. /// @@ -131,7 +131,7 @@ ProcessSpawnImpl::ProcessSpawnImpl(const std::string& executable, const ProcessArgs& args) : signals_(new SignalSet(SIGCHLD)), process_status_(), executable_(executable), args_(new char*[args.size() + 2]) { - // Set the handler which is invoked immediatelly when the signal + // Set the handler which is invoked immediately when the signal // is received. signals_->setOnReceiptHandler(boost::bind(&ProcessSpawnImpl::waitForProcess, this, _1)); @@ -199,7 +199,7 @@ bool ProcessSpawnImpl::isRunning(const pid_t pid) const { if (process_status_.find(pid) == process_status_.end()) { isc_throw(BadValue, "the process with the pid '" << pid - << "' hasn't been spawned and it status cannnot be" + << "' hasn't been spawned and it status cannot be" " returned"); } return ((pid != 0) && (kill(pid, 0) == 0)); diff --git a/src/lib/util/tests/process_spawn_app.sh b/src/lib/util/tests/process_spawn_app.sh index 464695603d..8ab94fc691 100755 --- a/src/lib/util/tests/process_spawn_app.sh +++ b/src/lib/util/tests/process_spawn_app.sh @@ -14,6 +14,24 @@ # OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR # PERFORMANCE OF THIS SOFTWARE. + +# This script is used for testing the ProcessSpawn utility class. This +# class is used to fork and execute a new process. It also allows for +# checking the exit code returned when the process terminates. +# The unit tests execute this script via ProcessSpawn class with +# different command line parameters to test the class functionality. +# +# In particular, they check if the class correctly records the exit code +# returned. The exit code returned is controlled by the caller. It is +# possible to explictily specify the exit code to be returned using +# the command line options. It is also possible to specify that the +# exit code is "unique" for the process, so as the test can check +# that two distinct processes spawned by the same ProcessSpawn +# object may return different status code. The command line of this +# script also allows for forcing the process to sleep so as the +# test has much enough time to verify that the convenience methods +# checking the state of the process, i.e. process running or not. + exit_code= while [ ! -z "${1}" ] @@ -29,6 +47,10 @@ do shift exit_code=${1} ;; + -s) + shift + sleep ${1} + ;; *) exit 123 ;; @@ -36,6 +58,8 @@ do shift done +# The exit code of 32 is returned when no args specified or +# when only the -s arg has been specified. if [ -z "${exit_code}" ]; then exit 32; fi diff --git a/src/lib/util/tests/process_spawn_unittest.cc b/src/lib/util/tests/process_spawn_unittest.cc index a23646fcfc..e29507f499 100644 --- a/src/lib/util/tests/process_spawn_unittest.cc +++ b/src/lib/util/tests/process_spawn_unittest.cc @@ -150,4 +150,23 @@ TEST(ProcessSpawn, getCommandLine) { } } +// This test verifies that it is possible to check if the process is +// running. +TEST(ProcessSpawn, isRunning) { + // Run the process which sleeps for 10 seconds, so as we have + // enough time to check if it is running. + std::vector args; + args.push_back("-s"); + args.push_back("10"); + ProcessSpawn process(getApp(), args); + pid_t pid = 0; + ASSERT_NO_THROW(pid = process.spawn()); + EXPECT_TRUE(process.isRunning(pid)); + + // Kill the process. + ASSERT_EQ(0, kill(pid, SIGKILL)); + // And make sure if died. + ASSERT_TRUE(waitForProcess(process, pid, 2)); +} + } // end of anonymous namespace -- 2.47.3