You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by am...@apache.org on 2018/07/11 13:03:39 UTC

[trafficserver] branch master updated (7190c1c -> e10da22)

This is an automated email from the ASF dual-hosted git repository.

amc pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git.


    from 7190c1c  Plugin, makefile, readme.
     new c14fac1  traffic_manager: fix --tsArgs to work.
     new e10da22  traffic_manager: Cleanup handling of proxy args.

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 mgmt/LocalManager.cc                   | 56 +++++++++++++++++-----------------
 mgmt/LocalManager.h                    |  4 ++-
 mgmt/api/CoreAPI.cc                    |  2 +-
 src/traffic_manager/traffic_manager.cc | 24 ++++-----------
 4 files changed, 38 insertions(+), 48 deletions(-)


[trafficserver] 02/02: traffic_manager: Cleanup handling of proxy args.

Posted by am...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

amc pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git

commit e10da228172ac548ec7d497850c6a5e9d7b43114
Author: Alan M. Carroll <am...@apache.org>
AuthorDate: Fri Jun 29 11:24:08 2018 -0500

    traffic_manager: Cleanup handling of proxy args.
---
 mgmt/LocalManager.cc                   | 56 +++++++++++++++++-----------------
 mgmt/LocalManager.h                    |  4 ++-
 mgmt/api/CoreAPI.cc                    |  2 +-
 src/traffic_manager/traffic_manager.cc | 22 ++++---------
 4 files changed, 38 insertions(+), 46 deletions(-)

diff --git a/mgmt/LocalManager.cc b/mgmt/LocalManager.cc
index 907650c..b31d00f 100644
--- a/mgmt/LocalManager.cc
+++ b/mgmt/LocalManager.cc
@@ -32,12 +32,17 @@
 #include "ts/ink_cap.h"
 #include "FileManager.h"
 #include <string_view>
+#include <algorithm>
+#include <ts/TextView.h>
+#include <ts/BufferWriter.h>
+#include <ts/bwf_std_format.h>
 
 #if TS_USE_POSIX_CAP
 #include <sys/capability.h>
 #endif
 
-#define MGMT_OPT "-M"
+using namespace std::literals;
+static const std::string_view MGMT_OPT{"-M"};
 
 void
 LocalManager::mgmtCleanup()
@@ -253,7 +258,6 @@ LocalManager::~LocalManager()
   ats_free(absolute_proxy_binary);
   ats_free(proxy_name);
   ats_free(proxy_binary);
-  ats_free(proxy_options);
   ats_free(env_prep);
 }
 
@@ -883,52 +887,48 @@ LocalManager::startProxy(const char *onetime_options)
   } else {
     int i = 0;
     char *options[32], *last, *tok;
-    bool open_ports_p = false;
-    char real_proxy_options[OPTIONS_SIZE];
+    char options_buffer[OPTIONS_SIZE];
+    ts::FixedBufferWriter w{options_buffer, OPTIONS_SIZE};
 
-    ink_strlcpy(real_proxy_options, proxy_options, OPTIONS_SIZE);
-    if (onetime_options && *onetime_options) {
-      ink_strlcat(real_proxy_options, " ", OPTIONS_SIZE);
-      ink_strlcat(real_proxy_options, onetime_options, OPTIONS_SIZE);
-    }
+    w.clip(1);
+    w.print("{}{}", ts::bwf::OptionalAffix(proxy_options), ts::bwf::OptionalAffix(onetime_options));
 
     // Make sure we're starting the proxy in mgmt mode
-    if (strstr(real_proxy_options, MGMT_OPT) == nullptr) {
-      ink_strlcat(real_proxy_options, " ", OPTIONS_SIZE);
-      ink_strlcat(real_proxy_options, MGMT_OPT, OPTIONS_SIZE);
+    if (w.view().find(MGMT_OPT) == std::string_view::npos) {
+      w.write(MGMT_OPT);
+      w.write(' ');
     }
 
-    // Check if we need to pass down port/fd information to
-    // traffic_server by seeing if there are any open ports.
-    for (int i = 0, limit = m_proxy_ports.size(); !open_ports_p && i < limit; ++i) {
-      if (ts::NO_FD != m_proxy_ports[i].m_fd) {
-        open_ports_p = true;
-      }
-    }
-
-    if (open_ports_p) {
+    // Pass down port/fd information to traffic_server if there are any open ports.
+    if (std::any_of(m_proxy_ports.begin(), m_proxy_ports.end(), [](HttpProxyPort &p) { return ts::NO_FD != p.m_fd; })) {
       char portbuf[128];
       bool need_comma_p = false;
 
-      ink_strlcat(real_proxy_options, " --httpport ", OPTIONS_SIZE);
+      w.write("--httpport "sv);
       for (auto &p : m_proxy_ports) {
         if (ts::NO_FD != p.m_fd) {
           if (need_comma_p) {
-            ink_strlcat(real_proxy_options, ",", OPTIONS_SIZE);
+            w.write(',');
           }
           need_comma_p = true;
           p.print(portbuf, sizeof(portbuf));
-          ink_strlcat(real_proxy_options, (const char *)portbuf, OPTIONS_SIZE);
+          w.write(portbuf);
         }
       }
     }
 
-    Debug("lm", "[LocalManager::startProxy] Launching %s with options '%s'", absolute_proxy_binary, real_proxy_options);
+    w.extend(1);
+    w.write('\0'); // null terminate.
+
+    Debug("lm", "[LocalManager::startProxy] Launching %s '%s'", absolute_proxy_binary, w.data());
 
+    // Unfortunately the normally obnoxious null writing of strtok is in this case a required
+    // side effect and other alternatives are noticeably more clunky.
     ink_zero(options);
-    options[0]   = absolute_proxy_binary;
-    i            = 1;
-    tok          = strtok_r(real_proxy_options, " ", &last);
+    options[0] = absolute_proxy_binary;
+    i          = 1;
+    tok        = strtok_r(options_buffer, " ", &last);
+    Debug("lm", "opt %d = '%s'", i, tok);
     options[i++] = tok;
     while (i < 32 && (tok = strtok_r(nullptr, " ", &last))) {
       Debug("lm", "opt %d = '%s'", i, tok);
diff --git a/mgmt/LocalManager.h b/mgmt/LocalManager.h
index 4cc5257..4ff3591 100644
--- a/mgmt/LocalManager.h
+++ b/mgmt/LocalManager.h
@@ -33,6 +33,8 @@
 
 #pragma once
 
+#include <string>
+
 #include "Alarms.h"
 #include "BaseManager.h"
 #include <records/I_RecHttp.h>
@@ -119,7 +121,7 @@ public:
   char *absolute_proxy_binary;
   char *proxy_name;
   char *proxy_binary;
-  char *proxy_options = nullptr; // These options should persist across proxy reboots
+  std::string proxy_options; // These options should persist across proxy reboots
   char *env_prep;
 
   int process_server_sockfd = ts::NO_FD;
diff --git a/mgmt/api/CoreAPI.cc b/mgmt/api/CoreAPI.cc
index 1d8eb77..69a4201 100644
--- a/mgmt/api/CoreAPI.cc
+++ b/mgmt/api/CoreAPI.cc
@@ -183,7 +183,7 @@ ProxyStateSet(TSProxyStateT state, TSCacheClearT clear)
       ink_strlcat(tsArgs, " -k", sizeof(tsArgs));
     }
 
-    mgmt_log("[ProxyStateSet] Traffic Server Args: '%s %s'\n", lmgmt->proxy_options ? lmgmt->proxy_options : "", tsArgs);
+    mgmt_log("[ProxyStateSet] Traffic Server Args: '%s %s'\n", lmgmt->proxy_options.c_str(), tsArgs);
 
     lmgmt->run_proxy = true;
     lmgmt->listenForProxy();
diff --git a/src/traffic_manager/traffic_manager.cc b/src/traffic_manager/traffic_manager.cc
index 2b6a75b..eb78717 100644
--- a/src/traffic_manager/traffic_manager.cc
+++ b/src/traffic_manager/traffic_manager.cc
@@ -56,6 +56,7 @@
 #endif
 #include <grp.h>
 #include <atomic>
+#include <ts/bwf_std_format.h>
 
 #define FD_THROTTLE_HEADROOM (128 + 64) // TODO: consolidate with THROTTLE_FD_HEADROOM
 #define DIAGS_LOG_FILENAME "manager.log"
@@ -64,6 +65,8 @@
 #error "Need lock free std::atomic<int>"
 #endif
 
+using namespace std::literals;
+
 // These globals are still referenced directly by management API.
 LocalManager *lmgmt = nullptr;
 FileManager *configFiles;
@@ -643,22 +646,9 @@ main(int argc, const char **argv)
   // so we append the outputlog location to the persistent proxy options
   //
   // TS needs them to be able to create BaseLogFiles for each value
-  TextBuffer args(1024);
-
-  if (*bind_stdout) {
-    const char *space = args.empty() ? "" : " ";
-    args.format("%s%s %s", space, "--" TM_OPT_BIND_STDOUT, bind_stdout);
-  }
-
-  if (*bind_stderr) {
-    const char *space = args.empty() ? "" : " ";
-    args.format("%s%s %s", space, "--" TM_OPT_BIND_STDERR, bind_stderr);
-  }
-  if (tsArgs) {
-    args.format("%s", tsArgs);
-  }
-
-  lmgmt->proxy_options = args.release();
+  ts::bwprint(lmgmt->proxy_options, "{}{}{}", ts::bwf::OptionalAffix(tsArgs),
+              ts::bwf::OptionalAffix(bind_stdout, " "sv, "--bind_stdout "sv),
+              ts::bwf::OptionalAffix(bind_stderr, " "sv, "--bind_stderr "sv));
 
   if (proxy_port) {
     HttpProxyPort::loadValue(lmgmt->m_proxy_ports, proxy_port);


[trafficserver] 01/02: traffic_manager: fix --tsArgs to work.

Posted by am...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

amc pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git

commit c14fac1ceda81c45bbc5fc70dd2440a2e68e6764
Author: Alan M. Carroll <am...@apache.org>
AuthorDate: Sat Jul 7 10:49:56 2018 -0500

    traffic_manager: fix --tsArgs to work.
---
 src/traffic_manager/traffic_manager.cc | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/traffic_manager/traffic_manager.cc b/src/traffic_manager/traffic_manager.cc
index e25a0b9..2b6a75b 100644
--- a/src/traffic_manager/traffic_manager.cc
+++ b/src/traffic_manager/traffic_manager.cc
@@ -639,11 +639,6 @@ main(int argc, const char **argv)
   // stat the 'sync_thr' until 'configFiles' has been initialized.
   RecLocalStart(configFiles);
 
-  /* Update cmd line overrides/environmental overrides/etc */
-  if (tsArgs) { /* Passed command line args for proxy */
-    lmgmt->proxy_options = ats_strdup(tsArgs);
-  }
-
   // TS needs to be started up with the same outputlog bindings each time,
   // so we append the outputlog location to the persistent proxy options
   //
@@ -659,6 +654,9 @@ main(int argc, const char **argv)
     const char *space = args.empty() ? "" : " ";
     args.format("%s%s %s", space, "--" TM_OPT_BIND_STDERR, bind_stderr);
   }
+  if (tsArgs) {
+    args.format("%s", tsArgs);
+  }
 
   lmgmt->proxy_options = args.release();