You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2020/12/01 14:38:47 UTC
[GitHub] [trafficserver] brbzull0 opened a new pull request #7364: traffic_ctl - plugin msg now require only the tag as mandatory field data field is now optional.
brbzull0 opened a new pull request #7364:
URL: https://github.com/apache/trafficserver/pull/7364
`traffic_ctl` can send a plugin message, which has a tag and a body. The CLI requires a tag and a body, but in some cases the body is irrelevant. However the CLI will fail if there’s no body. This change make the body/data optional, if not passed then the message handler will get data size as 0 and the data as NULL.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] brbzull0 commented on a change in pull request #7364: traffic_ctl - plugin msg now require only the tag as mandatory field data field is now optional.
Posted by GitBox <gi...@apache.org>.
brbzull0 commented on a change in pull request #7364:
URL: https://github.com/apache/trafficserver/pull/7364#discussion_r534404504
##########
File path: plugins/experimental/memory_profile/memory_profile.cc
##########
@@ -49,26 +49,28 @@ CallbackHandler(TSCont cont, TSEvent id, void *data)
TSDebug(PLUGIN_NAME, "Message to '%s' - %zu bytes of data", msg->tag, msg->data_size);
if (strcmp(PLUGIN_NAME, msg->tag) == 0) { // Message is for us
#if TS_HAS_JEMALLOC
- int retval = 0;
- if (strncmp((char *)msg->data, "dump", msg->data_size) == 0) {
- if ((retval = mallctl("prof.dump", nullptr, nullptr, nullptr, 0)) != 0) {
- TSError("mallct(prof.dump) failed retval=%d errno=%d", retval, errno);
+ if (msg->data_size) {
Review comment:
strncmp:
> The behavior is undefined when either lhs or rhs is the null pointer.
Not sure if the size = 0 will stop the function to check the ptr. Probably is safe this way.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] brbzull0 commented on a change in pull request #7364: traffic_ctl - plugin msg now require only the tag as mandatory field data field is now optional.
Posted by GitBox <gi...@apache.org>.
brbzull0 commented on a change in pull request #7364:
URL: https://github.com/apache/trafficserver/pull/7364#discussion_r534404799
##########
File path: example/plugins/c-api/lifecycle_plugin/lifecycle_plugin.c
##########
@@ -49,6 +49,10 @@ CallbackHandler(TSCont this, TSEvent id, void *data)
case TS_EVENT_LIFECYCLE_MSG: {
TSPluginMsg *msg = (TSPluginMsg *)data;
TSDebug(PLUGIN_NAME, "Message to '%s' - %zu bytes of data", msg->tag, msg->data_size);
+ if (msg->data_size == 0) {
Review comment:
yes, just want to make a point as this is an example plugin.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] SolidWallOfCode commented on a change in pull request #7364: traffic_ctl - plugin msg now require only the tag as mandatory field data field is now optional.
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on a change in pull request #7364:
URL: https://github.com/apache/trafficserver/pull/7364#discussion_r534284356
##########
File path: example/plugins/c-api/lifecycle_plugin/lifecycle_plugin.c
##########
@@ -49,6 +49,10 @@ CallbackHandler(TSCont this, TSEvent id, void *data)
case TS_EVENT_LIFECYCLE_MSG: {
TSPluginMsg *msg = (TSPluginMsg *)data;
TSDebug(PLUGIN_NAME, "Message to '%s' - %zu bytes of data", msg->tag, msg->data_size);
+ if (msg->data_size == 0) {
Review comment:
Isn't this clear from the message on line 51?
##########
File path: plugins/experimental/memory_profile/memory_profile.cc
##########
@@ -49,26 +49,28 @@ CallbackHandler(TSCont cont, TSEvent id, void *data)
TSDebug(PLUGIN_NAME, "Message to '%s' - %zu bytes of data", msg->tag, msg->data_size);
if (strcmp(PLUGIN_NAME, msg->tag) == 0) { // Message is for us
#if TS_HAS_JEMALLOC
- int retval = 0;
- if (strncmp((char *)msg->data, "dump", msg->data_size) == 0) {
- if ((retval = mallctl("prof.dump", nullptr, nullptr, nullptr, 0)) != 0) {
- TSError("mallct(prof.dump) failed retval=%d errno=%d", retval, errno);
+ if (msg->data_size) {
Review comment:
Is the size check needed here, or just in the debug message at line 72? `strncmp` should handle a zero size.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] shinrich merged pull request #7364: traffic_ctl - plugin msg now require only the tag as mandatory field data field is now optional.
Posted by GitBox <gi...@apache.org>.
shinrich merged pull request #7364:
URL: https://github.com/apache/trafficserver/pull/7364
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org