current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 4 down vote favorite

The environment I'm working in requires that all allocation is done through a special set of malloc/calloc functions and that a user ID is provided when allocating. The environment has no implementation of exceptions, so errors are signaled by returning NULL.

For this reason, I have tried to write a couple of C++ functions which make it easier to create and destroy C++ objects:

namespace details {
    template<typename T>
    typename enable_if<is_pod<T>::value, T>::type *
    create(uint32_t module_id) {
        // use module_id
        return (T *)calloc(1, sizeof(T));
    }

    template<typename T, typename ...Args>
    typename enable_if<!is_pod<T>::value, T>::type *
    create(uint32_t module_id, Args&&... args) {
        // use module_id
        T *p = (T *)calloc(1, sizeof(T));
        if (p)
            new (p) T(forward<Args>(args)...);
        return p;
    }

    template<typename T>
    void destroy(typename enable_if<is_pod<T>::value, T>::type *t) {
        if (t)
            free(t);
    }

    template<typename T>
    void destroy(typename enable_if<!is_pod<T>::value, T>::type *t) {
        if (!t)
            return;

        t->~T();
        free(t);
    }
};

template<typename T, typename ...Args>
T *create(uint32_t module_id, Args&&... args) {
    return details::create<T>(module_id, forward<Args>(args)...);
}

template<typename T>
void destroy(T *t) {
    details::destroy<T>(t);
}

Can this be done smarter? And are there any cases where my create/destroy functions are not handling as one would expect?

share|improve this question
1  
Why not use new/delete? –  Loki Astari Nov 5 '13 at 19:12
    
Can you provide an example on how to do that where the actually allocation is done with calloc, and where I can pass a module_id/user_id as an argument? –  Allan Nov 6 '13 at 9:00
    
No point in reviewing. It does exactly what it says it does. Nothing wrong with some very simple code. There is not enough context to understand if it is the correct thing to do. –  Loki Astari Nov 6 '13 at 18:26
    
@Loki Astari: I found one bug in the code which is discussed at: stackoverflow.com/questions/19808448/… I will update this post with the conclusion from this question –  Allan Nov 7 '13 at 8:00
1  
You are aware that you could define your own operator new, operator delete, and their array counterparts right? The advantage of that would be that they also work when objects are created outside of code you directly contril, such as standard containers. –  Christopher Creutzig Dec 1 '13 at 22:00

1 Answer 1

up vote 2 down vote

After some research, I'm convinced Christopher Creutzig is right. If you want all your allocations to go through customized functions, you're much better off replacing the global operator new. This will catch all calls to new for the translation unit, whether you replace them with create or not. That means when you create a std::vector<T> myvec(3), it will use your operator new for any heap allocations it needs to do, instead of sidestepping your global policy.

If you need to pass a module id, operator new can handle that as well by accepting extra parameters. I'm unclear how that mixes with examples such as the std::vector<T> heap allocations; it may require using a custom allocator for each STL container. Setting that aside, here's an example like yours following the lead on cppreference.com:

void* operator new(std::size_t cb, uint32_t module_id)
{
    // use module_id
    return calloc(1, cb);
}
void operator delete(void* ptr)
{
    free(ptr);   // I'm unsure if this requires a null check
}

type *p = new(module_id) type;

As an aside, if you're particularly tied to calling functions, I'd be tempted to remove the SFINAE specialization via is_pod<T> and just make it an inline check via is_trivially_constructible<T> and is_trivially_destructible<T>...that is if checking is necessary at all.

template<typename T>
T* create(uint32_t module_id)
{
    // use module_id
    T* p = calloc(1, sizeof(T));
    if (!is_trivially_constructable<T>::value && p)
    {
        new (p) T(forward<Args>(args)...);
    }
    return p;
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.