You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "shukitchan (via GitHub)" <gi...@apache.org> on 2024/06/11 22:47:13 UTC
[PR] Properly handle string return from function [trafficserver]
shukitchan opened a new pull request, #11442:
URL: https://github.com/apache/trafficserver/pull/11442
(no comment)
--
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
Re: [PR] Properly handle string return from function [trafficserver]
Posted by "shukitchan (via GitHub)" <gi...@apache.org>.
shukitchan commented on PR #11442:
URL: https://github.com/apache/trafficserver/pull/11442#issuecomment-2163570593
[approve ci rocky]
--
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
Re: [PR] Properly handle string return from function [trafficserver]
Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on PR #11442:
URL: https://github.com/apache/trafficserver/pull/11442#issuecomment-2176288689
Cherry-picked to v10.0.x
--
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
Re: [PR] Properly handle string return from function [trafficserver]
Posted by "shukitchan (via GitHub)" <gi...@apache.org>.
shukitchan commented on PR #11442:
URL: https://github.com/apache/trafficserver/pull/11442#issuecomment-2165729793
[approve ci rocky]
--
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
Re: [PR] Properly handle string return from function [trafficserver]
Posted by "traeak (via GitHub)" <gi...@apache.org>.
traeak commented on code in PR #11442:
URL: https://github.com/apache/trafficserver/pull/11442#discussion_r1638445447
##########
plugins/experimental/wasm/ats_context.h:
##########
@@ -276,9 +276,12 @@ class Context : public ContextBase
const char *
getTransformResult(size_t *body_size)
{
- std::string s = transform_result_.get();
- *body_size = s.size();
- return s.c_str();
+ if (body_size) {
Review Comment:
Check against nullptr please?
--
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
Re: [PR] Properly handle string return from function [trafficserver]
Posted by "JosiahWI (via GitHub)" <gi...@apache.org>.
JosiahWI commented on PR #11442:
URL: https://github.com/apache/trafficserver/pull/11442#issuecomment-2163597260
[approve ci rocky]
--
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
Re: [PR] Properly handle string return from function [trafficserver]
Posted by "JosiahWI (via GitHub)" <gi...@apache.org>.
JosiahWI commented on PR #11442:
URL: https://github.com/apache/trafficserver/pull/11442#issuecomment-2162730882
LSan detected a leak in the magick plugin, from an unknown module.
--
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
Re: [PR] Properly handle string return from function [trafficserver]
Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on code in PR #11442:
URL: https://github.com/apache/trafficserver/pull/11442#discussion_r1637278286
##########
plugins/experimental/wasm/ats_context.h:
##########
@@ -276,9 +276,8 @@ class Context : public ContextBase
const char *
getTransformResult(size_t *body_size)
{
- std::string s = transform_result_.get();
- *body_size = s.size();
- return s.c_str();
+ *body_size = transform_result_.get().size();
Review Comment:
What if body_size is nullptr?
--
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
Re: [PR] Properly handle string return from function [trafficserver]
Posted by "JosiahWI (via GitHub)" <gi...@apache.org>.
JosiahWI commented on PR #11442:
URL: https://github.com/apache/trafficserver/pull/11442#issuecomment-2162756643
[approve ci rocky]
--
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
Re: [PR] Properly handle string return from function [trafficserver]
Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on PR #11442:
URL: https://github.com/apache/trafficserver/pull/11442#issuecomment-2165916859
Looks good to me. I'll let a non Yahoo to +1!
Thanks @shukitchan
--
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
Re: [PR] Properly handle string return from function [trafficserver]
Posted by "JosiahWI (via GitHub)" <gi...@apache.org>.
JosiahWI commented on PR #11442:
URL: https://github.com/apache/trafficserver/pull/11442#issuecomment-2165752680
[approve ci rocky]
--
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
Re: [PR] Properly handle string return from function [trafficserver]
Posted by "JosiahWI (via GitHub)" <gi...@apache.org>.
JosiahWI commented on PR #11442:
URL: https://github.com/apache/trafficserver/pull/11442#issuecomment-2162731063
[approve ci rocky]
--
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
Re: [PR] Properly handle string return from function [trafficserver]
Posted by "shukitchan (via GitHub)" <gi...@apache.org>.
shukitchan commented on code in PR #11442:
URL: https://github.com/apache/trafficserver/pull/11442#discussion_r1637973380
##########
plugins/experimental/wasm/ats_context.h:
##########
@@ -276,9 +276,12 @@ class Context : public ContextBase
const char *
getTransformResult(size_t *body_size)
{
- std::string s = transform_result_.get();
- *body_size = s.size();
- return s.c_str();
+ if (body_size) {
+ *body_size = transform_result_.get().size();
+ return transform_result_.get().c_str();
Review Comment:
Actually you are right. Let me redo this.
--
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
Re: [PR] Properly handle string return from function [trafficserver]
Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on code in PR #11442:
URL: https://github.com/apache/trafficserver/pull/11442#discussion_r1637886093
##########
plugins/experimental/wasm/ats_context.h:
##########
@@ -276,9 +276,12 @@ class Context : public ContextBase
const char *
getTransformResult(size_t *body_size)
{
- std::string s = transform_result_.get();
- *body_size = s.size();
- return s.c_str();
+ if (body_size) {
+ *body_size = transform_result_.get().size();
+ return transform_result_.get().c_str();
Review Comment:
If I am looking at the right place, this may still dangle as `get()` returns by value.
https://github.com/apache/trafficserver/blob/master/plugins/experimental/wasm/ats_context.h#L183-L187
--
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
Re: [PR] Properly handle string return from function [trafficserver]
Posted by "shukitchan (via GitHub)" <gi...@apache.org>.
shukitchan commented on code in PR #11442:
URL: https://github.com/apache/trafficserver/pull/11442#discussion_r1637395750
##########
plugins/experimental/wasm/ats_context.h:
##########
@@ -276,9 +276,8 @@ class Context : public ContextBase
const char *
getTransformResult(size_t *body_size)
{
- std::string s = transform_result_.get();
- *body_size = s.size();
- return s.c_str();
+ *body_size = transform_result_.get().size();
Review Comment:
good catch
--
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
Re: [PR] Properly handle string return from function [trafficserver]
Posted by "shukitchan (via GitHub)" <gi...@apache.org>.
shukitchan merged PR #11442:
URL: https://github.com/apache/trafficserver/pull/11442
--
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