Replaced some of the surviving C string manipulation code of Sebastien and

Dennis used within ithe LoggerImplementation class with C++ std::ostringstream
code.

castor/log/Constants.hpp

-/******************************************************************************
- *                      castor/log/Constants.hpp
- *
- * This file is part of the Castor project.
- * See http://castor.web.cern.ch/castor
- *
- * Copyright (C) 2003  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 2
- * 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, write to the Free Software
- * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
- *
- *
- * Interface to the CASTOR logging system
- *
- * @author Steven.Murray@cern.ch
- *****************************************************************************/
-
-#ifndef CASTOR_LOG_CONSTANTS_HPP
-#define CASTOR_LOG_CONSTANTS_HPP 1
-
-#include <stdlib.h>
-
-namespace castor {
-namespace log {
-
-/**
- * Maximum length of the program name to be prepended to every log message.
- */
-const size_t LOG_MAX_PROGNAMELEN = 20;
-
-} // namespace log
-} // namespace castor
-
-#endif // CASTOR_LOG_CONSTANTS_HPP

castor/log/LoggerImplementation.cpp

@@ -22,12 +22,12 @@
  * @author Steven.Murray@cern.ch
  *****************************************************************************/
 
-#include "castor/log/Constants.hpp"
 #include "castor/log/LoggerImplementation.hpp"
 #include "h/Castor_limits.h"
 #include "h/getconfent.h"
 
 #include <errno.h>
+#include <sstream>
 #include <string.h>
 #include <sys/un.h>
 #include <sys/types.h>
@@ -48,7 +48,6 @@ castor::log::LoggerImplementation::LoggerImplementation(
   m_logFile(-1),
   m_connected(false),
   m_priorityToText(generatePriorityToTextMap()) {
-  checkProgramNameLen(programName);
   initMutex();
 }
 
@@ -122,19 +121,6 @@ std::map<int, std::string>
   return m;
 }
 
-//------------------------------------------------------------------------------
-// checkProgramNameLen
-//------------------------------------------------------------------------------
-void castor::log::LoggerImplementation::checkProgramNameLen(
-  const std::string &programName) throw(castor::exception::InvalidArgument) {
-  if(programName.length() > LOG_MAX_PROGNAMELEN) {
-    castor::exception::InvalidArgument ex;
-    ex.getMessage() << "Invalid argument: programName is too log: max=" <<
-      LOG_MAX_PROGNAMELEN << " actual=" << programName.length();
-    throw ex;
-  }
-}
-
 //------------------------------------------------------------------------------
 // initMutex
 //------------------------------------------------------------------------------
@@ -262,24 +248,22 @@ void castor::log::LoggerImplementation::logMsg(
   // Safe to get a reference to the textual representation of the priority
   const std::string &priorityText = priorityTextPair->second;
 
-  // Initialize buffer
-  char buffer[LOG_MAX_LINELEN * 2];
-  memset(buffer,  '\0', sizeof(buffer));
+  std::ostringstream logMsg;
 
   // Start message with priority, time, program and PID (syslog standard
   // format)
-  size_t len = buildSyslogHeader(buffer, m_maxMsgLen, priority | LOG_LOCAL3,
-    timeStamp, getpid());
+  logMsg << buildSyslogHeader(priority | LOG_LOCAL3, timeStamp, getpid());
 
-  // Append the log level, the thread id and the message text
+  // Determine the thread id
 #ifdef __APPLE__
-  len += snprintf(buffer + len, m_maxMsgLen - len, "LVL=%s TID=%d MSG=\"%s\" ",
-    priorityText.c_str(),(int)mach_thread_self(), msg.c_str());
+  const int tid = mach_thread_self();
 #else
-  len += snprintf(buffer + len, m_maxMsgLen - len, "LVL=%s TID=%d MSG=\"%s\" ",
-    priorityText.c_str(),(int)syscall(__NR_gettid), msg.c_str());
+  const int tid = syscall(__NR_gettid);
 #endif
 
+  // Append the log level, the thread id and the message text
+  logMsg << "LVL=" << priorityText << " TID=" << tid << " MSG=\"" << msg << "\" ";
+
   // Process parameters
   for(int i = 0; i < numParams; i++) {
     const Param &param = params[i];
@@ -293,50 +277,51 @@ void castor::log::LoggerImplementation::logMsg(
     const std::string value = cleanString(param.getValue(), false);
 
     // Write the name and value to the buffer
-    len += snprintf(buffer + len, m_maxMsgLen - len, "%.*s=\"%.*s\" ",
-      (int)LOG_MAX_PARAMNAMELEN, name.c_str(),
-      (int)LOG_MAX_PARAMSTRLEN, value.c_str());
-
-    // Check if there is enough space in the buffer
-    if(len >= m_maxMsgLen) {
-      buffer[m_maxMsgLen - 1] = '\n';
-      break;
-    }
+    logMsg << name << "=\"" << value << "\" ";
   }
 
   // Terminate the string
-  if(len < m_maxMsgLen) {
-    len += snprintf(buffer + (len - 1), m_maxMsgLen - len, "\n");
+  logMsg << "\n";
+
+  // If the message is too long for syslog then truncate it before calling
+  // reducedSyslog
+  if(logMsg.str().length() > m_maxMsgLen) {
+    std::ostringstream truncatedLogMsg;
+    truncatedLogMsg << logMsg.str().substr(0, m_maxMsgLen - 1);
+    truncatedLogMsg << "\n";
+    reducedSyslog(truncatedLogMsg.str());
+  } else {
+    reducedSyslog(logMsg.str());
   }
-
-  reducedSyslog(buffer, len);
 }
 
 //-----------------------------------------------------------------------------
 // buildSyslogHeader
 //-----------------------------------------------------------------------------
-int castor::log::LoggerImplementation::buildSyslogHeader(
-  char *const buffer,
-  const int buflen,
+std::string castor::log::LoggerImplementation::buildSyslogHeader(
   const int priority,
   const struct timeval &timeStamp,
   const int pid) const throw() {
-  struct tm tmp;
-  int len = snprintf(buffer, buflen, "<%d>", priority);
-  localtime_r(&(timeStamp.tv_sec), &tmp);
-  len += strftime(buffer + len, buflen - len, "%Y-%m-%dT%T", &tmp);
-  len += snprintf(buffer + len, buflen - len, ".%06ld",
+  char buf[80];
+  int bufLen = sizeof(buf);
+  int len = 0;
+  std::ostringstream oss;
+
+  oss << "<" << priority << ">";
+
+  struct tm localTime;
+  localtime_r(&(timeStamp.tv_sec), &localTime);
+  len += strftime(buf, bufLen, "%Y-%m-%dT%T", &localTime);
+  len += snprintf(buf + len, bufLen - len, ".%06ld",
     (unsigned long)timeStamp.tv_usec);
-  len += strftime(buffer + len, buflen - len, "%z: ", &tmp);
+  len += strftime(buf + len, bufLen - len, "%z: ", &localTime);
   // dirty trick to have the proper timezone format (':' between hh and mm)
-  buffer[len-2] = buffer[len-3];
-  buffer[len-3] = buffer[len-4];
-  buffer[len-4] = ':';
-  // if no source given, you by default the name of the process in which we run
-  // print source and pid
-  len += snprintf(buffer + len, buflen - len, "%s[%d]: ",
-    m_programName.c_str(), pid);
-  return len;
+  buf[len-2] = buf[len-3];
+  buf[len-3] = buf[len-4];
+  buf[len-4] = ':';
+  buf[sizeof(buf) - 1] = '\0';
+  oss << buf << m_programName.c_str() << "[" << pid << "]: ";
+  return oss.str();
 }
 
 //-----------------------------------------------------------------------------
@@ -391,8 +376,8 @@ std::string castor::log::LoggerImplementation::cleanString(const std::string &s,
 //-----------------------------------------------------------------------------
 // reducedSyslog
 //-----------------------------------------------------------------------------
-void castor::log::LoggerImplementation::reducedSyslog(const char *const msg,
-  const int msgLen) throw() {
+void castor::log::LoggerImplementation::reducedSyslog(const std::string &msg)
+  throw() {
   int send_flags = 0;
 #ifndef __APPLE__
   // MAC has has no MSG_NOSIGNAL
@@ -402,21 +387,25 @@ void castor::log::LoggerImplementation::reducedSyslog(const char *const msg,
   // enter critical section
   pthread_mutex_lock(&m_mutex);
 
-  // Get connected, output the message to the local logger.
-  if (!m_connected) {
+  // Try to connect if not already connected
+  if(!m_connected) {
     openLog();
   }
 
-  if (!m_connected ||
-    send(m_logFile, msg, msgLen, send_flags) < 0) {
-    if (m_connected) {
-      // Try to reopen the syslog connection.  Maybe it went down.
+  // If connected
+  if(m_connected) {
+    // If sending the log message fails then try to reopen the syslog
+    // connection and try again
+    if(0 > send(m_logFile, msg.c_str(), msg.length(), send_flags)) {
       closeLog();
       openLog();
-    }
-    if (!m_connected ||
-      send(m_logFile, msg, msgLen, send_flags) < 0) {
-      closeLog();  // attempt re-open next time
+      if (m_connected) {
+        // If the second attempt to send the log message fails then give up and
+        // attempt re-open next time
+        if(0 > send(m_logFile, msg.c_str(), msg.length(), send_flags)) {
+          closeLog();
+        }
+      }
     }
   }
 

castor/log/LoggerImplementation.hpp

@@ -164,13 +164,6 @@ private:
    */
   const std::map<int, std::string> m_priorityToText;
 
-  /**
-   * Throws castor::exception::InvalidArgument if the specified program name is
-   * too long.
-   */
-  void checkProgramNameLen(const std::string &programName)
-    throw(castor::exception::InvalidArgument);
-
   /**
    * Determines the maximum message length that the client syslog server can
    * handle.
@@ -210,10 +203,13 @@ private:
 
   /**
    * Build the header of a syslog message.
+   *
+   * @param priority The priority of the message.
+   * @param timeStamp The time stamp of the message.
+   * @param pid The process ID of the process logging the message.
+   * @return The header of the syslog message.
    */
-  int buildSyslogHeader(
-    char *const buffer,
-    const int buflen,
+  std::string buildSyslogHeader(
     const int priority,
     const struct timeval &timeStamp,
     const int pid) const throw();
@@ -235,9 +231,8 @@ private:
    * creation, such as when retrieving logs from the DB.
    *
    * @param msg The message to be logged.
-   * @param msgLen The length of the message top be logged.
    */
-  void reducedSyslog(const char *const msg, const int msgLen) throw();
+  void reducedSyslog(const std::string &msg) throw();
 
   /**
    * Closes the connection to syslog.

test/unittest/castor/log/LoggerImplementationTest.cpp

@@ -22,7 +22,6 @@
  * @author Steven.Murray@cern.ch
  *****************************************************************************/
 
-#include "castor/log/Constants.hpp"
 #include "castor/log/LoggerImplementation.hpp"
 
 #include <cppunit/extensions/HelperMacros.h>
@@ -41,22 +40,6 @@ public:
   void tearDown() {
   }
 
-  void testConstructorTooLongProgramName() {
-    // Create a program name that is 1 character too long
-    std::string tooLongProgname;
-    for(size_t i = 0; i <= LOG_MAX_PROGNAMELEN; i++) {
-      tooLongProgname += 'X';
-    }
-
-    {
-      std::auto_ptr<Logger> logger;
-      CPPUNIT_ASSERT_THROW_MESSAGE(
-        "Checking a program name that is too long throws InvalidArgument",
-        logger.reset(new LoggerImplementation(tooLongProgname)),
-        castor::exception::InvalidArgument);
-    }
-  }
-
   void testLogMsgWithAllParams() {
     std::auto_ptr<Logger> logger;
     CPPUNIT_ASSERT_NO_THROW_MESSAGE(
@@ -114,7 +97,6 @@ public:
   }   
 
   CPPUNIT_TEST_SUITE(LoggerImplementationTest);
-  CPPUNIT_TEST(testConstructorTooLongProgramName);
   CPPUNIT_TEST(testLogMsgWithAllParams);
   CPPUNIT_TEST(testLogMsgWithoutTimeStamp);
   CPPUNIT_TEST(testLogMsgWithoutParamsOrTimeStamp);

test/unittest/castor/log/ParamTest.cpp

@@ -22,7 +22,6 @@
  * @author Steven.Murray@cern.ch
  *****************************************************************************/
 
-#include "castor/log/Constants.hpp"
 #include "castor/log/Param.hpp"
 
 #include <cppunit/extensions/HelperMacros.h>