Replacing delete in C++, missinformation

Question

I'm trying to (and have solved) a problem with 16 byte alignment issues with a class that contains SSE optimised members. But what is bugging me is a large portion of the examples I have found online contain a line of code that to me seems totally redundant yet is repeated in many places.

public:
void* operator new (size_t size)throw (std::bad_alloc)
{
    void * p = _aligned_malloc(size, 16);
    if (p == 0)  throw std::bad_alloc();
    return p; 
}

void operator delete (void *p)
{
    Camera* pC = static_cast<Camera*>(p);
    _aligned_free(p);
}

The line in question is

Camera* pC = static_cast<Camera*>(p);

As pC is never referenced and goes out of scope at the end of the function, what is the point of doing it? I have tried taking the line out and it appears to make no difference at all yet that line appears in lots of examples! Am I missing something really obvious or has an anomalous line of code been copied from example to example blindly and become prevalent across a lot of "tutorials"?

Chaiken

One reason why I might do something like that is to assist debugging. With a breakpoint on the '_aligned_free(p);', I could then inspect the Camera* pC object. – Renarenado

If the example is always Camera then certainly it has been copied from tutorial to tutorial. Blindly or otherwise. – Progress

No, Camera is my own class - replace it with whatever the class name was in each tutorial :) – Chaiken

@Martin: true, but if that's the intention of the line in the tutorial then the tutorial should probably say so. Particularly because a beginner might not immediately realise that the "object" to which pC points has already been destructed, and therefore won't look quite as they might expect. – Progress

Overloading global operator new and operator delete is almost certainly not a good idea for this. Also, note that your operator new is broken since it should call all new_handlers in turn before throwing. /EDIT: forget the first part of the comment, I didn’t notice that this was inside a class. The second part of the comment still holds. – Maje

@StuartEagles: maybe you should link to 3 or 4 of these many places that contain the code. It's probably easier to comment on that whole section of the tutorial than on a few lines of code that, I agree, appear incongruous. As Konrad says, that operator new is dodgy anyway, so it might even turn out that it's best to find another tutorial. – Progress

While pointless and confusing IMHO, it's not unusual for people to write code like this as a (dubious) form of documentation... emphasising what the pointer really points to. Sometimes people dealing with void* like the uniformity of starting every function by casting it to the real type, hoping it'll make it easier to quickly insert meaningful operations like a line of trace. – Hogen

To Martin James comment: I think that you will need another statement AFTER _aligned_free(p); to debug it efficiently - with breakpoint on _aligned_free(p); the call is not executed yet, after "step over" you are exiting the block, so I would expect Camera* pC to be out of scope already. – Freedafreedman

@KonradRudolph It's not required of allocation functions to use the installed handlers as an error strategy (although it is mentioned). What is mandatory is to either return nullptr (and be no-throw), or throw an exception (catchable via std::bad_alloc) in case of failure. – Airflow

Thanks for the help guys - I have had a look at things and am not specifying my own global handlers, the only reason I was replacing the new operator was to force Camera to be 16 byte aligned to allow XMMATRIX members to be correctly aligned when Camera is dynamically allocated. Didn't expect this to become such a lively discussion! – Chaiken

@GermannArlington - you may well be right, especially with optimization on :( – Renarenado

@GermannArlington I think it was more to allow you to check that any class members that are supposed to be freed by the destructor had correctly been destroyed rather than checking that _aligned_free(p) had correctly deallocated the assigned memory. At least that was the way I interpreted it. – Chaiken

Answer 1

The object ends its lifetime as soon as the destructor is entered, so you cannot do much with this pointer. The line Camera* pC = static_cast<Camera*>(p); can be removed safely and the only reason it exists in tutorials is that many people just copy-n-paste the code here and there without actually thinking how it works.

The clean and correct code for your delete() will look as follows:

void operator delete (void *p)
{
    _aligned_free(p);
}

Excrescent

Answer 2

As discussed in many comments to your question, the following line is indeed redundant:

Camera* pC = static_cast<Camera*>(p);  // Just an unused variable

Even if the p was of type Camera* before (other possibilities are the subclasses like Canon*, Sony*, Nikon*), you still cannot do much with pC, because the Camera::~Camera() should have already been called. operator delete is called after that.

P.S.: I haven't come across such practice to cast a pointer to specific class, but if you encounter such advise in a tutorial then you may want to change it.

Bijection