trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a..@apache.org
Subject trafficserver git commit: TS-306: Enable log rotation for diags.log & traffic.out
Date Wed, 04 Nov 2015 19:27:18 GMT
Repository: trafficserver
Updated Branches:
  refs/heads/master 66ccfd0ed -> 7bcc3b4f6


TS-306: Enable log rotation for diags.log & traffic.out

Fix coverity and clang-analyzer issues

Fixed coverity issues:
   1338077 1338076 1338075 1338074 1338073 1338072 1338071


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/7bcc3b4f
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/7bcc3b4f
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/7bcc3b4f

Branch: refs/heads/master
Commit: 7bcc3b4f6acc003a4cc4478c2f7416590512afa3
Parents: 66ccfd0
Author: Daniel Xu <dlxu2@yahoo.com>
Authored: Wed Nov 4 00:16:03 2015 -0600
Committer: Alan M. Carroll <amc@apache.org>
Committed: Wed Nov 4 11:18:45 2015 -0600

----------------------------------------------------------------------
 lib/ts/BaseLogFile.h        |  3 +--
 lib/ts/Diags.cc             | 20 +++++++++++++++-----
 proxy/Main.cc               |  1 -
 proxy/logging/Log.cc        |  7 ++++++-
 proxy/logging/LogFile.cc    |  3 +++
 proxy/shared/DiagsConfig.cc |  4 ++--
 6 files changed, 27 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7bcc3b4f/lib/ts/BaseLogFile.h
----------------------------------------------------------------------
diff --git a/lib/ts/BaseLogFile.h b/lib/ts/BaseLogFile.h
index 2308118..af20227 100644
--- a/lib/ts/BaseLogFile.h
+++ b/lib/ts/BaseLogFile.h
@@ -109,7 +109,7 @@ public:
     _read_from_file();
   }
 
-  BaseMetaInfo(char *filename, time_t creation) : _creation_time(creation), _flags(VALID_CREATION_TIME)
+  BaseMetaInfo(char *filename, time_t creation) : _creation_time(creation), _log_object_signature(0),
_flags(VALID_CREATION_TIME)
   {
     _build_name(filename);
     _write_to_file();
@@ -123,7 +123,6 @@ public:
   }
 
   ~BaseMetaInfo() { ats_free(_filename); }
-
   bool
   get_creation_time(time_t *time)
   {

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7bcc3b4f/lib/ts/Diags.cc
----------------------------------------------------------------------
diff --git a/lib/ts/Diags.cc b/lib/ts/Diags.cc
index 00f802c..ef419a6 100644
--- a/lib/ts/Diags.cc
+++ b/lib/ts/Diags.cc
@@ -650,8 +650,11 @@ Diags::should_roll_diagslog()
   // Roll diags_log if necessary
   if (diags_log && diags_log->is_init()) {
     if (diagslog_rolling_enabled == RollingEnabledValues::ROLL_ON_SIZE) {
+      // if we can't even check the file, we can forget about rotating
       struct stat buf;
-      fstat(fileno(diags_log->m_fp), &buf);
+      if (fstat(fileno(diags_log->m_fp), &buf) != 0)
+        return false;
+
       int size = buf.st_size;
       if (diagslog_rolling_size != -1 && size >= (diagslog_rolling_size * BYTES_IN_MB))
{
         fflush(diags_log->m_fp);
@@ -700,6 +703,10 @@ Diags::should_roll_diagslog()
 bool
 Diags::should_roll_outputlog()
 {
+  // stdout_log and stderr_log should never be NULL as this point in time
+  ink_assert(stdout_log != NULL);
+  ink_assert(stderr_log != NULL);
+
   bool ret_val = false;
   bool need_consider_stderr = true;
 
@@ -713,15 +720,18 @@ Diags::should_roll_outputlog()
   */
 
   // Roll stdout_log if necessary
-  if (stdout_log && stdout_log->is_init()) {
+  if (stdout_log->is_init()) {
     if (outputlog_rolling_enabled == RollingEnabledValues::ROLL_ON_SIZE) {
+      // if we can't even check the file, we can forget about rotating
       struct stat buf;
-      fstat(fileno(stdout_log->m_fp), &buf);
+      if (fstat(fileno(stdout_log->m_fp), &buf) != 0)
+        return false;
+
       int size = buf.st_size;
       if (outputlog_rolling_size != -1 && size >= outputlog_rolling_size * BYTES_IN_MB)
{
         // since usually stdout and stderr are the same file on disk, we should just
         // play it safe and just flush both BaseLogFiles
-        if (stderr_log && stderr_log->is_init())
+        if (stderr_log->is_init())
           fflush(stderr_log->m_fp);
         fflush(stdout_log->m_fp);
 
@@ -746,7 +756,7 @@ Diags::should_roll_outputlog()
       if (outputlog_rolling_interval != -1 && (now - outputlog_time_last_roll) >=
outputlog_rolling_interval) {
         // since usually stdout and stderr are the same file on disk, we should just
         // play it safe and just flush both BaseLogFiles
-        if (stderr_log && stderr_log->is_init())
+        if (stderr_log->is_init())
           fflush(stderr_log->m_fp);
         fflush(stdout_log->m_fp);
 

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7bcc3b4f/proxy/Main.cc
----------------------------------------------------------------------
diff --git a/proxy/Main.cc b/proxy/Main.cc
index e7fd038..5ebb87a 100644
--- a/proxy/Main.cc
+++ b/proxy/Main.cc
@@ -308,7 +308,6 @@ class DiagsLogContinuation : public Continuation
 {
 public:
   DiagsLogContinuation() : Continuation(new_ProxyMutex()) { SET_HANDLER(&DiagsLogContinuation::periodic);
}
-
   int
   periodic(int /* event ATS_UNUSED */, Event * /* e ATS_UNUSED */)
   {

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7bcc3b4f/proxy/logging/Log.cc
----------------------------------------------------------------------
diff --git a/proxy/logging/Log.cc b/proxy/logging/Log.cc
index 8ed4394..dfd0bfd 100644
--- a/proxy/logging/Log.cc
+++ b/proxy/logging/Log.cc
@@ -1230,6 +1230,10 @@ Log::flush_thread_main(void * /* args ATS_UNUSED */)
         continue;
       }
 
+      int logfilefd = logfile->get_fd();
+      // This should always be true because we just checked it.
+      ink_assert(logfilefd >= 0);
+
       // write *all* data to target file as much as possible
       //
       while (total_bytes - bytes_written) {
@@ -1242,7 +1246,8 @@ Log::flush_thread_main(void * /* args ATS_UNUSED */)
           break;
         }
 
-        len = ::write(logfile->get_fd(), &buf[bytes_written], total_bytes - bytes_written);
+        len = ::write(logfilefd, &buf[bytes_written], total_bytes - bytes_written);
+
         if (len < 0) {
           Error("Failed to write log to %s: [tried %d, wrote %d, %s]", logfile->get_name(),
total_bytes - bytes_written,
                 bytes_written, strerror(errno));

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7bcc3b4f/proxy/logging/LogFile.cc
----------------------------------------------------------------------
diff --git a/proxy/logging/LogFile.cc b/proxy/logging/LogFile.cc
index c9c40ce..dce5f41 100644
--- a/proxy/logging/LogFile.cc
+++ b/proxy/logging/LogFile.cc
@@ -147,6 +147,9 @@ LogFile::change_header(const char *header)
 int
 LogFile::open_file()
 {
+  // whatever we want to open should have a name
+  ink_assert(m_name != NULL);
+
   // is_open() takes into account if we're using BaseLogFile or a naked fd
   if (is_open()) {
     return LOG_FILE_NO_ERROR;

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7bcc3b4f/proxy/shared/DiagsConfig.cc
----------------------------------------------------------------------
diff --git a/proxy/shared/DiagsConfig.cc b/proxy/shared/DiagsConfig.cc
index 87203a9..9219ce4 100644
--- a/proxy/shared/DiagsConfig.cc
+++ b/proxy/shared/DiagsConfig.cc
@@ -266,7 +266,7 @@ DiagsConfig::RegisterDiagConfig()
 }
 
 
-DiagsConfig::DiagsConfig(const char *filename, const char *tags, const char *actions, bool
use_records)
+DiagsConfig::DiagsConfig(const char *filename, const char *tags, const char *actions, bool
use_records) : diags_log(NULL)
 {
   char diags_logpath[PATH_NAME_MAX];
   ats_scoped_str logpath;
@@ -281,7 +281,7 @@ DiagsConfig::DiagsConfig(const char *filename, const char *tags, const
char *act
   ////////////////////////////////////////////////////////////////////
 
   if (!use_records) {
-    diags = new Diags(tags, actions, NULL);
+    diags = new Diags(tags, actions, diags_log);
     config_diags_norecords();
     return;
   }


Mime
View raw message