Smart pointer with local buffer optimization

Question

This is example of smart pointer with type erased deleter and local buffer optimization enabled. It is from the book https://books.google.am/books/about/Hands_On_Design_Patterns_with_C++.html?id=iQGGDwAAQBAJ&printsec=frontcover&source=kp_read_button&redir_esc=y#v=onepage&q&f=false

#include <iostream>

template <typename T>
class smart_ptr
{
    struct deleter_base 
    {
        virtual ~deleter_base() {};
        virtual void apply(T* p) = 0;
    };
    
    template <typename Deleter>
    struct deleter : public deleter_base
    {
        deleter(Deleter d): m_d(d) {}
        void apply(T* p) override 
        {
            m_d(p);
        }
        Deleter m_d;
    };

    deleter_base* m_db;
    T* m_p;
    char m_buf[16];
    
public:
    template <typename Deleter>
    smart_ptr(T* p, Deleter d)
        : m_p(p)
        , m_db( sizeof(Deleter) > sizeof(m_buf) ? new deleter<Deleter>(d) : new (m_buf) deleter<Deleter>(d))
    {
    }
    
    ~smart_ptr() 
    {
        m_db->apply(m_p);
        if (static_cast<void*>(m_db) == static_cast<void*>(m_buf))
        {
            m_db->~deleter_base();
        }
        else 
        {
            delete m_db;
        }
    }
    
    T& operator*() {return *m_p;}
    const T& operator*() const {return *m_p;}
    T* operator->() { return m_p; }
    const T* operator->() const { return m_p; }
};

struct Test 
{
    Test(double d): m_d(d) { std::cout << "Test::Test" << std::endl; }
    ~Test() { std::cout << "Test::~Test" << std::endl; }
    
    double m_d;
};

int main()
{
    auto deleter = [](Test * t) {delete t;};
    smart_ptr<Test> spd(new Test(3.6), deleter);
    
    std::cout << spd->m_d << std::endl;
    
}

One question. When deleter does not fit inside local buffer we allocate memory for it and leave m_buf uninitialized. Is it a bug or I miss something?

In the destructor we compare m_buf which has random value against value returned by new deleter<Deleter>(d). It is not impossible that those values match, right?

`m_buf` provides the storage (is used with placement new). As long as the implementation always relies on `m_db` to refer to the deleter, it's regardless if `m_buf` is uninitialized if unused. You may init. with `{ }` to make this more robust but it might be considered as pessimization. — Scheff's Cat, Jul 05 '21 at 09:35
A bug is something (not) working in a wrong way. How is the buffer's content "wrong" if we aren't using it? — StoryTeller - Unslander Monica, Jul 05 '21 at 09:35
But we use it inside the destructor of smart_ptr. We read there from uninitialized memory, no ? — Ashot, Jul 05 '21 at 09:36
We compare the **address** of the buffer to something. We do not access its content before that. — StoryTeller - Unslander Monica, Jul 05 '21 at 09:37
...but with an explicit check before: `if (static_cast(m_db) == static_cast(m_buf))` which guards the case it is not uninitialized. — Scheff's Cat, Jul 05 '21 at 09:37

score 1 · Accepted Answer · answered Jul 05 '21 at 09:37

1

It is not a bug - it is valid to have uninitialized memory. You only have Undefined Behavior (UB) when you read from uninitialized memory.

This makes sense - if merely having uninitialized memory would already be a bug, then placement new such as in the smart_ptr ctor above would be pointless. But that is safe, because it starts with a write.

answered Jul 05 '21 at 09:37

MSalters

173,980
10
155
350

m_buf will contain random value, What if accidently m_db content matches that random value. Is it not impossible, right? – Ashot Jul 05 '21 at 09:55
1

@Ashot: It will indeed contain a random value, when not used for the `Deleter`. But `m_buf` **address** won't be random. That will be slightly bigger than `this` due to the preceding `m_db` and `m_p`. – MSalters Jul 05 '21 at 10:00
`m_buf`'s address is unrelated. We don't compare the address of `m_buf` but it's value. It is compared against value returned by `new deleter(d)`. I don't see a reason why those values should be always different. – Ashot Jul 05 '21 at 10:14
1

@Ashot: That's wrong. `m_buf` is used as `static_cast(m_buf)`. Now `m_buf` is an array, so this cast causes an implicit conversion from `char[16]` to `char*` and then an explicit conversion from `char*` to `void*`. That `char*` is the address of `m_buf[0]`, the begin of the array. – MSalters Jul 05 '21 at 10:21

Smart pointer with local buffer optimization

1 Answers1