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 <[email protected]> Authored: Wed Nov 4 00:16:03 2015 -0600 Committer: Alan M. Carroll <[email protected]> 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; }
