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 2022/06/14 16:34:00 UTC
[GitHub] [trafficserver] cmcfarlen opened a new pull request, #8911: Override LogField constructor to avoid reinterpret_cast at call site
cmcfarlen opened a new pull request, #8911:
URL: https://github.com/apache/trafficserver/pull/8911
Alternative to #8900
This adds a LogField constructor override that accepts the `unmarshal` function with the additional slice parameter and handles the reinterpret cast. This lets the type system handle the decision to cast and removes that requirement of the caller.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a diff in pull request #8911: Override LogField constructor to avoid reinterpret_cast at call site
Posted by GitBox <gi...@apache.org>.
ywkaras commented on code in PR #8911:
URL: https://github.com/apache/trafficserver/pull/8911#discussion_r897060804
##########
proxy/logging/LogField.cc:
##########
@@ -244,6 +244,12 @@ LogField::LogField(const char *name, const char *symbol, Type type, MarshalFunc
strcmp(m_symbol, "cqtn") == 0 || strcmp(m_symbol, "cqtd") == 0 || strcmp(m_symbol, "cqtt") == 0);
}
Review Comment:
Could you add a comment here like:
Typical ABIs guarantee that (using reinterpret_cast) it's safe to assign a function address to a function pointer if the function's argument types are a prefix of the argument types of the pointed to function type, and the return types match. Arguments are nominally pushed onto the top of the stack from last to first. (In practice, the top locations in the call stack are generally mapped to registers.) Therefore, the arguments that the function will use are at the same offset from the top of the stack.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] zwoop commented on pull request #8911: Override LogField constructor to avoid reinterpret_cast at call site
Posted by GitBox <gi...@apache.org>.
zwoop commented on PR #8911:
URL: https://github.com/apache/trafficserver/pull/8911#issuecomment-1177862096
This isn't necessary for 9.2.x, for now at least. If we start getting merge conflicts, we will revisit.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a diff in pull request #8911: Override LogField constructor to avoid reinterpret_cast at call site
Posted by GitBox <gi...@apache.org>.
ywkaras commented on code in PR #8911:
URL: https://github.com/apache/trafficserver/pull/8911#discussion_r897060804
##########
proxy/logging/LogField.cc:
##########
@@ -244,6 +244,12 @@ LogField::LogField(const char *name, const char *symbol, Type type, MarshalFunc
strcmp(m_symbol, "cqtn") == 0 || strcmp(m_symbol, "cqtd") == 0 || strcmp(m_symbol, "cqtt") == 0);
}
Review Comment:
Could you add a comment here like:
Typical ABIs guarantee that (using reinterpret_cast) it's safe to assign a function address to a function pointer if the function's argument types are a prefix of the argument types of the pointed to function type, and the return types match. Arguments are nominally pushed onto the top of the stack from last to first. (In practice, the top locations in the call stack are generally mapped to registers.) Therefore, the arguments that the function will use are at the same offset from the top of the stack.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] randall merged pull request #8911: Override LogField constructor to avoid reinterpret_cast at call site
Posted by GitBox <gi...@apache.org>.
randall merged PR #8911:
URL: https://github.com/apache/trafficserver/pull/8911
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org