You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by GitBox <gi...@apache.org> on 2022/04/30 09:32:41 UTC

[GitHub] [mesos] cf-natali opened a new pull request, #432: Fixed clang-tidy warnings due to capturing this in a deferred lambda.

cf-natali opened a new pull request, #432:
URL: https://github.com/apache/mesos/pull/432

   Use `defer(self(), lambda)` instead to avoid the risk of use-after-free.
   
   See on the mesos-tidy CI job:
   ```
   /tmp/SRC/src/csi/v0_volume_manager.cpp:1078:13: warning: callback
   capturing this should be dispatched/deferred to a specific PID
   [mesos-this-capture]
         .then([=](const Map<string, string>& secrets) {
   ```
   
   Together with the recent fixes merged, this fix should allow the
   mesos-tidy CI job to be green again:
   https://ci-builds.apache.org/job/Mesos/job/Mesos-Tidybot/


-- 
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: reviews-unsubscribe@mesos.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [mesos] qianzhangxa merged pull request #432: Fixed clang-tidy warnings due to capturing this in a deferred lambda.

Posted by GitBox <gi...@apache.org>.
qianzhangxa merged PR #432:
URL: https://github.com/apache/mesos/pull/432


-- 
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: reviews-unsubscribe@mesos.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [mesos] qianzhangxa commented on a diff in pull request #432: Fixed clang-tidy warnings due to capturing this in a deferred lambda.

Posted by GitBox <gi...@apache.org>.
qianzhangxa commented on code in PR #432:
URL: https://github.com/apache/mesos/pull/432#discussion_r864525131


##########
src/csi/v0_volume_manager.cpp:
##########
@@ -1075,15 +1075,15 @@ Future<Nothing> VolumeManagerProcess::__publishVolume(const string& volumeId)
 
   if (!volumeState.node_stage_secrets().empty()) {
     rpcResult = resolveSecrets(volumeState.node_stage_secrets())
-      .then([=](const Map<string, string>& secrets) {

Review Comment:
   Nit: Should we use `process::defer` instead? I see `process::defer` is used widely in this file.



-- 
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: reviews-unsubscribe@mesos.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [mesos] cf-natali commented on a diff in pull request #432: Fixed clang-tidy warnings due to capturing this in a deferred lambda.

Posted by GitBox <gi...@apache.org>.
cf-natali commented on code in PR #432:
URL: https://github.com/apache/mesos/pull/432#discussion_r865130859


##########
src/csi/v0_volume_manager.cpp:
##########
@@ -1075,15 +1075,15 @@ Future<Nothing> VolumeManagerProcess::__publishVolume(const string& volumeId)
 
   if (!volumeState.node_stage_secrets().empty()) {
     rpcResult = resolveSecrets(volumeState.node_stage_secrets())
-      .then([=](const Map<string, string>& secrets) {

Review Comment:
   Sure  - done.



-- 
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: reviews-unsubscribe@mesos.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org