From a5e69119eb95f63bc132ebf7dcfa21116ac9a881 Mon Sep 17 00:00:00 2001 From: Stephen Morris Date: Tue, 1 Apr 2014 12:15:45 +0100 Subject: [PATCH] [3360] Minor changes to comments and documentation made during review --- doc/guide/bind10-guide.xml | 42 +++++++++---------- src/lib/dhcpsrv/csv_lease_file4.h | 6 +-- src/lib/dhcpsrv/csv_lease_file6.h | 6 +-- src/lib/dhcpsrv/memfile_lease_mgr.h | 20 ++++----- .../dhcpsrv/tests/csv_lease_file6_unittest.cc | 4 +- src/lib/util/csv_file.cc | 4 +- src/lib/util/csv_file.h | 29 +++++++------ 7 files changed, 53 insertions(+), 58 deletions(-) diff --git a/doc/guide/bind10-guide.xml b/doc/guide/bind10-guide.xml index c9b3f98402..771e3bb02e 100644 --- a/doc/guide/bind10-guide.xml +++ b/doc/guide/bind10-guide.xml @@ -3713,17 +3713,17 @@ Dhcp4/subnet4 [] list (default)
Default storage for leases - Server is designed to support multiple lease database storages. In larger deployments - it is often desired to store leases in the database. The - describes one of the possible ways to do it. - By default, the server will use a flat CSV file, rather than the database to store + The server is able to store lease data in different repositories. Larger deployments + may elect to store leases in a database. + describes one way to do it. + By default, the server will use a CSV file rather than a database to store lease information. One of the advantages of using a file is that it eliminates - dependency on third party software, such as MySQL deamon and developer package. + dependency on third party database software. - The configuration of the backend (Memfile) performing writes and reads from the - file is controlled through the Dhcp4/lease-database parameters. When default - parameters are left, the Memfile backend will write leases to a disk in the + The configuration of the file backend (Memfile) + is controlled through the Dhcp4/lease-database parameters. When default + parameters are used, the Memfile backend will write leases to a disk in the [bind10-install-dir]/var/bind10/kea-leases4.csv. @@ -3738,11 +3738,9 @@ Dhcp4/subnet4 [] list (default) will change the default location of the lease file to /tmp/kea-leases4.csv. - The "persist" parameter controls whether the leases are written to disk or not. + The "persist" parameter controls whether the leases are written to disk. It is strongly recommended that this parameter is set to "true" at all times - during the normal operation of the server. The typical case when lease writes - can be disabled is testing: unit testing, performance testing when it is - desired that server is not disk-bound. + during the normal operation of the server
@@ -4906,16 +4904,16 @@ Dhcp6/subnet6/ list
Default storage for leases - Server is designed to support multiple lease database storages. In larger deployments - it is often desired to store leases in the database. The - describes one of the possible ways to do it. - By default, the server will use a flat CSV file, rather than the database to store + The server is able to store lease data in different repositories. Larger deployments + may elect to store leases in a database. + describes one way to do it. + By default, the server will use a CSV file rather than a database to store lease information. One of the advantages of using a file is that it eliminates - dependency on third party software, such as MySQL deamon and developer package. + dependency on third party database software. - The configuration of the backend (Memfile) performing writes and reads from the - file is controlled through the Dhcp4/lease-database parameters. When default + The configuration of the file backend (Memfile) + is controlled through the Dhcp6/lease-database parameters. When default parameters are left, the Memfile backend will write leases to a disk in the [bind10-install-dir]/var/bind10/kea-leases6.csv. @@ -4931,11 +4929,9 @@ Dhcp6/subnet6/ list will change the default location of the lease file to /tmp/kea-leases6.csv. - The "persist" parameter controls whether the leases are written to disk or not. + The "persist" parameter controls whether the leases are written to disk. It is strongly recommended that this parameter is set to "true" at all times - during the normal operation of the server. The typical case when lease writes - can be disabled is testing: unit testing, performance testing when it is - desired that server is not disk-bound. + during the normal operation of the server.
diff --git a/src/lib/dhcpsrv/csv_lease_file4.h b/src/lib/dhcpsrv/csv_lease_file4.h index ce68b91aed..fc13ea2389 100644 --- a/src/lib/dhcpsrv/csv_lease_file4.h +++ b/src/lib/dhcpsrv/csv_lease_file4.h @@ -29,9 +29,9 @@ namespace dhcp { /// @brief Provides methods to access CSV file with DHCPv4 leases. /// -/// This class contains methods customized to read DHCPv4 leases from the CSV -/// file. It expects that the CSV file being parsed, contains the set of columns -/// with well known names (initialized in the class constructor). +/// This class contains methods customized to read and write DHCPv4 leases from +/// and to the CSV file. It expects that the CSV file being parsed contains a +/// set of columns with well known names (initialized in the class constructor). /// /// @todo This class doesn't validate the lease values read from the file. /// The @c Lease4 is a structure that should be itself responsible for this diff --git a/src/lib/dhcpsrv/csv_lease_file6.h b/src/lib/dhcpsrv/csv_lease_file6.h index 76faa81f16..4e0c595b66 100644 --- a/src/lib/dhcpsrv/csv_lease_file6.h +++ b/src/lib/dhcpsrv/csv_lease_file6.h @@ -28,9 +28,9 @@ namespace dhcp { /// @brief Provides methods to access CSV file with DHCPv6 leases. /// -/// This class contains methods customized to read DHCPv6 leases from the CSV -/// file. It expects that the CSV file being parsed, contains the set of columns -/// with well known names (initialized in the class constructor). +/// This class contains methods customized to read and write DHCPv6 leases from +/// and to the CSV file. It expects that the CSV file being parsed contains a +/// set of columns with well known names (initialized in the class constructor). /// /// @todo This class doesn't validate the lease values read from the file. /// The @c Lease6 is a structure that should be itself responsible for this diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.h b/src/lib/dhcpsrv/memfile_lease_mgr.h index 58b31313db..f1826a3435 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.h +++ b/src/lib/dhcpsrv/memfile_lease_mgr.h @@ -36,20 +36,20 @@ namespace dhcp { /// by the @c CSVLeaseFile4 and @c CSVLeaseFile6 classes. /// /// The backend stores leases incrementally, i.e. updates to leases are appended -/// at the end of the lease file. When leases is to be deleted, the lease -/// record is appended to the lease file, with valid lifetime set to 0. +/// at the end of the lease file. To record the deletion of a lease, the lease +/// record is appended to the lease file with the valid lifetime set to 0. /// -/// When backend is starting up, it reads leases from the lease file (one by -/// one) and adds them to the in-memory container as follows: -/// - if lease record being parsed identifies a lease which is not present +/// When the backend is starting up, it reads leases from the lease file (one +/// by one) and adds them to the in-memory container as follows: +/// - if the lease record being parsed identifies a lease which is not present /// in the container, and the lease has valid lifetime greater than 0, /// the lease is added to the container, -/// - if lease record being parsed identifies a lease which is present in the -/// container, and the valid lifetime of the lease record being parsed is +/// - if the lease record being parsed identifies a lease which is present in +/// the container, and the valid lifetime of the lease record being parsed is /// greater than 0, the lease in the container is updated -/// - if lease record being parsed has valid lifetime equal to 0, and the -/// corresponding lease exists in the container, the lease is removed -/// from the container. +/// - if the lease record being parsed has valid lifetime equal to 0, and the +/// corresponding lease exists in the container, the lease is removed from +/// the container. /// /// After the container holding leases is initialized, each subsequent update, /// removal or addition of the lease is appended to the lease file diff --git a/src/lib/dhcpsrv/tests/csv_lease_file6_unittest.cc b/src/lib/dhcpsrv/tests/csv_lease_file6_unittest.cc index c1bc5b6697..40770e4efb 100644 --- a/src/lib/dhcpsrv/tests/csv_lease_file6_unittest.cc +++ b/src/lib/dhcpsrv/tests/csv_lease_file6_unittest.cc @@ -147,10 +147,10 @@ TEST_F(CSVLeaseFile6Test, parse) { EXPECT_FALSE(lease->fqdn_rev_); EXPECT_TRUE(lease->hostname_.empty()); - // Reading fourth lease should be successful. + // Reading the fourth lease should be successful. EXPECT_TRUE(lf->next(lease)); ASSERT_TRUE(lease); - // Verify that lease is correct. + // Verify that the lease is correct. EXPECT_EQ("3000:1::", lease->addr_.toText()); ASSERT_TRUE(lease->duid_); EXPECT_EQ("00:01:02:03:04:05:06:0a:0b:0c:0d:0e:0f", lease->duid_->toText()); diff --git a/src/lib/util/csv_file.cc b/src/lib/util/csv_file.cc index 6fc44ca4b6..637106ff54 100644 --- a/src/lib/util/csv_file.cc +++ b/src/lib/util/csv_file.cc @@ -334,14 +334,14 @@ CSVFile::open() { void CSVFile::recreate() { - // There is no sense to create a file is we don't specify columns for it. + // There is no sense creating a file if we don't specify columns for it. if (getColumnCount() == 0) { close(); isc_throw(CSVFileError, "no columns defined for the newly" " created CSV file '" << filename_ << "'"); } - // Close any danglining files. + // Close any dangling files. close(); fs_.reset(new std::fstream(filename_.c_str(), std::fstream::out)); if (!fs_->is_open()) { diff --git a/src/lib/util/csv_file.h b/src/lib/util/csv_file.h index f832b82bcb..591455cd33 100644 --- a/src/lib/util/csv_file.h +++ b/src/lib/util/csv_file.h @@ -36,27 +36,26 @@ public: /// @brief Represents a single row of the CSV file. /// /// The object of this type can create the string holding a collection of the -/// comma separated values, representing a row of the CSV file. It allows to -/// select ANY character as a separator for the values. The default separator -/// is a comma sign. +/// comma separated values, representing a row of the CSV file. It allows the +/// selection of any character as a separator for the values. The default +/// separator is the comma symbol. /// /// The @c CSVRow object can be constructed in two different ways. The first /// option is that the caller creates an object holding empty values /// and then adds values one by one. Note that it is possible to either add -/// a string or a number. The number is stringified to the appropriate text +/// a string or a number. The number is converted to the appropriate text /// representation. When all the values are added, the text representation of /// the row can be obtained by calling @c CSVRow::render function or output /// stream operator. /// -/// The @c CSVRow object can be also constructed by parsing the row of the -/// CSV file. In this case, the separator has to be known in advance and -/// passed to the class constructor. Constructor will call the @c CSVRow::parse -/// function internally to tokenize the CSV row and create collection of the -/// values. The class accessors can be then used to retrieve individual -/// values. +/// The @c CSVRow object can be also constructed by parsing a row of a CSV +/// file. In this case, the separator has to be known in advance and passed to +/// the class constructor. The constructor will call the @c CSVRow::parse +/// function internally to tokenize the CSV row and create the collection of +/// values. The class accessors can be then used to retrieve individual values. /// /// This class is meant to be used by the @c CSVFile class to manipulate -/// the individual rows of the CSV file. +/// individual rows of the CSV file. class CSVRow { public: @@ -71,7 +70,7 @@ public: /// This constructor is exception-free. /// /// @param cols Number of values in the row. - /// @param separator Character being a separator between values in the + /// @param separator Character used as a separator between values in the /// text representation of the row. CSVRow(const size_t cols = 0, const char separator = ','); @@ -80,7 +79,7 @@ public: /// This constructor should be used to parse a single row of the CSV /// file. The separator being used for the particular row needs to /// be known in advance and specified as an argument of the constructor - /// if other than default separator is used in the row being parsed. + /// if other than the default separator is used in the row being parsed. /// An example string to be parsed by this function looks as follows: /// "foo,bar,foo-bar". /// @@ -147,8 +146,8 @@ public: /// @brief Creates a text representation of the CSV file row. /// /// This function iterates over all values currently held in the internal - /// @c values_ container and appends them into a string. The values are - /// separated using a separator character specified in the constructor. + /// @c values_ container and appends them to a string. The values are + /// separated using the separator character specified in the constructor. /// /// This function is exception free. /// -- GitLab