Question

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?

Was it helpful?

Solution

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.

OTHER TIPS

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.

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.

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.

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

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.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top