Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cleanup: use std::make_shared to replace new #12276

Merged
merged 6 commits into from Jan 6, 2017
Merged

cleanup: use std::make_shared to replace new #12276

merged 6 commits into from Jan 6, 2017

Conversation

wycbox
Copy link

@wycbox wycbox commented Dec 2, 2016

Signed-off-by: Yunchuan Wen yunchuan.wen@kylin-cloud.com

@gregsfortytwo
Copy link
Member

Looks good at a quick glance. @wwformat, did you use some kind of script to pick these out? Be helpful if you can share it. :)

@@ -562,7 +562,7 @@ int main(int argc, const char **argv)
::encode(v, final);
::encode(mapbl, final);

MonitorDBStore::TransactionRef t(new MonitorDBStore::Transaction);
MonitorDBStore::TransactionRef t(std::make_shared<MonitorDBStore::Transaction>());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, why not use auto instead?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel the 'auto' keyword is suitable for complex template declare

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I have updated and use auto keyword.

Yunchuan Wen added 6 commits December 9, 2016 13:45
Signed-off-by: Yunchuan Wen <yunchuan.wen@kylin-cloud.com>
Signed-off-by: Yunchuan Wen <yunchuan.wen@kylin-cloud.com>
Signed-off-by: Yunchuan Wen <yunchuan.wen@kylin-cloud.com>
Signed-off-by: Yunchuan Wen <yunchuan.wen@kylin-cloud.com>
Signed-off-by: Yunchuan Wen <yunchuan.wen@kylin-cloud.com>
Signed-off-by: Yunchuan Wen <yunchuan.wen@kylin-cloud.com>
@tchaikov
Copy link
Contributor

tchaikov commented Dec 9, 2016

/home/jenkins-build/build/workspace/ceph-pull-requests/qa/workunits/cephtool/test.sh:1842: test_mon_ping:  ceph ping 'mon.*'
...
Error connecting to cluster: TimedOut
...
/home/jenkins-build/build/workspace/ceph-pull-requests/qa/workunits/cephtool/test.sh:1: test_mon_ping:  rm -fr /tmp/cephtool.T95
...

The following tests FAILED:
	  5 - cephtool-test-mon.sh (Failed)

retest this please.

@wycbox
Copy link
Author

wycbox commented Dec 9, 2016

retest this please.

I repeat the test, and the step is:
cd /root/develop/ceph-builder/build
../src/test/cephtool-test-mon.sh

The result is:
/root/develop/ceph-builder/qa/workunits/cephtool/test.sh:2033: main: echo OK
OK
/root/develop/ceph-builder/qa/workunits/cephtool/test.sh:1: main: rm -fr /tmp/cephtool.3z4

@tchaikov
Copy link
Contributor

tchaikov commented Dec 9, 2016

@wwformat sorry for the confusion, "retest this please" was a command for jenkins to re-trigger the build/test job. i wanted to see if the failure is transient or not.

@tchaikov tchaikov self-assigned this Dec 9, 2016
@gregsfortytwo
Copy link
Member

Still wondering how you found/picked out these ones. In the MDS "grep 'new Mutation'" matches the count of changed ones here, but there are several other shared_ptr typedefs which haven't been hit. Are these just the ones you know about and were bugging you? Was there some single script, or set of scripts we'll get another PR on? :)

@wycbox
Copy link
Author

wycbox commented Dec 10, 2016

Still wondering how you found/picked out these ones. In the MDS "grep 'new Mutation'" matches the count of changed ones here, but there are several other shared_ptr typedefs which haven't been hit. Are these just the ones you know about and were bugging you? Was there some single script, or set of scripts we'll get another PR on? :)

There are many declare of shared_ptr like XXXRef or XXX::Ref.
So, I use a very simple script to filter the code, and pick the right result manullay.
#/bin/bash
git grep -E "^( |\t)[a-zA-Z0-9_]+(::[a-zA-Z0-9_]+)( )+[a-zA-Z0-9_]+(::[a-zA-Z0-9_]+)( |\t)(( |\t)new( |\t)+" | grep Ref
git grep -E "=( )
[a-zA-Z0-9_]+(::[a-zA-Z0-9_]+)( |\t)(( |\t)*new( |\t)+" | grep Ref

@tchaikov tchaikov removed their assignment Jan 6, 2017
@jcsp jcsp merged commit a7b0b1f into ceph:master Jan 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants