You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by jp...@apache.org on 2015/12/13 19:17:27 UTC
trafficserver git commit: TS-4054: refactor and clarify diags_log
initialization
Repository: trafficserver
Updated Branches:
refs/heads/master 88c35d77a -> 805ba78e4
TS-4054: refactor and clarify diags_log initialization
Refactor `setup_diagslog()`. Make `setup_diagslog()` return a bool
of whether or not the setup of `blf` was successful or not. Changed
from `setup_diagslog()` actually setting the value for `diags_log`
because this way it is more clear where `diags_log` should be getting
free'd.
This closes #363.
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/805ba78e
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/805ba78e
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/805ba78e
Branch: refs/heads/master
Commit: 805ba78e497a6d04fea775a208d14047567d2707
Parents: 88c35d7
Author: Daniel Xu <dl...@yahoo.com>
Authored: Sun Dec 13 10:14:05 2015 -0800
Committer: James Peach <jp...@apache.org>
Committed: Sun Dec 13 10:14:05 2015 -0800
----------------------------------------------------------------------
lib/ts/Diags.cc | 44 +++++++++++++++++++++++++++-----------------
lib/ts/Diags.h | 3 ---
2 files changed, 27 insertions(+), 20 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/805ba78e/lib/ts/Diags.cc
----------------------------------------------------------------------
diff --git a/lib/ts/Diags.cc b/lib/ts/Diags.cc
index d84cc65..518b024 100644
--- a/lib/ts/Diags.cc
+++ b/lib/ts/Diags.cc
@@ -50,6 +50,8 @@ bool DiagsConfigState::enabled[2] = {false, false};
// Global, used for all diagnostics
inkcoreapi Diags *diags = NULL;
+static bool setup_diagslog(BaseLogFile *blf);
+
template <int Size>
static void
vprintline(FILE *fp, char(&buffer)[Size], va_list ap)
@@ -154,7 +156,9 @@ Diags::Diags(const char *bdt, const char *bat, BaseLogFile *_diags_log)
stdout_log->open_file(); // should never fail
stderr_log->open_file(); // should never fail
- setup_diagslog(_diags_log);
+ if (setup_diagslog(_diags_log)) {
+ diags_log = _diags_log;
+ }
//////////////////////////////////////////////////////////////////
// start off with empty tag tables, will build in reconfigure() //
@@ -586,22 +590,22 @@ Diags::error_va(DiagsLevel level, const char *file, const char *func, const int
/*
* Sets up and error handles the given BaseLogFile object to work
- * with this instance of Diags
+ * with this instance of Diags.
+ *
+ * Returns true on success, false otherwise
*/
-void
-Diags::setup_diagslog(BaseLogFile *blf)
+static bool
+setup_diagslog(BaseLogFile *blf)
{
- ink_assert(diags_log == NULL);
-
- if (blf != NULL && blf->open_file() != BaseLogFile::LOG_FILE_NO_ERROR) {
- delete blf;
-
- log_log_error("Could not open diags log file: %s\n", strerror(errno));
- return;
+ if (blf != NULL) {
+ if (blf->open_file() != BaseLogFile::LOG_FILE_NO_ERROR) {
+ log_log_error("Could not open diags log file: %s\n", strerror(errno));
+ delete blf;
+ return false;
+ }
}
- diags_log = blf;
- log_log_trace("Exiting setup_diagslog, name=%s, this=%p\n", blf->get_name(), this);
+ return true;
}
void
@@ -658,8 +662,11 @@ Diags::should_roll_diagslog()
if (diags_log->roll()) {
char *oldname = ats_strdup(diags_log->get_name());
log_log_trace("in should_roll_logs() for diags.log, oldname=%s\n", oldname);
- delete diags_log;
- setup_diagslog(new BaseLogFile(oldname));
+ BaseLogFile *n = new BaseLogFile(oldname);
+ if (setup_diagslog(n)) {
+ delete diags_log;
+ diags_log = n;
+ }
ats_free(oldname);
ret_val = true;
}
@@ -672,8 +679,11 @@ Diags::should_roll_diagslog()
diagslog_time_last_roll = now;
char *oldname = ats_strdup(diags_log->get_name());
log_log_trace("in should_roll_logs() for diags.log, oldname=%s\n", oldname);
- delete diags_log;
- setup_diagslog(new BaseLogFile(oldname));
+ BaseLogFile *n = new BaseLogFile(oldname);
+ if (setup_diagslog(n)) {
+ delete diags_log;
+ diags_log = n;
+ }
ats_free(oldname);
ret_val = true;
}
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/805ba78e/lib/ts/Diags.h
----------------------------------------------------------------------
diff --git a/lib/ts/Diags.h b/lib/ts/Diags.h
index 0fefeff..7f3210b 100644
--- a/lib/ts/Diags.h
+++ b/lib/ts/Diags.h
@@ -115,9 +115,7 @@ public:
}
SrcLoc(const SrcLoc &rhs) : file(rhs.file), func(rhs.func), line(rhs.line) {}
-
SrcLoc(const char *_file, const char *_func, int _line) : file(_file), func(_func), line(_line) {}
-
SrcLoc &operator=(const SrcLoc &rhs)
{
this->file = rhs.file;
@@ -272,7 +270,6 @@ private:
time_t outputlog_time_last_roll;
time_t diagslog_time_last_roll;
- void setup_diagslog(BaseLogFile *blf);
bool rebind_stdout(int new_fd);
bool rebind_stderr(int new_fd);
void