You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by dx...@apache.org on 2017/08/16 16:49:22 UTC
[trafficserver] branch master updated: Clean up Diags
This is an automated email from the ASF dual-hosted git repository.
dxu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new fddd7d8 Clean up Diags
fddd7d8 is described below
commit fddd7d83360482536e03b781e96714adcb42ed34
Author: Daniel Xu <dx...@dxuuu.xyz>
AuthorDate: Fri May 5 12:30:09 2017 -0500
Clean up Diags
Few cleanups:
1) Remove typedefs from some enums. typedef'ing enums stopped being
useful when C++ came to town.
2) De-duplicate the separate stdout and stderr fiddling functions. These
functions essentially do the same thing except on different objects. We
should not repeat code.
---
cmd/traffic_manager/traffic_manager.cc | 8 +-
lib/ts/Diags.cc | 161 ++++++++++++---------------------
lib/ts/Diags.h | 36 ++++----
proxy/Main.cc | 12 +--
4 files changed, 86 insertions(+), 131 deletions(-)
diff --git a/cmd/traffic_manager/traffic_manager.cc b/cmd/traffic_manager/traffic_manager.cc
index 7a6c98e..e47bb22 100644
--- a/cmd/traffic_manager/traffic_manager.cc
+++ b/cmd/traffic_manager/traffic_manager.cc
@@ -492,8 +492,8 @@ main(int argc, const char **argv)
// up the manager
diagsConfig = new DiagsConfig("Manager", DIAGS_LOG_FILENAME, debug_tags, action_tags, false);
diags = diagsConfig->diags;
- diags->set_stdout_output(bind_stdout);
- diags->set_stderr_output(bind_stderr);
+ diags->set_std_output(StdStream::STDOUT, bind_stdout);
+ diags->set_std_output(StdStream::STDERR, bind_stderr);
RecLocalInit();
LibRecordsConfigInit();
@@ -538,8 +538,8 @@ main(int argc, const char **argv)
diagsConfig = new DiagsConfig("Manager", DIAGS_LOG_FILENAME, debug_tags, action_tags, true);
diags = diagsConfig->diags;
RecSetDiags(diags);
- diags->set_stdout_output(bind_stdout);
- diags->set_stderr_output(bind_stderr);
+ diags->set_std_output(StdStream::STDOUT, bind_stdout);
+ diags->set_std_output(StdStream::STDERR, bind_stderr);
if (is_debug_tag_set("diags")) {
diags->dump();
diff --git a/lib/ts/Diags.cc b/lib/ts/Diags.cc
index b0c1c82..70095d1 100644
--- a/lib/ts/Diags.cc
+++ b/lib/ts/Diags.cc
@@ -743,13 +743,13 @@ Diags::should_roll_outputlog()
if (stdout_log->roll()) {
char *oldname = ats_strdup(stdout_log->get_name());
log_log_trace("in %s(), oldname=%s\n", __func__, oldname);
- set_stdout_output(oldname);
+ set_std_output(StdStream::STDOUT, oldname);
// if stderr and stdout are redirected to the same place, we should
// update the stderr_log object as well
if (!strcmp(oldname, stderr_log->get_name())) {
log_log_trace("oldname == stderr_log->get_name()\n");
- set_stderr_output(oldname);
+ set_std_output(StdStream::STDERR, oldname);
need_consider_stderr = false;
}
ats_free(oldname);
@@ -773,13 +773,13 @@ Diags::should_roll_outputlog()
outputlog_time_last_roll = now;
char *oldname = ats_strdup(stdout_log->get_name());
log_log_trace("in %s, oldname=%s\n", __func__, oldname);
- set_stdout_output(oldname);
+ set_std_output(StdStream::STDOUT, oldname);
// if stderr and stdout are redirected to the same place, we should
// update the stderr_log object as well
if (!strcmp(oldname, stderr_log->get_name())) {
log_log_trace("oldname == stderr_log->get_name()\n");
- set_stderr_output(oldname);
+ set_std_output(StdStream::STDERR, oldname);
need_consider_stderr = false;
}
ats_free(oldname);
@@ -806,105 +806,64 @@ Diags::should_roll_outputlog()
}
/*
- * Binds stdout to stdout_path, provided that stdout_path != "".
- * Also sets up a BaseLogFile for stdout.
+ * Sets up a BaseLogFile for the specified file. Then it binds the specified standard steam
+ * to the aforementioned BaseLogFile.
*
- * Returns true on binding and setup, false otherwise
- *
- * TODO make this a generic function (ie combine set_stdout_output and
- * set_stderr_output
+ * Returns true on successful binding and setup, false otherwise
*/
bool
-Diags::set_stdout_output(const char *stdout_path)
+Diags::set_std_output(StdStream stream, const char *file)
{
- if (strcmp(stdout_path, "") == 0) {
- return false;
- }
+ const char *target_stream;
+ BaseLogFile **current;
+ BaseLogFile *old_log, *new_log;
- BaseLogFile *old_stdout_log = stdout_log;
- BaseLogFile *new_stdout_log = new BaseLogFile(stdout_path);
-
- // on any errors we quit
- if (!new_stdout_log || new_stdout_log->open_file(output_logfile_perm) != BaseLogFile::LOG_FILE_NO_ERROR) {
- log_log_error("[Warning]: unable to open file=%s to bind stdout to\n", stdout_path);
- log_log_error("[Warning]: stdout is currently not bound to anything\n");
- delete new_stdout_log;
- lock();
- stdout_log = nullptr;
- unlock();
- return false;
- }
- if (!new_stdout_log->is_open()) {
- log_log_error("[Warning]: file pointer for stdout %s = nullptr\n", stdout_path);
- log_log_error("[Warning]: stdout is currently not bound to anything\n");
- delete new_stdout_log;
- lock();
- stdout_log = nullptr;
- unlock();
+ // If the caller is stupid, we give up
+ if (strcmp(file, "") == 0) {
return false;
}
- // now exchange the stdout_log pointer
- lock();
- stdout_log = new_stdout_log;
- bool ret = rebind_stdout(fileno(new_stdout_log->m_fp));
- unlock();
-
- if (old_stdout_log) {
- delete old_stdout_log;
- }
-
- // "this should never happen"^{TM}
- ink_release_assert(ret);
-
- return ret;
-}
-
-/*
- * Binds stderr to stderr_path, provided that stderr_path != "".
- * Also sets up a BaseLogFile for stderr.
- *
- * Returns true on binding and setup, false otherwise
- */
-bool
-Diags::set_stderr_output(const char *stderr_path)
-{
- if (strcmp(stderr_path, "") == 0) {
- return false;
- }
-
- BaseLogFile *old_stderr_log = stderr_log;
- BaseLogFile *new_stderr_log = new BaseLogFile(stderr_path);
-
- // on any errors we quit
- if (!new_stderr_log || new_stderr_log->open_file(output_logfile_perm) != BaseLogFile::LOG_FILE_NO_ERROR) {
- log_log_error("[Warning]: unable to open file=%s to bind stderr to\n", stderr_path);
- log_log_error("[Warning]: stderr is currently not bound to anything\n");
- delete new_stderr_log;
+ // Figure out which standard stream we want to redirect
+ if (stream == StdStream::STDOUT) {
+ target_stream = "stdout";
+ current = &stdout_log;
+ } else {
+ target_stream = "stderr";
+ current = &stderr_log;
+ }
+ (void)target_stream; // silence clang-analyzer for now
+ old_log = *current;
+ new_log = new BaseLogFile(file);
+
+ // On any errors we quit
+ if (!new_log || new_log->open_file(output_logfile_perm) != BaseLogFile::LOG_FILE_NO_ERROR) {
+ log_log_error("[Warning]: unable to open file=%s to bind %s to\n", file, target_stream);
+ log_log_error("[Warning]: %s is currently not bound to anything\n", target_stream);
+ delete new_log;
lock();
- stderr_log = nullptr;
+ *current = nullptr;
unlock();
return false;
}
- if (!new_stderr_log->is_open()) {
- log_log_error("[Warning]: file pointer for stderr %s = nullptr\n", stderr_path);
- log_log_error("[Warning]: stderr is currently not bound to anything\n");
- delete new_stderr_log;
+ if (!new_log->is_open()) {
+ log_log_error("[Warning]: file pointer for %s %s = nullptr\n", target_stream, file);
+ log_log_error("[Warning]: %s is currently not bound to anything\n", target_stream);
+ delete new_log;
lock();
- stderr_log = nullptr;
+ *current = nullptr;
unlock();
return false;
}
- // now exchange the stderr_log pointer
+ // Now exchange the pointer to the standard stream in question
lock();
- stderr_log = new_stderr_log;
- bool ret = rebind_stderr(fileno(stderr_log->m_fp));
+ *current = new_log;
+ bool ret = rebind_std_stream(stream, fileno(new_log->m_fp));
unlock();
- if (old_stderr_log) {
- delete old_stderr_log;
- }
+ // Free the BaseLogFile we rotated out
+ if (old_log)
+ delete old_log;
// "this should never happen"^{TM}
ink_release_assert(ret);
@@ -913,34 +872,30 @@ Diags::set_stderr_output(const char *stderr_path)
}
/*
- * Helper function that rebinds stdout to specified file descriptor
+ * Helper function that rebinds a specified stream to specified file descriptor
*
* Returns true on success, false otherwise
*/
bool
-Diags::rebind_stdout(int new_fd)
+Diags::rebind_std_stream(StdStream stream, int new_fd)
{
- if (new_fd < 0) {
- log_log_error("[Warning]: TS unable to bind stdout to new file descriptor=%d", new_fd);
+ const char *target_stream;
+ int stream_fd;
+
+ // Figure out which stream to dup2
+ if (stream == StdStream::STDOUT) {
+ target_stream = "stdout";
+ stream_fd = STDOUT_FILENO;
} else {
- dup2(new_fd, STDOUT_FILENO);
- return true;
+ target_stream = "stderr";
+ stream_fd = STDERR_FILENO;
}
- return false;
-}
+ (void)target_stream; // silence clang-analyzer for now
-/*
- * Helper function that rebinds stderr to specified file descriptor
- *
- * Returns true on success, false otherwise
- */
-bool
-Diags::rebind_stderr(int new_fd)
-{
- if (new_fd < 0) {
- log_log_error("[Warning]: TS unable to bind stderr to new file descriptor=%d", new_fd);
- } else {
- dup2(new_fd, STDERR_FILENO);
+ if (new_fd < 0)
+ log_log_error("[Warning]: TS unable to bind %s to new file descriptor=%d", target_stream, new_fd);
+ else {
+ dup2(new_fd, stream_fd);
return true;
}
return false;
diff --git a/lib/ts/Diags.h b/lib/ts/Diags.h
index f055d01..6fbea8f 100644
--- a/lib/ts/Diags.h
+++ b/lib/ts/Diags.h
@@ -49,10 +49,10 @@
class Diags;
// extern int diags_on_for_plugins;
-typedef enum {
+enum DiagsTagType {
DiagsTagType_Debug = 0, // do not renumber --- used as array index
DiagsTagType_Action = 1
-} DiagsTagType;
+};
struct DiagsModeOutput {
bool to_stdout;
@@ -61,18 +61,20 @@ struct DiagsModeOutput {
bool to_diagslog;
};
-typedef enum { // do not renumber --- used as array index
- DL_Diag = 0, // process does not die
- DL_Debug, // process does not die
- DL_Status, // process does not die
- DL_Note, // process does not die
- DL_Warning, // process does not die
- DL_Error, // process does not die
- DL_Fatal, // causes process termination
- DL_Alert, // causes process termination
- DL_Emergency, // causes process termination
- DL_Undefined // must be last, used for size!
-} DiagsLevel;
+enum DiagsLevel { // do not renumber --- used as array index
+ DL_Diag = 0, // process does not die
+ DL_Debug, // process does not die
+ DL_Status, // process does not die
+ DL_Note, // process does not die
+ DL_Warning, // process does not die
+ DL_Error, // process does not die
+ DL_Fatal, // causes process termination
+ DL_Alert, // causes process termination
+ DL_Emergency, // causes process termination
+ DL_Undefined // must be last, used for size!
+};
+
+enum StdStream { STDOUT = 0, STDERR };
enum RollingEnabledValues { NO_ROLLING = 0, ROLL_ON_TIME, ROLL_ON_SIZE, ROLL_ON_TIME_OR_SIZE, INVALID_ROLLING_VALUE };
@@ -215,8 +217,7 @@ public:
bool should_roll_diagslog();
bool should_roll_outputlog();
- bool set_stdout_output(const char *_bind_stdout);
- bool set_stderr_output(const char *_bind_stderr);
+ bool set_std_output(StdStream stream, const char *file);
const char *base_debug_tags; // internal copy of default debug tags
const char *base_action_tags; // internal copy of default action tags
@@ -240,8 +241,7 @@ private:
time_t outputlog_time_last_roll;
time_t diagslog_time_last_roll;
- bool rebind_stdout(int new_fd);
- bool rebind_stderr(int new_fd);
+ bool rebind_std_stream(StdStream stream, int new_fd);
void
lock() const
diff --git a/proxy/Main.cc b/proxy/Main.cc
index 92916d3..789d0f0 100644
--- a/proxy/Main.cc
+++ b/proxy/Main.cc
@@ -267,8 +267,8 @@ public:
Debug("log", "received SIGUSR2, reloading traffic.out");
// reload output logfile (file is usually called traffic.out)
- diags->set_stdout_output(bind_stdout);
- diags->set_stderr_output(bind_stderr);
+ diags->set_std_output(StdStream::STDOUT, bind_stdout);
+ diags->set_std_output(StdStream::STDERR, bind_stderr);
}
if (signal_received[SIGTERM] || signal_received[SIGINT]) {
@@ -1564,8 +1564,8 @@ main(int /* argc ATS_UNUSED */, const char **argv)
// related errors and if diagsConfig isn't get up yet that will crash on a NULL pointer.
diagsConfig = new DiagsConfig("Server", DIAGS_LOG_FILENAME, error_tags, action_tags, false);
diags = diagsConfig->diags;
- diags->set_stdout_output(bind_stdout);
- diags->set_stderr_output(bind_stderr);
+ diags->set_std_output(StdStream::STDOUT, bind_stdout);
+ diags->set_std_output(StdStream::STDERR, bind_stderr);
if (is_debug_tag_set("diags")) {
diags->dump();
}
@@ -1651,8 +1651,8 @@ main(int /* argc ATS_UNUSED */, const char **argv)
diagsConfig = new DiagsConfig("Server", DIAGS_LOG_FILENAME, error_tags, action_tags, true);
diags = diagsConfig->diags;
RecSetDiags(diags);
- diags->set_stdout_output(bind_stdout);
- diags->set_stderr_output(bind_stderr);
+ diags->set_std_output(StdStream::STDOUT, bind_stdout);
+ diags->set_std_output(StdStream::STDERR, bind_stderr);
if (is_debug_tag_set("diags")) {
diags->dump();
}
--
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>'].