From 65b303381ab49a04baec57b1837f67d435294602 Mon Sep 17 00:00:00 2001
From: Eric Cano <Eric.Cano@cern.ch>
Date: Wed, 16 Mar 2016 17:00:54 +0100
Subject: [PATCH] Synchronized (and simplified) the implementation of tpconfig
with the parsing of cta.conf. Renamed TponfigLines class to Tpconfig. Unit
tested the class. Fixed message value of Errnum being overwritten when
"ex.getMessage() <<" construct is used on the object.
---
common/exception/Errnum.cpp | 2 +-
common/threading/SocketPairTest.cpp | 1 -
tapeserver/cta-taped.cpp | 2 +-
tapeserver/daemon/CMakeLists.txt | 5 +-
tapeserver/daemon/Tpconfig.cpp | 81 +++++++++
.../{TpconfigLines.hpp => Tpconfig.hpp} | 4 +-
tapeserver/daemon/TpconfigLine.cpp | 11 +-
tapeserver/daemon/TpconfigLine.hpp | 5 +-
tapeserver/daemon/TpconfigLines.cpp | 161 ------------------
tapeserver/daemon/TpconfigTests.cpp | 89 ++++++++++
10 files changed, 189 insertions(+), 172 deletions(-)
create mode 100644 tapeserver/daemon/Tpconfig.cpp
rename tapeserver/daemon/{TpconfigLines.hpp => Tpconfig.hpp} (91%)
delete mode 100644 tapeserver/daemon/TpconfigLines.cpp
create mode 100644 tapeserver/daemon/TpconfigTests.cpp
diff --git a/common/exception/Errnum.cpp b/common/exception/Errnum.cpp
index ceb4dac62a..b92077cd9c 100644
--- a/common/exception/Errnum.cpp
+++ b/common/exception/Errnum.cpp
@@ -40,7 +40,7 @@ void Errnum::ErrnumConstructorBottomHalf(const std::string & what) {
if (what.size())
w2 << what << " ";
w2 << "Errno=" << m_errnum << ": " << m_strerror;
- getMessage().str(w2.str());
+ getMessage() << w2.str();
}
void Errnum::throwOnReturnedErrno (const int err, const std::string &context) {
diff --git a/common/threading/SocketPairTest.cpp b/common/threading/SocketPairTest.cpp
index 31b357bee0..ed4b5276cf 100644
--- a/common/threading/SocketPairTest.cpp
+++ b/common/threading/SocketPairTest.cpp
@@ -96,7 +96,6 @@ TEST(cta_threading_SocketPair, MaxLength) {
ASSERT_EQ(smallMessage, sp.receive(SocketPair::Side::child));
ASSERT_EQ(bigMessage, sp.receive(SocketPair::Side::child));
ASSERT_EQ(hugeMessage, sp.receive(SocketPair::Side::child));
- //ASSERT_THROW(sp.receive(SocketPair::Side::child), SocketPair::Overflow);
ASSERT_EQ(smallMessage, sp.receive(SocketPair::Side::child));
}
diff --git a/tapeserver/cta-taped.cpp b/tapeserver/cta-taped.cpp
index f8fbe66235..2132b6978a 100644
--- a/tapeserver/cta-taped.cpp
+++ b/tapeserver/cta-taped.cpp
@@ -22,7 +22,7 @@
#include "common/processCap/ProcessCap.hpp"
#include "tapeserver/daemon/CommandLineParams.hpp"
#include "tapeserver/daemon/TapedConfiguration.hpp"
-#include "tapeserver/daemon/TpconfigLines.hpp"
+#include "tapeserver/daemon/Tpconfig.hpp"
#include "tapeserver/daemon/TapeDaemon.hpp"
#include "version.h"
diff --git a/tapeserver/daemon/CMakeLists.txt b/tapeserver/daemon/CMakeLists.txt
index ae3e08dd3c..0708bceb23 100644
--- a/tapeserver/daemon/CMakeLists.txt
+++ b/tapeserver/daemon/CMakeLists.txt
@@ -7,11 +7,12 @@ add_library(ctatapedaemon
TapedConfiguration.cpp
TapeDaemon.cpp
TpconfigLine.cpp
- TpconfigLines.cpp)
+ Tpconfig.cpp)
add_library(ctadaemonunittests SHARED
ConfigurationFileTests.cpp
- TapedConfigurationTests.cpp)
+ TapedConfigurationTests.cpp
+ TpconfigTests.cpp)
target_link_libraries(ctadaemonunittests
ctatapedaemon
diff --git a/tapeserver/daemon/Tpconfig.cpp b/tapeserver/daemon/Tpconfig.cpp
new file mode 100644
index 0000000000..cdda65ea91
--- /dev/null
+++ b/tapeserver/daemon/Tpconfig.cpp
@@ -0,0 +1,81 @@
+/*
+ * The CERN Tape Archive (CTA) project
+ * Copyright (C) 2015 CERN
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "tapeserver/daemon/Tpconfig.hpp"
+#include "common/utils/utils.hpp"
+#include "common/exception/Errnum.hpp"
+
+#include <errno.h>
+#include <memory>
+#include <fstream>
+#include <algorithm>
+
+//------------------------------------------------------------------------------
+// parseTpconfigFile
+//------------------------------------------------------------------------------
+cta::tape::daemon::Tpconfig
+cta::tape::daemon::Tpconfig::parseFile(const std::string &filename) {
+ Tpconfig ret;
+ // Try to open the configuration file, throwing an exception if there is a
+ // failure
+ std::ifstream file(filename);
+ if (file.fail()) {
+ cta::exception::Errnum ex;
+ ex.getMessage() << " In Tpconfig::parseFile()"
+ ": Failed to open tpConfig file"
+ ": filename=" << filename;
+ throw ex;
+ }
+
+ std::string line;
+ size_t lineNumber=0;
+ while(++lineNumber, std::getline(file, line)) {
+ // If there is a comment, then remove it from the line
+ line = line.substr(0, line.find('#'));
+ // get rid of potential tabs
+ std::replace(line.begin(),line.end(),'\t',' ');
+ std::istringstream sline(line);
+ // This is the expected fields in the line.
+ std::string unitName, logicalLibarry, devFilePath, librarySlot;
+ // If there is nothing on the line, ignore it.
+ if (!(sline >> unitName)) continue;
+ // But if there is anything, we expect all parameters.
+ if (!( (sline >> logicalLibarry)
+ &&(sline >> devFilePath)
+ &&(sline >> librarySlot))) {
+ cta::exception::Exception ex;
+ ex.getMessage() << "In Tpconfig::parseFile(): " <<
+ "missing parameter(s) from line=" << line <<
+ " filename=" << filename;
+ throw ex;
+ }
+ std::string extra;
+ if ( sline >> extra) {
+ cta::exception::Exception ex;
+ ex.getMessage() << "In Tpconfig::parseFile(): " <<
+ "extra parameter(s) from line=" << line <<
+ " filename=" << filename;
+ throw ex;
+ }
+ // The constructor implicitly validates the lengths
+ const TpconfigLine configLine(unitName, logicalLibarry, devFilePath, librarySlot);
+ // Store the value of the data-columns in the output list parameter
+ ret.push_back(TpconfigLine(configLine));
+ }
+ return ret;
+}
diff --git a/tapeserver/daemon/TpconfigLines.hpp b/tapeserver/daemon/Tpconfig.hpp
similarity index 91%
rename from tapeserver/daemon/TpconfigLines.hpp
rename to tapeserver/daemon/Tpconfig.hpp
index 0810f48a01..ed497be9d9 100644
--- a/tapeserver/daemon/TpconfigLines.hpp
+++ b/tapeserver/daemon/Tpconfig.hpp
@@ -28,7 +28,7 @@ namespace cta { namespace tape { namespace daemon {
/**
* A list of lines parsed from a TPCONFIG file.
*/
-class TpconfigLines: public std::list<TpconfigLine> {
+class Tpconfig: public std::list<TpconfigLine> {
public:
CTA_GENERATE_EXCEPTION_CLASS(InvalidArgument);
@@ -38,7 +38,7 @@ public:
* @param filename The filename of the TPCONFIG file.
* @return The result of parsing the TPCONFIG file.
*/
- static TpconfigLines parseFile(const std::string &filename);
+ static Tpconfig parseFile(const std::string &filename);
}; // class TpconfigLines
}}} // namespace cta::tape::daemon
diff --git a/tapeserver/daemon/TpconfigLine.cpp b/tapeserver/daemon/TpconfigLine.cpp
index 9f30af84c4..0026913df0 100644
--- a/tapeserver/daemon/TpconfigLine.cpp
+++ b/tapeserver/daemon/TpconfigLine.cpp
@@ -17,6 +17,7 @@
*/
#include "tapeserver/daemon/TpconfigLine.hpp"
+#include "common/exception/Exception.hpp"
//------------------------------------------------------------------------------
// Constructor.
@@ -25,9 +26,17 @@ cta::tape::daemon::TpconfigLine::TpconfigLine(
const std::string &unitName,
const std::string &logicalLibrary,
const std::string &devFilename,
- const std::string &librarySlot) throw():
+ const std::string &librarySlot):
unitName(unitName),
logicalLibrary(logicalLibrary),
devFilename(devFilename),
librarySlot(librarySlot) {
+ if (unitName.size() > maxNameLen)
+ throw cta::exception::Exception("In TpconfigLine::TpconfigLine: unitName too long");
+ if (logicalLibrary.size() > maxNameLen)
+ throw cta::exception::Exception("In TpconfigLine::TpconfigLine: logicalLibrary too long");
+ if (devFilename.size() > maxNameLen)
+ throw cta::exception::Exception("In TpconfigLine::TpconfigLine: devFilename too long");
+ if (librarySlot.size() > maxNameLen)
+ throw cta::exception::Exception("In TpconfigLine::TpconfigLine: librarySlot too long");
}
diff --git a/tapeserver/daemon/TpconfigLine.hpp b/tapeserver/daemon/TpconfigLine.hpp
index 50c8a43823..9c77107edb 100644
--- a/tapeserver/daemon/TpconfigLine.hpp
+++ b/tapeserver/daemon/TpconfigLine.hpp
@@ -60,10 +60,9 @@ struct TpconfigLine {
const std::string &unitName,
const std::string &logicalLibrary,
const std::string &devFilename,
- const std::string &librarySlot) throw();
+ const std::string &librarySlot);
- static const size_t maxUnitNameLen;
- static const size_t maxLogicalLibraryNameLen;
+ static const size_t maxNameLen = 100;
}; // struct TpconfigLine
}}} // namespace cta::tape::daemon
diff --git a/tapeserver/daemon/TpconfigLines.cpp b/tapeserver/daemon/TpconfigLines.cpp
deleted file mode 100644
index 0657752f5a..0000000000
--- a/tapeserver/daemon/TpconfigLines.cpp
+++ /dev/null
@@ -1,161 +0,0 @@
-/*
- * The CERN Tape Archive (CTA) project
- * Copyright (C) 2015 CERN
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program. If not, see <http://www.gnu.org/licenses/>.
- */
-
-#include "tapeserver/daemon/TpconfigLines.hpp"
-#include "common/utils/utils.hpp"
-#include "common/exception/Errnum.hpp"
-
-#include <errno.h>
-#include <memory>
-
-//------------------------------------------------------------------------------
-// parseTpconfigFile
-//------------------------------------------------------------------------------
-cta::tape::daemon::TpconfigLines
-cta::tape::daemon::TpconfigLines::parseFile(const std::string &filename) {
- TpconfigLines lines;
-
- // Open the TPCONFIG file for reading
- std::unique_ptr<FILE, decltype(&::fclose)> file(fopen(filename.c_str(), "r"), &::fclose);
- {
- const int savedErrno = errno;
-
- // Throw an exception if the file could not be opened
- if(file.get() == NULL) {
- cta::exception::Errnum ex(savedErrno);
-
- ex.getMessage() <<
- "Failed to parse TPCONFIG file"
- ": Failed to open file"
- ": filename='" << filename << "'"
- ": " << cta::utils::errnoToString(savedErrno);
-
- throw ex;
- }
- }
-
- // Line buffer
- char lineBuf[1024];
-
- // The error number recorded immediately after fgets is called
- int fgetsErrno = 0;
-
- // For each line was read
- for(int lineNb = 1; fgets(lineBuf, sizeof(lineBuf), file.get()); lineNb++) {
- fgetsErrno = errno;
-
- // Create a std::string version of the line
- std::string line(lineBuf);
-
- // Remove the newline character if there is one
- {
- const std::string::size_type newlinePos = line.find("\n");
- if(newlinePos != std::string::npos) {
- line = line.substr(0, newlinePos);
- }
- }
-
- // If there is a comment, then remove it from the line
- {
- const std::string::size_type startOfComment = line.find("#");
- if(startOfComment != std::string::npos) {
- line = line.substr(0, startOfComment);
- }
- }
-
- // Left and right trim the line of whitespace
- line = cta::utils::trimString(std::string(line));
-
- // If the line is not empty
- if(line != "") {
-
- // Replace each occurance of whitespace with a single space
- line = cta::utils::singleSpaceString(line);
-
- // Split the line into its component data-columns
- std::vector<std::string> columns;
- cta::utils::splitString(line, ' ', columns);
-
- // The expected number of data-columns in a TPCONFIG data-line is 4:
- // unitName dgn devFilename librarySlot
- const unsigned int expectedNbOfColumns = 4;
-
- // Throw an exception if the number of data-columns is invalid
- if(columns.size() != expectedNbOfColumns) {
- InvalidArgument ex;
- ex.getMessage() <<
- "Failed to parse TPCONFIG file"
- ": Invalid number of data columns in TPCONFIG line"
- ": filename='" << filename << "'"
- " lineNb=" << lineNb <<
- " expectedNbColumns=" << expectedNbOfColumns <<
- " actualNbColumns=" << columns.size() <<
- " expectedFormat='unitName dgn devFilename librarySlot'";
- throw ex;
- }
-
- const TpconfigLine configLine(
- columns[0], // unitName
- columns[1], // logicalLibrary
- columns[2], // devFilename
- columns[3] // librarySlot
- );
-
- if(TpconfigLine::maxUnitNameLen < configLine.unitName.length()) {
- InvalidArgument ex;
- ex.getMessage() <<
- "Failed to parse TPCONFIG file"
- ": Tape-drive unit-name is too long"
- ": filename='" << filename << "'"
- " lineNb=" << lineNb <<
- " unitName=" << configLine.unitName <<
- " maxUnitNameLen=" << TpconfigLine::maxUnitNameLen <<
- " actualUnitNameLen=" << configLine.unitName.length();
- throw ex;
- }
-
- if(TpconfigLine::maxLogicalLibraryNameLen < configLine.logicalLibrary.length()) {
- InvalidArgument ex;
- ex.getMessage() <<
- "Failed to parse TPCONFIG file"
- ": logical library is too long"
- ": filename='" << filename << "'"
- " lineNb=" << lineNb <<
- " logicalLibrary=" << configLine.logicalLibrary <<
- " maxLogicalLibraryLen=" << TpconfigLine::maxLogicalLibraryNameLen <<
- " actualLogicalLibraryLen=" << configLine.logicalLibrary.length();
- throw ex;
- }
-
- // Store the value of the data-columns in the output list parameter
- lines.push_back(TpconfigLine(configLine));
- }
- }
-
- // Throw an exception if there was error whilst reading the file
- if(ferror(file.get())) {
- std::stringstream err;
- err <<"Failed to parse TPCONFIG file"
- ": Failed to read file"
- ": filename='" << filename << "'";
- cta::exception::Errnum ex(fgetsErrno, err.str());
- throw ex;
- }
-
- return lines;
-}
diff --git a/tapeserver/daemon/TpconfigTests.cpp b/tapeserver/daemon/TpconfigTests.cpp
new file mode 100644
index 0000000000..b79eea25ac
--- /dev/null
+++ b/tapeserver/daemon/TpconfigTests.cpp
@@ -0,0 +1,89 @@
+/*
+ * The CERN Tape Archive (CTA) project
+ * Copyright (C) 2015 CERN
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <gtest/gtest.h>
+
+#include "tapeserver/daemon/Tpconfig.hpp"
+#include "tests/TempFile.hpp"
+#include "common/exception/Errnum.hpp"
+
+namespace unitTests {
+
+TEST(cta_Daemon, Tpconfig_base) {
+ TempFile tf;
+ // Test with empty file
+ tf.stringFill("");
+ cta::tape::daemon::Tpconfig tpc = cta::tape::daemon::Tpconfig::parseFile(tf.path());
+ ASSERT_EQ(0, tpc.size());
+ // Test with file full of comments (but no valid line)
+ tf.stringFill("# some comment\n"
+ "\t \t # Some non-empty line (spaces)\n"
+ "\t\t\t \n");
+ tpc = cta::tape::daemon::Tpconfig::parseFile(tf.path());
+ ASSERT_EQ(0, tpc.size());
+
+ // Test with non-existing file
+ ASSERT_THROW(cta::tape::daemon::Tpconfig::parseFile("/no/such/file"), cta::exception::Errnum);
+ // Check we get the expected Errno.
+ try {
+ cta::tape::daemon::Tpconfig::parseFile("/no/such/file");
+ ASSERT_TRUE(false); // We should never get here.
+ } catch (cta::exception::Errnum & ex) {
+ ASSERT_NE(std::string::npos, ex.getMessageValue().find("Errno=2:"));
+ }
+
+ // Test with a line too short
+ tf.stringFill("TapeDrive");
+ ASSERT_THROW(cta::tape::daemon::Tpconfig::parseFile(tf.path()), cta::exception::Exception);
+ try {
+ cta::tape::daemon::Tpconfig::parseFile(tf.path());
+ ASSERT_TRUE(false); // We should never get here.
+ } catch (cta::exception::Exception & ex) {
+ ASSERT_NE(std::string::npos, ex.getMessageValue().find("missing"));
+ }
+
+ // Test with line too long
+ tf.stringFill("TapeDrive lib /dev/tape libSlot ExtraArgument");
+ ASSERT_THROW(cta::tape::daemon::Tpconfig::parseFile(tf.path()), cta::exception::Exception);
+ try {
+ cta::tape::daemon::Tpconfig::parseFile(tf.path());
+ ASSERT_TRUE(false); // We should never get here.
+ } catch (cta::exception::Exception & ex) {
+ ASSERT_NE(std::string::npos, ex.getMessageValue().find("extra parameter"));
+ }
+
+ // Test with several entries (valid file with various extra blanks)
+ tf.stringFill(" drive0 lib0 \t\t\t /dev/tape0 lib0slot0\n"
+ "drive1 lib0 /dev/tape1 lib0slot1 \n"
+ "drive2 lib0 /dev/tape2 lib0slot2");
+ tpc = cta::tape::daemon::Tpconfig::parseFile(tf.path());
+ ASSERT_EQ(3, tpc.size());
+ int i=0;
+ for(auto & t: tpc) {
+ ASSERT_EQ("drive", t.unitName.substr(0,5));
+ ASSERT_EQ("lib0", t.logicalLibrary);
+ ASSERT_EQ("/dev/tape", t.devFilename.substr(0,9));
+ ASSERT_EQ("lib0slot", t.librarySlot.substr(0,8));
+ ASSERT_EQ('0'+i, t.unitName.back());
+ ASSERT_EQ('0'+i, t.devFilename.back());
+ ASSERT_EQ('0'+i, t.librarySlot.back());
+ i++;
+ }
+}
+
+} // namespace unitTests
--
GitLab