boost::shared_ptr semantics (copying)

Question

I just wanted to have a fresh pair of eyes that the below code is correct in that:

The pointers contained in the object trifoo (stored in a ptr_vector) are the shared pointers f, g, h.

Also, what is the result of the shared_ptr copy in the constructor of trifoo; is this the correct method of 'sharing' shared_ptr, ensuring reference counts are increased etc. All my other doubts I was able to test to verify, but I'm not sure how I can check this (properly). Any critique is welcome also.

#include <boost/ptr_container/ptr_vector.hpp>
#include <boost/shared_ptr.hpp>

class foo {
    int a, b;
public:
    foo(int A, int B) : a(A), b(B) {}
};

typedef boost::shared_ptr<foo> foo_ptr;

class trifoo {
    foo_ptr c, d, e;
public:
    trifoo(const foo_ptr& C, const foo_ptr& D, const foo_ptr& E) : c(C), d(D), e(E) {}
};

int main() {
    for (int i = 0; i < 5000000; i++) {
        foo_ptr f(new foo(1,2));
        foo_ptr g(new foo(2,3));
        foo_ptr h(new foo(4,5));

        boost::ptr_vector<trifoo> tris;

        tris.push_back(new trifoo(f, g, h));
    }

    return 0;
}

Note: the pointless loop was to test memory leaks, of which none occurred.

Looks fine to me. You should prefer an initialization list to assignment, though, and your `tris` vector will only ever have one element, you probably wanted that out of the loop. — GManNickG, Jan 30 '11 at 05:39
That was simply to test on the capabilities of a ptr_vector, this implementation is not for a practical application - and thanks. — hiddensunset4, Jan 30 '11 at 06:16

score 6 · Accepted Answer · answered Jan 30 '11 at 06:02

6

The code seems to be technically correct.

Semantics of copying a shared_ptr, however it is done, is that the reference count of the referred-to object is increased. It just works. Nothing to worry about.

Some style issues, though:

Passing shared_ptr by reference, or declaring it const, is meaningless. That's because it can always be copied. Just pass those shared_ptr's by value.
Use constructor initializer lists instead of assignments where practically possible.
Having the three new's in different expressions is very good. It avoids an exception safety pitfall. But even better, put that creation logic in a factory function.

Cheers & hth.,

answered Jan 30 '11 at 06:02

Cheers and hth. - Alf

142,714
15
209
331

Thanks for the answer, in regards to the factory function, having just learnt about this concept, would that (approximately) mean to have a function such as: foo_ptr foo::makeFooPtr(int a ,int b) { return foo_ptr(new foo(a,b)); } Or at least along those lines. – hiddensunset4 Jan 30 '11 at 06:11
Wouldn't doing it by reference save a copy (if a copy is made)? I would take them by-value, then swap them into my members, in C++03. – GManNickG Jan 30 '11 at 06:11
This was my biggest doubt, I wasn't sure how the shared_ptr would parse in, the semantics about what is actually copied was unclear to me. I can only imagine for the reference counter to remain consistent, it must point to the same pointer regardless of the copy? Also, how might I implement a swap as mentioned. – hiddensunset4 Jan 30 '11 at 06:18
@Daniel: See the second half of [this answer](http://stackoverflow.com/questions/4839257/is-it-necessary-to-define-move-constructors-from-different-classes/4839571#4839571). – GManNickG Jan 30 '11 at 06:22
1

Hey Gman, interesting results from a few tests (simple sys\time.h benchmarks). The pass by value and then c = C, was 30% slower than the: c.swap(C) or const foo_ptr &C implementations (which were equal +- %5); and the slow down noticeable also with large iterations. I decided to use the swap methods, but I was wondering, is it possible to do 'c.swap(C)' in an initializer list? Not that it should matter either way. Note that all the tests were done over small iterations of around 500, this is because anything to large and the results were too variable (assumed to be because of other processes). – hiddensunset4 Jan 30 '11 at 09:09
@Daniel: The const-ref and copy-then-swap should perform the same, except in the case where the copy is *elided*, which your test may not have considered. (Though the timer may not be precise enough to tell, and 500 iterations is awfully low.) This copy-and-swap can be done in C++0x with move-semantics (that is, you move things instead of copy them), but for now, no. – GManNickG Jan 30 '11 at 23:16
1

Thanks for the hints, I'm not entirely sure on the true meaning of elision, but I'm reading around. As I said, if I went over 500 iterations, it became too variable between executions, presumably due to other system loads. Smaller iterations but averaged (so maybe 10,000 run, but never consecutively) gave me those results. Not a single test of 500. – hiddensunset4 Jan 30 '11 at 23:33

boost::shared_ptr semantics (copying)

1 Answers1