[GitHub] [trafficserver] bneradt commented on a change in pull request #6972: Traffic Dump: dump server-side protocol stack

[GitBox <gi...@apache.org>]

bneradt commented on a change in pull request #6972:
URL: https://github.com/apache/trafficserver/pull/6972#discussion_r451245597



##########
File path: src/traffic_server/InkAPI.cc
##########
@@ -9670,6 +9696,27 @@ TSHttpSsnClientProtocolStackGet(TSHttpSsn ssnp, int n, const char **result, int
   return TS_SUCCESS;
 }
 
+// Return information about the protocols used by the server
+TSReturnCode
+TSHttpTxnServerProtocolStackGet(TSHttpTxn txnp, int n, const char **result, int *actual)
+{
+  sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
+  sdk_assert(n == 0 || result != nullptr);
+  HttpSM *sm = reinterpret_cast<HttpSM *>(txnp);
+  int count  = 0;
+  if (sm && n > 0) {
+    auto mem = static_cast<std::string_view *>(alloca(sizeof(std::string_view) * n));

Review comment:
       The memory from `alloca` is freed on function return automatically:
   
   https://linux.die.net/man/3/alloca
   
   That might look problematic on the surface, but note that `mem` is only ultimate used to get the data() members of the statically allocated global string_view tags (IP_PROTO_TAG_HTTP_2_0, etc.) which are assigned to the caller's result char**. Thus I don't see a use after free or leak here. Also, the string_view constructor isn't needed because these wind up all just pointing to those global string_view tags.
   
   (In case this is helpful, I didn't come up with this on my own. This function's logic is borrowed from the existing TSHttpSsnClientProtocolStackGet implementation, with modifications to reference the state machine function.) 




----------------------------------------------------------------
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