[GitHub] trafficserver pull request #1073: TS-4399 TS-4400 Management API messes up p...

[jpeach]

Github user jpeach closed the pull request at:

https://github.com/apache/trafficserver/pull/1073


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1073: TS-4399 TS-4400 Management API messes up p...

[SolidWallOfCode]

Github user SolidWallOfCode commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1073#discussion_r86604224
  
--- Diff: mgmt/LocalManager.cc ---
@@ -902,8 +906,13 @@ LocalManager::startProxy()
 Vec real_proxy_options;
 
 real_proxy_options.append(proxy_options, strlen(proxy_options));
+if (onetime_options && *onetime_options) {
+  real_proxy_options.append(" ", strlen(" "));
+  real_proxy_options.append(onetime_options, strlen(onetime_options));
+}
 
-if (!strstr(proxy_options, MGMT_OPT)) { // Make sure we're starting 
the proxy in mgmt mode
+// Make sure we're starting the proxy in mgmt mode
+if (strstr(proxy_options, MGMT_OPT) == 0 && strstr(onetime_options, 
MGMT_OPT) == 0) {
--- End diff --

Couldn't you just check `real_proxy_options` here? If `onetime_options` 
aren't already in there, they won't be added anyway.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1073: TS-4399 TS-4400 Management API messes up p...

[jpeach]

Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1073#discussion_r85870712
  
--- Diff: mgmt/LocalManager.cc ---
@@ -189,6 +189,7 @@ LocalManager::LocalManager(bool proxy_on) : 
BaseManager(), run_proxy(proxy_on),
   proxy_launch_outstanding  = false;
   mgmt_shutdown_outstanding = MGMT_PENDING_NONE;
   proxy_running = 0;
+  coreapi_sleep = true;
--- End diff --

As mentioned in the other comment, I think we can get away without this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1073: TS-4399 TS-4400 Management API messes up p...

[jpeach]

Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1073#discussion_r85870674
  
--- Diff: mgmt/LocalManager.cc ---
@@ -902,8 +907,13 @@ LocalManager::startProxy()
 Vec real_proxy_options;
 
 real_proxy_options.append(proxy_options, strlen(proxy_options));
+if (onetime_options && *onetime_options) {
+  real_proxy_options.append(" ", strlen(" "));
+  real_proxy_options.append(onetime_options, strlen(onetime_options));
+}
 
-if (!strstr(proxy_options, MGMT_OPT)) { // Make sure we're starting 
the proxy in mgmt mode
+// Make sure we're starting the proxy in mgmt mode
+if (!strstr(proxy_options, MGMT_OPT) && !strstr(onetime_options, 
MGMT_OPT)) {
--- End diff --

Prefer ``strstr(...) == 0``. It's just that little bit more readable.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1073: TS-4399 TS-4400 Management API messes up p...

[jpeach]

Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1073#discussion_r85870603
  
--- Diff: mgmt/api/CoreAPI.cc ---
@@ -184,21 +184,19 @@ ProxyStateSet(TSProxyStateT state, TSCacheClearT 
clear)
   ink_strlcat(tsArgs, " -k", sizeof(tsArgs));
 }
 
-if (strlen(tsArgs) > 0) { /* Passed command line args for proxy */
-  ats_free(lmgmt->proxy_options);
-  lmgmt->proxy_options = ats_strdup(tsArgs);
-  mgmt_log("[ProxyStateSet] Traffic Server Args: '%s'\n", 
lmgmt->proxy_options);
-}
+mgmt_log("[ProxyStateSet] Traffic Server Args: '%s %s'\n", 
lmgmt->proxy_options ? lmgmt->proxy_options : "", tsArgs);
 
 lmgmt->run_proxy = true;
 lmgmt->listenForProxy();
+lmgmt->startProxy(tsArgs);
--- End diff --

Do you actually need to do the sleeping stuff here? Since you now call 
``LocalManager:: startProxy()`` directly, you have the return value to know 
that it succeeded. I don't think that the contract for this API needs to 
include waiting for a message.

```C
return lmgmt->startProxy(tsArgs) ? TS_ERR_OKAY : TS_ERR_FAIL;
```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1073: TS-4399 TS-4400 Management API messes up p...

[jpeach]

Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1073#discussion_r84208455
  
--- Diff: mgmt/api/CoreAPI.cc ---
@@ -41,6 +41,7 @@
 #include "ts/Diags.h"
 #include "ts/ink_hash_table.h"
 #include "ExpandingArray.h"
+#include 
--- End diff --

Remove this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1073: TS-4399 TS-4400 Management API messes up p...

[jpeach]

Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1073#discussion_r84208247
  
--- Diff: cmd/traffic_manager/traffic_manager.cc ---
@@ -844,6 +852,11 @@ main(int argc, const char **argv)
 }
   }
 
+  // Delete the proxy options we saved for CoreAPI.
+  // This probably isn't needed b/c the process dies here...
+  for (auto it = callback_proxy_options.begin(); it < 
callback_proxy_options.end(); ++it)
+free(*it);
+
--- End diff --

Remove this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1073: TS-4399 TS-4400 Management API messes up p...

[jpeach]

Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1073#discussion_r84208632
  
--- Diff: mgmt/LocalManager.cc ---
@@ -902,8 +907,11 @@ LocalManager::startProxy()
 Vec real_proxy_options;
 
 real_proxy_options.append(proxy_options, strlen(proxy_options));
+real_proxy_options.append(" ", strlen(" "));
+real_proxy_options.append(onetime_options, strlen(onetime_options));
--- End diff --

Let's allow ``onetime_options`` to be NULL, so
```C
if (onetime_options && *onetime_options) {
  ...
}
```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1073: TS-4399 TS-4400 Management API messes up p...

[jpeach]

Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1073#discussion_r84208118
  
--- Diff: cmd/traffic_manager/traffic_manager.cc ---
@@ -611,31 +614,35 @@ main(int argc, const char **argv)
 
   /* Update cmd line overrides/environmental overrides/etc */
   if (tsArgs) { /* Passed command line args for proxy */
+char* new_proxy_opts = new char[strlen(lmgmt->proxy_options) + 
strlen(tsArgs) + 1];
+strcpy(new_proxy_opts, lmgmt->proxy_options);
+strcat(new_proxy_opts, tsArgs);
 ats_free(lmgmt->proxy_options);
-lmgmt->proxy_options = tsArgs;
-mgmt_log("[main] Traffic Server Args: '%s'\n", lmgmt->proxy_options);
+lmgmt->proxy_options = new_proxy_opts;
   }
 
-  // we must pass in bind_stdout and bind_stderr values to TS
-  // we do it so TS is able to create BaseLogFiles for each value
+  // TS needs to be started up with the same outputlog bindings each time,
+  // so we append the outputlog location to the persistent proxy options
+  //
+  // TS needs them to be able to create BaseLogFiles for each value
   if (*bind_stdout != 0) {
-size_t l = strlen(lmgmt->proxy_options);
-size_t n = 3/* " --" */
-   + sizeof(TM_OPT_BIND_STDOUT) /* nul accounted for here */
-   + 1  /* space */
-   + strlen(bind_stdout);
-lmgmt->proxy_options = static_cast(ats_realloc(lmgmt->proxy_options, n + l));
-snprintf(lmgmt->proxy_options + l, n, " --%s %s", TM_OPT_BIND_STDOUT, 
bind_stdout);
+const char *bind_stdout_opt = " --" TM_OPT_BIND_STDOUT " ";
+char *new_proxy_opts = new char[strlen(lmgmt->proxy_options) + 
strlen(bind_stdout_opt) + strlen(bind_stdout) + 1];
+strcpy(new_proxy_opts, lmgmt->proxy_options);
+strcat(new_proxy_opts, bind_stdout_opt);
+strcat(new_proxy_opts, bind_stdout);
+delete lmgmt->proxy_options;
--- End diff --

You can replace all this string code with ``TextBuffer``.

```C
TextBuffer args;

if (*bind_stdout) {
   const char * space = args.empty() ? "" : " ";
   args.format("%s%s", space, "--" TM_OPT_BIND_STDOUT);
}

...

lmgmt->proxy_options = args.release();
```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1073: TS-4399 TS-4400 Management API messes up p...

[jpeach]

Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1073#discussion_r84208555
  
--- Diff: mgmt/LocalManager.h ---
@@ -74,7 +74,7 @@ class LocalManager : public BaseManager
   void signalAlarm(int alarm_id, const char *desc = NULL, const char *ip = 
NULL);
 
   void processEventQueue();
-  bool startProxy();
+  bool startProxy(char *onetime_options);
--- End diff --

``const char *``.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1073: TS-4399 TS-4400 Management API messes up p...

[jpeach]

Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1073#discussion_r84208567
  
--- Diff: mgmt/LocalManager.cc ---
@@ -831,9 +832,13 @@ LocalManager::processEventQueue()
 /*
  * startProxy()
  *   Function fires up a proxy process.
+ *
+ * Args:
+ *   onetime_options: one time options that traffic_server should be 
started with (ie
+ *these options do not persist across reboots)
  */
 bool
-LocalManager::startProxy()
+LocalManager::startProxy(char *onetime_options)
--- End diff --

``const char *``


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1073: TS-4399 TS-4400 Management API messes up p...

[jpeach]

Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1073#discussion_r84208323
  
--- Diff: mgmt/LocalManager.cc ---
@@ -239,8 +239,9 @@ LocalManager::LocalManager(bool proxy_on) : 
BaseManager(), run_proxy(proxy_on),
   process_server_timeout_msecs = 
REC_readInteger("proxy.config.lm.pserver_timeout_msecs", );
   proxy_name   = REC_readString("proxy.config.proxy_name", 
);
   proxy_binary = 
REC_readString("proxy.config.proxy_binary", );
-  proxy_options= 
REC_readString("proxy.config.proxy_binary_opts", );
   env_prep = REC_readString("proxy.config.env_prep", 
);
+  proxy_options= new char[1];
+  strcpy(proxy_options, "");  // so later we can always `delete 
proxy_options` without worrying
--- End diff --

You don't need this. It is safe for ``proxy_options`` to be NULL.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1073: TS-4399 TS-4400 Management API messes up p...

[jpeach]

Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1073#discussion_r84208107
  
--- Diff: cmd/traffic_manager/traffic_manager.cc ---
@@ -445,6 +446,8 @@ main(int argc, const char **argv)
   int binding_version  = 0;
   BindingInstance *binding = NULL;
 
+  std::vector callback_proxy_options;
--- End diff --

Remove this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1073: TS-4399 TS-4400 Management API messes up p...

[jpeach]

Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1073#discussion_r84208189
  
--- Diff: cmd/traffic_manager/traffic_manager.cc ---
@@ -611,31 +614,35 @@ main(int argc, const char **argv)
 
   /* Update cmd line overrides/environmental overrides/etc */
   if (tsArgs) { /* Passed command line args for proxy */
+char* new_proxy_opts = new char[strlen(lmgmt->proxy_options) + 
strlen(tsArgs) + 1];
+strcpy(new_proxy_opts, lmgmt->proxy_options);
+strcat(new_proxy_opts, tsArgs);
 ats_free(lmgmt->proxy_options);
-lmgmt->proxy_options = tsArgs;
-mgmt_log("[main] Traffic Server Args: '%s'\n", lmgmt->proxy_options);
+lmgmt->proxy_options = new_proxy_opts;
--- End diff --

At this point ``lmgmt->proxy_options`` isn't set, so just copy ``tsArgs`` 
into the text buffer if it is present.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1073: TS-4399 TS-4400 Management API messes up p...

[jpeach]

Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1073#discussion_r84208102
  
--- Diff: cmd/traffic_manager/traffic_manager.cc ---
@@ -27,6 +27,7 @@
 #include "ts/ink_sock.h"
 #include "ts/ink_args.h"
 #include "ts/ink_syslog.h"
+#include 
--- End diff --

Remove this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1073: TS-4399 TS-4400 Management API messes up p...

[jpeach]

Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1073#discussion_r84208238
  
--- Diff: cmd/traffic_manager/traffic_manager.cc ---
@@ -611,31 +614,35 @@ main(int argc, const char **argv)
 
   /* Update cmd line overrides/environmental overrides/etc */
   if (tsArgs) { /* Passed command line args for proxy */
+char* new_proxy_opts = new char[strlen(lmgmt->proxy_options) + 
strlen(tsArgs) + 1];
+strcpy(new_proxy_opts, lmgmt->proxy_options);
+strcat(new_proxy_opts, tsArgs);
 ats_free(lmgmt->proxy_options);
-lmgmt->proxy_options = tsArgs;
-mgmt_log("[main] Traffic Server Args: '%s'\n", lmgmt->proxy_options);
+lmgmt->proxy_options = new_proxy_opts;
   }
 
-  // we must pass in bind_stdout and bind_stderr values to TS
-  // we do it so TS is able to create BaseLogFiles for each value
+  // TS needs to be started up with the same outputlog bindings each time,
+  // so we append the outputlog location to the persistent proxy options
+  //
+  // TS needs them to be able to create BaseLogFiles for each value
   if (*bind_stdout != 0) {
-size_t l = strlen(lmgmt->proxy_options);
-size_t n = 3/* " --" */
-   + sizeof(TM_OPT_BIND_STDOUT) /* nul accounted for here */
-   + 1  /* space */
-   + strlen(bind_stdout);
-lmgmt->proxy_options = static_cast(ats_realloc(lmgmt->proxy_options, n + l));
-snprintf(lmgmt->proxy_options + l, n, " --%s %s", TM_OPT_BIND_STDOUT, 
bind_stdout);
+const char *bind_stdout_opt = " --" TM_OPT_BIND_STDOUT " ";
+char *new_proxy_opts = new char[strlen(lmgmt->proxy_options) + 
strlen(bind_stdout_opt) + strlen(bind_stdout) + 1];
+strcpy(new_proxy_opts, lmgmt->proxy_options);
+strcat(new_proxy_opts, bind_stdout_opt);
+strcat(new_proxy_opts, bind_stdout);
+delete lmgmt->proxy_options;
+lmgmt->proxy_options = new_proxy_opts;
   }
 
   if (*bind_stderr != 0) {
-size_t l = strlen(lmgmt->proxy_options);
-size_t n = 3/* space dash dash */
-   + sizeof(TM_OPT_BIND_STDERR) /* nul accounted for here */
-   + 1  /* space */
-   + strlen(bind_stderr);
-lmgmt->proxy_options = static_cast(ats_realloc(lmgmt->proxy_options, n + l));
-snprintf(lmgmt->proxy_options + l, n, " --%s %s", TM_OPT_BIND_STDERR, 
bind_stderr);
+const char *bind_stderr_opt = " --" TM_OPT_BIND_STDERR " ";
+char *new_proxy_opts = new char[strlen(lmgmt->proxy_options) + 
strlen(bind_stderr_opt) + strlen(bind_stderr) + 1];
+strcpy(new_proxy_opts, lmgmt->proxy_options);
+strcat(new_proxy_opts, bind_stderr_opt);
+strcat(new_proxy_opts, bind_stderr);
+delete lmgmt->proxy_options;
--- End diff --

You should not mix new/delete and malloc/free here. But this problem goes 
away with the text buffer.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1073: TS-4399 TS-4400 Management API messes up p...

[jpeach]

Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1073#discussion_r84208742
  
--- Diff: cmd/traffic_manager/traffic_manager.cc ---
@@ -835,10 +824,11 @@ main(int argc, const char **argv)
   } else {
 sleep_time = 1;
   }
-  if (lmgmt->startProxy()) {
+  if (ProxyStateSet(TS_PROXY_ON, TS_CACHE_CLEAR_NONE) == TS_ERR_OKAY) {
 just_started = 0;
 sleep_time   = 0;
   } else {
+mgmt_log("in ProxyStateSet else branch");
--- End diff --

Yes please :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1073: TS-4399 TS-4400 Management API messes up p...

[jpeach]

Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1073#discussion_r84208541
  
--- Diff: mgmt/api/CoreAPI.cc ---
@@ -53,6 +54,9 @@
 // global variable
 CallbackTable *local_event_callbacks;
 
+// Used to get any proxy options that traffic_manager might want to tack on
+std::function&)> proxy_options_callback;
--- End diff --

Remove this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1073: TS-4399 TS-4400 Management API messes up p...

[jpeach]

Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1073#discussion_r83994912
  
--- Diff: cmd/traffic_manager/traffic_manager.cc ---
@@ -610,32 +616,37 @@ main(int argc, const char **argv)
   RecLocalStart(configFiles);
 
   /* Update cmd line overrides/environmental overrides/etc */
-  if (tsArgs) { /* Passed command line args for proxy */
-ats_free(lmgmt->proxy_options);
-lmgmt->proxy_options = tsArgs;
-mgmt_log("[main] Traffic Server Args: '%s'\n", lmgmt->proxy_options);
-  }
+  if (tsArgs)  /* Passed command line args for proxy */
+callback_proxy_options.push_back(ats_strdup(tsArgs));
 
-  // we must pass in bind_stdout and bind_stderr values to TS
-  // we do it so TS is able to create BaseLogFiles for each value
+  // we must give bind_stdout and bind_stderr values to 
callback_proxy_options
+  // so LocalManager can start up TS with the correct options
+  //
+  // TS needs them to be able to create BaseLogFiles for each value
   if (*bind_stdout != 0) {
-size_t l = strlen(lmgmt->proxy_options);
-size_t n = 3/* " --" */
-   + sizeof(TM_OPT_BIND_STDOUT) /* nul accounted for here */
-   + 1  /* space */
-   + strlen(bind_stdout);
-lmgmt->proxy_options = static_cast(ats_realloc(lmgmt->proxy_options, n + l));
-snprintf(lmgmt->proxy_options + l, n, " --%s %s", TM_OPT_BIND_STDOUT, 
bind_stdout);
+char *bind_stdout_opt = new char[strlen("--") + 
strlen(TM_OPT_BIND_STDOUT)];
+strcpy(bind_stdout_opt, "--");
+strcat(bind_stdout_opt, TM_OPT_BIND_STDOUT);
+callback_proxy_options.push_back(bind_stdout_opt);
+callback_proxy_options.push_back(ats_strdup(bind_stdout));
   }
 
   if (*bind_stderr != 0) {
-size_t l = strlen(lmgmt->proxy_options);
-size_t n = 3/* space dash dash */
-   + sizeof(TM_OPT_BIND_STDERR) /* nul accounted for here */
-   + 1  /* space */
-   + strlen(bind_stderr);
-lmgmt->proxy_options = static_cast(ats_realloc(lmgmt->proxy_options, n + l));
-snprintf(lmgmt->proxy_options + l, n, " --%s %s", TM_OPT_BIND_STDERR, 
bind_stderr);
+char *bind_stderr_opt = new char[strlen("--") + 
strlen(TM_OPT_BIND_STDERR)];
+strcpy(bind_stderr_opt, "--");
+strcat(bind_stderr_opt, TM_OPT_BIND_STDERR);
--- End diff --

```C
foo.push_back("--" TM_OPT_BIND_STDERR")


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1073: TS-4399 TS-4400 Management API messes up p...

[danobi]

Github user danobi commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/1073#discussion_r83509798
  
--- Diff: cmd/traffic_manager/traffic_manager.cc ---
@@ -835,10 +824,11 @@ main(int argc, const char **argv)
   } else {
 sleep_time = 1;
   }
-  if (lmgmt->startProxy()) {
+  if (ProxyStateSet(TS_PROXY_ON, TS_CACHE_CLEAR_NONE) == TS_ERR_OKAY) {
 just_started = 0;
 sleep_time   = 0;
   } else {
+mgmt_log("in ProxyStateSet else branch");
--- End diff --

Note to self: remove this line


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] trafficserver pull request #1073: TS-4399 TS-4400 Management API messes up p...

[danobi]

GitHub user danobi opened a pull request:

https://github.com/apache/trafficserver/pull/1073

TS-4399 TS-4400 Management API messes up proxy options

TS-4399: Management API breaks diagnostic log rotation
TS-4400: TSProxyStateSet persist cache clearing across restart

The two issues are related in that they both deal with the
management API not correctly handling proxy flags.

For TS-4399, it was because the management API was not aware
of traffic_manager setting extra proxy options. This was fixed
by providing CoreAPI a callback to get extra proxy options from
traffic_manager.

For TS-4400, it was because the management API was not properly
clearing optional flags between proxy reboots. This was fixed
by resetting the proxy options before each reboot.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/danobi/trafficserver TS-4399_TS-4400

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/trafficserver/pull/1073.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1073


commit 9f28180ae4bebfb529a8a7f3bf6758cb44a3c57a
Author: Daniel Xu 
Date:   2016-10-03T19:05:57Z

TS-4399 TS-4400 Management API messes up proxy options

TS-4399: Management API breaks diagnostic log rotation
TS-4400: TSProxyStateSet persist cache clearing across restart

The two issues are related in that they both deal with the
management API not correctly handling proxy flags.

For TS-4399, it was because the management API was not aware
of traffic_manager setting extra proxy options. This was fixed
by providing CoreAPI a callback to get extra proxy options from
traffic_manager.

For TS-4400, it was because the management API was not properly
clearing optional flags between proxy reboots. This was fixed
by resetting the proxy options before each reboot.

commit 3365b2048e7bf0660faccf573ffd198857d03d25
Author: Daniel Xu 
Date:   2016-10-03T20:14:24Z

Fix memory leaks




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---