How to use auto_ptr in this case

Question

I have the following code:

void do_something(Image *image)
{

    Image *smoothed = NULL;
    Image *processed = NULL;
    if (condition_met) {
        smoothed = smooth(image);
        processed = smoothed;
    }
    else {
        processed = image;
    }

    ...
    delete smoothed_image
}

I was wondering if I can do the following and if this is the correct way. I am confused about setting another pointer from an auto_ptr object and whether that changes the ownership somehow.

void do_something(Image *image)
{
    auto_ptr<Image *> smoothed;
    Image *processed = NULL;

    if (condition_met) {
        smoothed = smooth(image); // This should own the pointer returned by smooth
        processed = smoothed; // Is this OK? processed is not an auto_ptr
    }
    else {
        processed = image;
    }

    ...
    // Destructor for the 'smoothed' variable should be called.
    // The 'image' variable is not deleted.
}

Will the destructor get called as I intend it and is this the correct way to do this?

Answer 1

A few points to make. Assuming that the signature of the smooth function is

Image* smooth(Image*);

Then your code would minimally have to be changed to

void do_something(Image *image)
{
    auto_ptr<Image> smoothed;
    Image *processed = NULL;

    if (condition_met) {
      smoothed.reset(smooth(image)); // This should own the pointer returned by smooth
      processed = smoothed.get(); // Maybe?  Depends on what you're doing
    }
    else {
      processed = image;
    }

It's reasonable in some cases to pull the raw pointer out of a smart pointer as is done in by smoothed.get() above, but you have to understand that as written the pointer held by smoothed will be deleted at the end of the function, even though you've done something else with the raw pointer. Not enough information here to see if that's a problem, but it's a smell.

std::auto_ptr is now deprecated in favor of std::unique_ptr. The big reason for this is the way the contents of auto_ptr are moved:

std::auto_ptr<int> a = new int(5);
std::auto_ptr<int> b = a;  // Copy?  No!

In this code the pointer held by a has been transferred to b. a is no longer holding anything. This runs counter to how we usually think of copy behavior so it's easy to mess up.

C++11 introduced the concept of r-value references (there are loads of articles around the net explaining this, including What are move semantics?). Having r-value references allows unique_ptr to prevent data from being moved where it doesn't make sense.

std::unique_ptr<int> a = new(5);
std::unique_ptr<int> b = a; // Compile fail.  Not copyable
std::unique_ptr<int> c = std::move(a); // OK, shows intent

With this available, it would be possible to update your smooth() function signature to

std::unique_ptr<Image> smooth(Image*) {
  ...
  return newImage;
}

Which clearly instructs the caller that they should take ownership of of the returned pointer. Now you can say

unique_ptr<Image> smoothed;
...
smoothed = smooth(image);

Because a value returned from a function is an r-value (not bound to a variable yet), the pointer is safely moved to smoothed.

In the end, yes, using a smart pointer is preferable to calling delete. Just think through your design and try to be clear about pointer ownership.