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 <[email protected]> Authored: Sun Dec 13 10:14:05 2015 -0800 Committer: James Peach <[email protected]> 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
