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

@@ -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) {

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));
 }
 

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"

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

tapeserver/daemon/Tpconfig.cpp

+/*
+ * 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;
+}

tapeserver/daemon/TpconfigLines.hpp → 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

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");
 }

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

tapeserver/daemon/TpconfigLines.cpp

-/*
- * 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;
-}

tapeserver/daemon/TpconfigTests.cpp

+/*
+ * 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