Gluing Smart Pointers to Legacy Code

I had to work on a legacy C++ code base that doesn’t use smart pointers, everything is instead tracked by plain pointers. The original reasons for that include downright crappy compiler and just the code being quite old. Nowadays, I don’t see anything preventing the use of smart pointers in the legacy code base.

The question is, how do you transition from plain pointers to smart pointers when you have a big code base that cannot be updated over night.

I’ll present some techniques in this post. You can also check out my video on the same topic.

Example

Let’s suppose the code looks roughly like this:
(The actual legacy code base looks much worse, but let’s focus on the pointer business.)

bool createDestroyable(Destroyable *& destroyableOut)
{
    destroyableOut = new Destroyable;

    if ( !destroyableOut->init() )
    {
        destroyableOut->destroy();
        destroyableOut = NULL;
        return false;
    }

    return true;
}

bool legacy()
{
    Destroyable * ptr = NULL;
    
    bool rc = createDestroyable( ptr);
    rc = rc && ptr->doStuff();

    if (ptr != NULL)
    {
        ptr->destroy();
    }

    return rc;
}

And that’s just an example, there are many functions like createDestroyable() and even more call sites, so it is impossible to change the signature of createDestroyable() and update all call sites in one go.

Simple Approach

The first step is to replace Destroyable * ptr by unique_ptr ptr, but then createDestroyable() becomes incompatible so a plain pointer is still needed.

bool legacyUpgraded()
{
    unique_ptr<Destroyable, DestroyDeleter> ptr;
    bool rc = true;

    { 
        Destroyable * temp = NULL;
        rc = rc && createDestroyable(temp);
        ptr.reset(temp);
    }

    rc = rc && ptr->doStuff();

    return rc;
}

The result is quite underwhelming, sure it’s almost RAII and maybe exception-safe (depending on exact behavior of createDestroyable()), but the temp pointer hand-off is quite awkward.

A wrapper function can hide the problem, either as an overload or a distinctly named variation. Either way the code is backward-compatible with legacy call sites.

bool createDestroyableWrapper(
    unique_ptr<Destroyable, DestroyDeleter> & destroyableOut )
{
    Destroyable *ptr = nullptr;
    bool rc = false;
    try
    {
        rc = createDestroyable(ptr);
    }
    catch (...)
    {
        // required for exception safety, in case createDestroyable() 
        // throws after assigning object address to ptr
        destroyableOut.reset(ptr);
        throw;
    }

    destroyableOut.reset(ptr);
    return rc;
}

bool legacyMoreUpgraded()
{
	unique_ptr<Destroyable, DestroyDeleter> ptr;
	
	return createDestroyableWrapper(ptr) && ptr->doStuff();
}

The problem is this stills adds a non-trivial wrapper function and one such wrapper function is needed for each functions like createDestroyable(), so it could be too error-prone.

Output Pointer – Argument Glue

It is possible to devise a class for output pointers, “OutputPointer“, whose constructors accept both plain pointer reference and smart pointer reference. If createDestroyable() is changed to use OutputPointer, then the conversion rules allow the function to be called with both plain pointer and smart pointer reference and so it remains backward-compatible.

template<typename T, typename TDeleter = default_delete>
class OutputPointer
{
private:

    enum class PointerType
    {
        PLAIN,
        UNIQUE,
    };

    union Pointer
    {
        unique_ptr<T, TDeleter> * unique;
        T ** plain;
    };

    PointerType type;
    Pointer pointer;

public:
    
    OutputPointer(T *& value)
        : type(PointerType::PLAIN)
    {
        pointer.plain = &value;
    }

    OutputPointer(unique_ptr<T, TDeleter> & value)
        : type(PointerType::UNIQUE)
    {
        pointer.unique = &value;
    }
    
    // In case the pointer needs to pass through middlemen.
    OutputPointer(OutputPointer & outPointer)
        : type(outPointer.type), pointer(outPointer.pointer)
    {
    }

    ...
}

bool createDestroyable(
    OutputPointer<Destroyable, DestroyDeleter> destroyableOut
    // by value, OutputPointer itself acts as a reference
)
{ 
    ...
}

Destroyable * plain = NULL;
unique_ptr<Destroyable,DestroyDeleter> unique;
rc = createDestroyable( plain ); // Still compatible
rc = createDestroyable( unique ); // Works too

So far I have shown how OutputPointer can accept both plain and smart pointer reference and internally store the pointer via enum and union. But how should it behave?

I believe it should act like a smart pointer reference, because that’s the ultimate goal. OutputPointer is here only to make the transition easier but once all call sites are upgraded to use smart pointer, plain pointer support should be removed. The point is to get rid of plain pointers.

class OutputPointer
{
    ...

    void reset(T * value)
    {
        switch (type)
        {
        case PointerType::PLAIN:
            if (*pointer.plain != nullptr)
            {
                TDeleter()(*pointer.plain); 
                // This means deleter has to be default-constructible.
            }
            *pointer.plain = value;
            break;
        case PointerType::UNIQUE:
            pointer.unique->reset(value);
            break;
        default:
            throw logic_error("Invalid type.");
        }
    }

    // The following methods are mostly useful for
    // backward-compatibility of old method code.
    
    OutputPointer & operator=(T * value)
    {
        reset(value);
        return *this;
    }

    ... // more methods that are available on plain and smart pointers
        // like operator->(), operator!(), etc.
}

bool createDestroyable(
    OutputPointer<Destroyable, DestroyDeleter> destroyableOut)
{
    // Oftentimes, no code changes are needed in the body,
    // because OutputPointer is similar to plain pointer in usage.
    destroyableOut = new Destroyable;
    return destroyableOut->init();
}

Now any method like createDestroyable() can be easily updated to support both plain and smart pointer references. Unless…

Smart Pointer Adapter – Call Site Glue

Sometimes createDestroyable() cannot be changed. Maybe it is defined in a third party library or internal library that is code-frozen at the moment.

So the only thing that can be changed are call sites and wrappers, which leads to the awkward temporary plain pointer hand-off again.

bool createDestroyableWrapper(
    unique_ptr<Destroyable, DestroyDeleter> & destroyableOut )
{
    Destroyable *ptr = nullptr;
    bool rc = false;
    try
    {
        rc = createDestroyable(ptr);
    }
    catch (...)
    {
        // required for exception safety, in case createDestroyable() 
        // throws after assigning object address to ptr
        destroyableOut.reset(ptr);
        throw;
    }

    destroyableOut.reset(ptr);
    return rc;
}

It would be great if there was a class that would hide the complexity of the hand-off. Well, there is a way, but it requires understanding of the following rules:

• Temporary object lives until the end of the full expression containing it, at which point its destructor is executed.
• A “full expression” is an expression that’s not a sub-expression of another expression.

This means a temporary object with a plain pointer member can be created, then a reference to the plain pointer can be passed to createDestroyable() and when the full expression ends the destructor of the temporary object can update the smart pointer.

template<typename TSmartPointer, typename T>
class SmartPointerAdapter
{
    TSmartPointer * smartPointer;
    T * plainPointer;

public:
    
    SmartPointerAdapter(TSmartPointer * uniquePointer)
        : smartPointer(uniquePointer)
    {
        plainPointer = uniquePointer->get();
    }
    
    SmartPointerAdapter(SmartPointerAdapter && other)
    {
        smartPointer = other.smartPointer;
        plainPointer = other.plainPointer;

        // due to the destructor mechanism, there can be only one
        other.smartPointer = NULL; 
        other.plainPointer = NULL;
    }

    ~SmartPointerAdapter()
    {
        // SmartPointerAdapterOperations can be defined
        // for any suitable smart pointer type
        if (smartPointer && 
            SmartPointerAdapterOperations::get(*smartPointer) != plainPointer)
        {
            SmartPointerAdapterOperations::reset(*smartPointer, plainPointer);
        }
    }

    T *& get()
    {
        return plainPointer;
    }

    void operator=(SmartPointerAdapter &) = delete;
};

bool createDestroyableWrapper(
	unique_ptr<Destroyable, DestroyDeleter> & destroyableOut)
{
    return createDestroyable(
        SmartPointerAdapter< 
            unique_ptr<Destroyable, DestroyDeleter>,
            Destroyable
        >(&destroyableOut).get()
    );
}

Arguably, that’s not the easiest thing to call. It can be improved by type inference through function.

template<typename T, typename TDeleter>
SmartPointerAdapter<unique_ptr<T, TDeleter>, T>
    makeSmartPointerAdapter(unique_ptr<T, TDeleter> & uniquePointer)
{
    return SmartPointerAdapter<unique_ptr<T, TDeleter>, T>(&uniquePointer);
}

bool createDestroyableWrapper(
    unique_ptr<Destroyable, DestroyDeleter> & destroyableOut)
{
    return createDestroyable(
        makeSmartPointerAdapter(destroyableOut).get()
    );
}

That’s better, still a bit wordy though, the .get() part in particular.

There are some subtle nuances, makeSmartPointerAdapter() returns by value, which is a temporary value from caller perspective and lives until the end of createDestroyableWrapper . This is why the call to .get() must be made outside, if makeSmartPointerAdapter() returned the result of .get() instead, then the full expression where SmartPointerAdapter is created would be inside the function and the plain pointer reference would become invalid outside.

Fortunately, even if the function won’t help with the .get() part, a macro will.

#define adaptSmartPointer( smartPointer ) \
(makeSmartPointerAdapter(smartPointer).get())

bool createDestroyableWrapper(
    unique_ptr<Destroyable, DestroyDeleter> & destroyableOut)
{
    return createDestroyable( adaptSmartPointer(destroyableOut) );
}

Now that’s terse code 🙂

However it is also treacherous to some degree, remember when I said “Temporary object lives until the end of the full expression containing it.“? Consider the following snippet:

static bool pitfall()
{
    unique_ptr<Destroyable, DestroyDeleter> ptr;

    return (
        createDestroyable(adaptSmartPointer(ptr))
        &&
        ptr->doStuff() // error - still the same full expression,
                       // ptr is not updated yet
    );
}

One has to be careful to terminate the full expression before the adapted smart pointer is used again. Semicolon usually does the job. There are other things to watch out for:

• Simultaneous access to the smart pointer (asynchronous calls or reentrancy),
• Callee retaining the reference to the plain pointer for future use.

That said, when it works it works well. It should be a last resort technique, though, due to the subtle mechanics involved.

Conclusion

Both OutputPointer and SmartPointerAdapter attack the problem from different angle and are quite complementary to each other. Unfortunately there are some pitfalls around SmartPointerAdapter so it should be used carefully.

Full source (including more detailed examples) is available on my github.