views:

202

answers:

6

Hi, gents:

I am now hacking an old C code, try to make it more C++/Boost style:

there is a resource allocation function looks like:

my_src_type* src;
my_src_create(&src, ctx, topic, handle_src_event, NULL, NULL);

i try to wrap src by a shared_ptr:

shared_ptr<my_src_type> pSrc;

I forgot to mention just now. I need to do this as a loop

std::map<string, shared_ptr<my_src_type>  > dict;
my_src_type* raw_ptr;

BOOST_FOREACH(std::string topic, all_topics)
{
    my_src_create(&raw_ptr, ctx, topic, handle_src_event, NULL, NULL);
    boost::shared_ptr<my_src_type> pSrc(raw_ptr);
    dict[topic] = pSrc;
}

Can I do it like this?

A: 

Are you also reworking my_src_create()?

Given the name of the function, I would return a boost::shared_ptr rather than passing in a pre-constructed one as that is a lot clearer and cleaner IMHO. Unless the function is also returning some sort of fail/success code, but that would be better dealt by throwing an exception.

If you desperately want to pass in an empty shared_ptr, you can pass it in by non-const reference and use shared_ptr::reset() inside my_src_create to assign it a new value.

Timo Geusch
A: 

You can always do it "the old way" and then assign that src pointer to shared_ptr using reset() or make_shared().

Marcin Gil
A: 

I would use this code to solve your problem:

my_src_type* src;
my_src_create(&src, ctx, topic, handle_src_event, NULL, NULL);
boost::shared_ptr<my_src_type> pSrc(src);

From that point on pSrc would manage the allocated memory pointed by my_src_type* src.

EDIT: removing an erronous part of my answer.

fogo
The second version is not valid: `pSrc.get()` returns a copy of the pointer, not the actual pointer. Modifying that would have no effect inside the shared pointer
David Rodríguez - dribeas
Yeah, I know. Just got it top of my head without checking. Too early in these parts of the world :) I edited it out.
fogo
+1  A: 

No.

Basically, you have to do it the old C way and then convert the result to a shared_pointer somehow.

You can do it by simply initializing the shared_pointer

my_src_type* pSrc;
my_src_create(&src, ctx, topic, handle_src_event, NULL, NULL);
shared_ptr<my_src_type> sp(pSrc);

but beware, this will fail if the my_src_create function can return an already existing object. Also, if there is a my_src_destroy function, it won't be called.

IMHO the cleanest way is to wrap your structure in a C++ class:

class MySrc {
  my_src_type* pSrc;
public:
  MySrc(...) { my_src_create(&pSrc, ...); }
  ~MySrc() { my_src_destroy(&pSrc); }
private:
  MySrc(const MySrc&);
  void operator=(const MySrc&); // disallow copying
};

and then use shared pointer for MySrc the ususal way.

jpalecek
In the first block, the shared pointer constructor that takes a raw pointer is explicit, so you have to use explicit initialization: `shared_ptr<my_src_type> sp(pSrc);` instead of `shared_ptr<my_src_type> sp=pSrc;` (also `shared_ptr` is misspelled)
David Rodríguez - dribeas
In your second snippet, you should either make the class non-copyable, or override the default copy constructor and assignment operator to perform a deep-copy.
Emile Cormier
@David Rodríguez - dribeas, @Emile Cormier: Thanks, reflected your objections in an edit.
jpalecek
+2  A: 

I don't think that your questions makes much sense. If my_src_create is being reworked, then pass a reference to the shared pointer, or return a shared pointer instead. If you are not reworking that method then you cannot really do it. I recommend to use a raw pointer for creation and then wrap it into a shared pointer:

shared_ptr<my_src_type> src;
{
   my_src_type* raw_src;
   my_src_create(&raw_src, ctx, topic, handle_src_event, NULL, NULL);
   src.reset( raw_src ); // hand ownership to shared_ptr
}

Acquiring the pointer inside the shared pointer and modifying it would break the shared pointer invariants: you would be changing the pointer but not updating the share count.

David Rodríguez - dribeas
+4  A: 

Using shared_ptr on C-style resources

With boost::shared_ptr, you can pass a function pointer to a "deleter" that will be called automatically when the reference count reaches zero. This feature allows shared_ptr to be used to manage resources returned by legacy C APIs.

Consider leaving your legacy my_src_create intact, and provide a new "factory" function that returns a shared_ptr:

void my_src_deleter(my_src_type* raw_ptr)
{
    my_src_destroy(raw_ptr);
}

typedef boost::shared_ptr<my_src_type> my_src_shared_ptr;

my_src_shared_ptr create_my_src(...)
{
    my_src_type* raw_ptr;
    my_src_create(&raw_ptr, ctx, topic, handle_src_event, NULL, NULL);
    return my_src_shared_ptr(raw_ptr, &my_src_deleter);
}

std::map<string, my_src_shared_ptr> dict;

BOOST_FOREACH(std::string topic, all_topics)
{
    dict[topic] = create_my_src(ctx, topic, handle_src_event, NULL, NULL);
}

Wrapping Legacy C Struct/Functions in a Class

Alternatively, (as jpalecek suggested) you can wrap my_src in a class. Creation and destruction of legacy my_src objects is handled in the constructor and destructor. If you're going to do this, you should think about whether or not you want your MySrc class to be copyable. If MySrc is heavyweight or expensive to create, you'll probably want to make it non-copyable and consider using shared_ptr<MySrc> if there is going to be shared ownership of MySrc:

class MySrc
{
public:
    typedef boost::shared_ptr<MySrc> Ptr;
    MySrc(...) { my_src_create(&src_, ...); }
    ~MySrc() { my_src_destroy(&src_); }
    // Other member functions that uses my_src legacy functions

private:
    my_src_type* src_;
    // Make copy-constructor and assignment private to disallow copies
    MySrc(const MySrc& rhs) {}
    MySrc& operator=(const MySrc& rhs) {return *this;}
};

std::map<string, MySrc::Ptr> dict;

BOOST_FOREACH(std::string topic, all_topics)
{
    dict[topic] = MySrc::Ptr(
        new MySrc(ctx, topic, handle_src_event, NULL, NULL) );
}

Note that you can also use the MySrc class to wrap legacy functions that operate on my_src instances.

If you want MySrc to be copyable, then make sure to implement the copy-constructor and assignment operator so that a deep-copy is performed.

Emile Cormier