11

When using various API's that have variable size structures (structures that must be allocated as byte[] and then cast into the struct), it would be nice if the unique_ptr holder could point to the structure, since that's what we'll be using.

Example:

std::unique_ptr<VARIABLE_SIZE_STRUCT[]> v; 
v.reset(reinterpret_cast<VARIABLE_SIZE_STRUCT*>(new BYTE[bytesRequired]));

This allowed `v to provide the view to the structure itself, which is preferable because that we don't need a second variable and we don't care about the byte pointer except for deletion.

The problem comes in with the possibility of thunking the pointer on the cast (making it unsafe to free). I see no reasonable reason why the compiler would change the pointer value on cast (since there's no inheritance), but I hear that the standard reserves the right to thunk any pointer on any cast, so as far as standard-compliant coding goes, this approach is out the window, right? Or is there some reason it is safe? Is there a way to at least static_assert this, or some other way to make it safe or cleanly deal with this type of structure?

VoidStar
  • 5,241
  • 1
  • 31
  • 45
  • For reference, on whether `reinterpret_cast` actually changes pointer values: http://stackoverflow.com/questions/3298774/any-real-example-of-reinterpret-cast-changing-a-pointer-value – John Zwinck Mar 04 '15 at 06:32
  • If your not passing around the pointer, and just want it to be automatically deleted, perhaps consider a very small wrapper class, even within the function if its only used once. – Segmented Mar 04 '15 at 06:58

2 Answers2

5

You're correct, this is unsafe. It is possible to make it safe, however.

The standard guarantees that if you reinterpret_cast to a different type, then back to the original type, you get back the original value.

You can use this along with a custom deleter, to ensure your internal pointer is cast back to the type it was allocated as before releasing it.

auto deleter = [](VARIABLE_SIZE_STRUCT* ptr) 
{ 
    delete[] reinterpret_cast<uint8_t*>(ptr); 
}; 

std::unique_ptr<VARIABLE_SIZE_STRUCT, decltype(deleter)> v
    (reinterpret_cast<VARIABLE_SIZE_STRUCT*>(new uint8_t[256]), deleter);

At this point, you're probably better off creating your own wrapper and not using unique_ptr.

Collin Dauphinee
  • 13,664
  • 1
  • 40
  • 71
  • 2
    *"The standard guarantees that if you reinterpret_cast to a different type, then back to the original type, you get back the original value."* - under 5.2.10/7, that's only guaranteed if `VARIABLE_SIZE_STRUCT` alignment requirements are no stricter than `BYTE`, which is quite likely to be untrue. It will work on most architectures anyway, but not be guaranteed by the Standard. – Tony Delroy Mar 04 '15 at 07:11
  • You could probably make it safe by messing around with std::aligned_storage. – Segmented Mar 04 '15 at 07:24
  • 1
    "At this point, you're probably better off creating your own wrapper and not using unique_ptr", i strongly disagree, using a standard class that everyone knows with a well tested implementation is highly better than a custom class that others will have to understand which could also easily introduce new bugs. The custom deleter is an awesome feature which exists for a good reason :) – Drax Mar 04 '15 at 12:44
5

  • your allocation may not have the alignment needed by VARIABLE_SIZE_STRUCT

  • the allocated memory has not had an object of VARIABLE_SIZE_STRUCT placement-newed in it - assuming you take care of that, the unique_ptr's default destructor logic should find the expected object instance to destruct, but the deallocation itself would then not be done with delete [] on a BYTE* - to have defined behaviour you'd have to customise the deleter to invoke first ~VARIABLE_SIZE_STRUCT() then delete[]...

If you're concerned about "thunking", you could do a check at run-time:

BYTE* p;
v.reset(reinterpret_cast<VARIABLE_SIZE_STRUCT*>(p = new BYTE[bytesRequired]));
assert(reinterpret_cast<BYTE*>(v.get()) == p);

The background on that - 5.2.10/7:

An object pointer can be explicitly converted to an object pointer of a different type. When a prvalue v of object pointer type is converted to the object pointer type “pointer to cv T”, the result is static_cast<cvT*>(static_cast<cv void*>(v)). Converting a prvalue of type “pointer to T1” to the type “pointer to T2” (where T1 and T2 are object types and where the alignment requirements of T2 are no stricter than those of T1) and back to its original type yields the original pointer value.

So, if the alignment requirements for VARIABLE_SIZE_STRUCT are stricter than for BYTE, you're not guaranteed to retrieve the original pointer using reinterpret_cast<BYTE*>.

Tony Delroy
  • 102,968
  • 15
  • 177
  • 252
  • Thanks, I completely passed over the necessity of delete[] in my example. Thankfully this was an artificially simplified depiction of our code so I don't really have this bug, but still, it's a nice catch. – VoidStar Mar 04 '15 at 06:49
  • Also, VARIABLE_SIZE_STRUCT and almost all like it are C structures, so placement new is not needed. Although I guess someone might try the pattern in C++ but they could provide their own creation function if its a C++ class. – VoidStar Mar 04 '15 at 06:53
  • 2
    @VoidStar you could use std::is_pod as a static_assert to rule out such cases, otherwise I agree with Tony D, placement new would be safer. – Segmented Mar 04 '15 at 07:03
  • 1
    @VoidStar: *"...but they could provide their own creation function"* - no way we could know you weren't "they" trying to do exactly that ;-). Anyway, people do sometimes still do stuff like this in C++, wanting to avoid an extra/separate memory allocation for the variable component, or because they're interfacing at a binary data level but that doesn't preclude wanting to use a constructor to initialise fields... amazing what you can find out there - occasionally justified but more often just sad. – Tony Delroy Mar 04 '15 at 07:03