C++: How do I prevent a function from accepting a pointer that is allocated in-line?

Question

Couldn't figure out how to word the question accurately, so here's an example:

Given this function prototype:

void Foo(myClass* bar);

I want to prevent this usage:

Foo(new myClass());

and instead require a previously created object:

myClass* bar = NULL;
bar = new myClass();
Foo(bar);

or

myClass bar;
Foo(&bar);

Thanks.

---

EDIT

Here's a clarified example:

void Mouse::SetImage(BITMAP* image, int focusX, int focusY) {
    if(_image) {
        set_mouse_sprite(NULL);
        set_mouse_sprite_focus(0, 0);
        show_mouse(NULL);
        destroy_bitmap(_image);
        _image = NULL;
    }
    if(image) {
        _image = create_bitmap(image->w, image->h);
        clear_bitmap(_image);
        blit(image, _image, 0, 0, 0, 0, image->w, image->h);
    }
    if(image == NULL) {
        focusX = 0;
        focusY = 0;
    }
    _focusX = focusX;
    _focusY = focusY;
    _dirtyImage = true;
}

Whatever image the user passes in gets copied to the object's image.

If I deallocate the passed in image after copying it and the image is used elsewhere in the program it will crash the program with an access violation.

If they allocate the storage in-line and I don't deallocated it, a memory leak occurs. The problem is compounded if they call the SetImage method multiple times over the course of the running program.

Comments about using alternative libraries or on the Allegro Library itself will be ignored, I already know it's horrible. I don't have a choice.

Answer 1

Your design needs to make a choice. Either take ownership and delete it, or don't take ownership. Either way, it's up to the user to know how to use your function. They either need to know that your function will destroy the image (and maybe pass their own copy as needed), or they need to be smart enough to manage their own resources.

Typically, you don't want to steal ownership away just to delete it. So I would not delete anything. If someone is silly enough to lose the ability to delete the image they pass, that's not this functions problem. In other words, you should try to protect against Murphy, but forget about protecting against Machiavelli.

That said, raw pointer use is bad! Poor C++ code is marked by manual resource management and resource issues. You should have a wrapper around an image, that will delete the image in the destructor. That way you can never leak, even if an exception is thrown. Provide it with a reset() method which discards it's old image resource and gets a new one.

It sounds like you want shared ownership, so you'll want a reference counted resource wrapper. The issue is then solved: if someone does an "inline" allocation, it'll be put into the shared pointer and then deleted automatically when it's done. (And even better is to have an explicit constructor so someone has to know they'll be sharing the resource.)

This is done in a smart pointer called shared_ptr. Boost has one, TR1 has one, and C++0x has one. Just give it a custom deleted (one that frees the image), and you never worry about resource management again.

This should be done with all resources. The concept here is Scoped-bound Resource Management (SBRM); that a resource is managed automatically by taking advantage of the lifetime rules of automatic (stack) variables. It's known alos as it's original but uglier name Resource-Acquisition Is Initialization (RAII). Do some research into this area and you'll find your code is easier and cleaner.

---

It cannot be done without changing the type of the parameter. You could change it to:

void Foo(myClass*& bar);

Because a non-const reference can only be bound to an lvalue:

void foo(int*&);

int main(void)
{
    int *i = 0;
    int j;

    foo(i); // well-formed
    foo(&j); // ill-formed
    foo(new int); // ill-formed
}

However, this disallows taking the address of an lvalue. You can of course do the simple:

int main(void)
{
    int j;
    int* pj = &j;
    foo(pj); // well-formed
}

And it works. But I don't know why you'd want to do any of this.

---

The above solution would allow you to modify the argument (because it's a reference). If you wanted to enforce const within the function, you could make a utility like this:

template <typename T>
class require_lvalue
{
public:
    require_lvalue(T& pX) :
    mX(pX)
    {}

    const T& get(void) const
    {
        return mX;
    }

    operator const T&(void) const
    {
        return get();
    }

private:
    // non-copy-assignable
    require_lvalue& operator=(const require_lvalue&);

    const T& mX;
};

void foo(require_lvalue<int*>);

Same result, except you have a const-reference within the function.

---

Note that MSVC has a bug, and accepts this:

foo(new int);

in both cases, even though it shouldn't. (It does not accept new int(), however.)

Answer 2

It's impossible to have such a distinction of usage. And in all the cases it's a valid parameter. I really can't understand why you need this...