3

I'm having a hard time understanding smart pointers (still in the beginning stages of learning tbh). Maybe I've been starring at the problem too long and I'm missing the easy concept...

I'm in the process of turning all my "new/deletes" into smart pointers so I don't have such a big issue with memory leaks/corruption.

With unique_ptr's you can't just:

PCHAR test;
std::unique_ptr<char[]> buffer = std::make_unique<char[]>(10);
buffer.get() = test;

(Please correct me if I'm wrong) So instead, I'm passing a raw shared_ptr to get the address of bytes I need to look into PE Headers. pFileBase will have the bytes "MZ" but my shared_ptr is not coming back with those bytes. What am I missing?

Is there a way to have WinAPI functions return into a smart pointer? I'm also aware my shared_ptr is not char[] so that is my next step on fixing.

BOOL InitializeFromDisk(std::wstring &wsTempPath, char *pFileBase)
{
 ...
 pFileBase = (PCHAR)MapViewOfFile(hFileMapping, FILE_MAP_READ, 0, 0, 0);
 if (pFileBase == 0) return FALSE;
 return TRUE;
}
int main()
{
 std::shared_ptr<char> pFile = std::make_shared<char>(0);
 InitializeFromDisk(L"c:\\...", pFile.get());
 ...
 PIMAGE_DOS_SIGNATURE pDosHdr;
 std::copy(pFile, 2, pDosHdr); //I'm sure this line doesn't quit work yet 
}
NathanOliver
  • 171,901
  • 28
  • 288
  • 402
A C
  • 53
  • 1
  • 5
  • A valuable reference: http://en.cppreference.com/w/cpp/memory – Jesper Juhl Jul 25 '17 at 19:18
  • You'll need to use a regular pointer, get the address, and then construct a `unique_ptr` from that pointer (you'll need to supply a custom deleter as well). Also `InitializeFromDisk` needs to take the pointer by reference otherwise the call site won't see the change of the pointer's value. – NathanOliver Jul 25 '17 at 19:53
  • @NathanOliver Unless specifically called for, you should never deallocate memory using standard [tag:c++] methods that was allocated by the [tag:win32] API. They represent fundamentally different layers of the hardware. – MooseBoys Jul 25 '17 at 20:02
  • 1
    @MooseBoys That is why I said *you'll need to supply a custom deleter as well*. `UnmapViewOfFile` should be called once the OP is done with the pointer so if you do that in the custom deleter you no longer have to remember to do that. – NathanOliver Jul 25 '17 at 20:04
  • @NathanOliver [tag:win32] APIs that return plain pointers are exceedingly rare. It's probably not a good idea to take a dependency on `stl` smart pointers in such code if it only solves a small fraction of the use cases you'll need to cover. – MooseBoys Jul 25 '17 at 20:10
  • Possible duplicate of [How to use C++ standard smart pointers with Windows HANDLEs?](https://stackoverflow.com/questions/9842938/how-to-use-c-standard-smart-pointers-with-windows-handles) – zett42 Jul 25 '17 at 20:15
  • 1
    I wouldn't use a smart pointer for this. I'd wrap the API in a class and take advantage of RAII. The handle or whatever would be acquired in the constructor and released in the destructor, and if anything needed to be wrapped in a smart pointer, it would be this class. – user4581301 Jul 25 '17 at 21:03

2 Answers2

2

I might do something like this. Smart pointers have move constructors, so it's pretty efficient to return them, and doing so also yields better code. Note the use of the deleter argument in the shared_ptr constructor.

std::shared_ptr<VOID> InitializeFromDisk(const std::wstring& wsTempPath, char *pFileBase)
{
    ...
    auto pMappedFile = MapViewOfFile(hFileMapping, FILE_MAP_READ, 0, 0, 0);
    if (pMappedFile == nullptr)
    {
        auto lastError = GetLastError();
        throw system_error(lastError, system_category());
    }
    return shared_ptr<VOID>(pMappedFile, [](auto p) { UnmapViewOfFile(p); });
}
Michael Gunter
  • 12,528
  • 1
  • 24
  • 58
  • 1
    `throw system_error(GetLastError(), system_category());` ... is somewhat brittle as you have no guarantee that `system_category()` won't change the last error value. – zett42 Jul 25 '17 at 20:53
  • The C++ spec may make no guarantee of this (because it doesn't know anything about `GetLastError`), but I can assure you that no version of Microsoft's C++ implementation would ever set the last error value in `system_category()`. Feel free to dig through the code. It's in `C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\include\system_error` – Michael Gunter Jul 25 '17 at 21:18
  • 1
    What guarantees do you have that Microsoft will never *change* this behaviour? Apart from that, readers of your answer may take it as an example that can be applied to similar cases, which would be dangerous, because this code is **not** safe in general. – zett42 Jul 26 '17 at 06:22
0

In , most APIs do not return objects as plain memory. Instead, they usually return either HANDLEs or object instances that derive from IUnknown. To free the memory associated with a HANDLE, you will usually call CloseHandle. To free the memory associated with an IUnknown object, call ->Release(). Some allocations require special release calls. In your example, the pointer returned by MapViewOfFile must be deallocated with UnmapViewOfFile.

For the more common types of objects, smart pointer wrappers are implemented in the Microsoft::WRL library. For example:

Microsoft::WRL::ComPtr<ID3D12Device> spDevice;
D3D12CreateDevice(..., IID_PPV_ARGS(&spDevice));

Microsoft::WRL::Wrappers::FileHandle shFile;
shFile = CreateFile2(...);

Both spDevice and shFile will be appropriately deallocated upon going out of scope.

For other allocations, like the returned MapViewOfFile pointer, you'll have to create your own smart pointer/handle class.

MooseBoys
  • 6,641
  • 1
  • 19
  • 43
  • You can pass a delete function into the smart pointer constructor. This is a better solution than creating a custom smart pointer class. – Michael Gunter Jul 25 '17 at 20:02
  • @MichaelGunter `std::shared_ptr` etc. are poorly suited to [tag:win32] style allocations. While it *kind of* works for the `MapViewOfFile` case, most allocations (e.g. `HANDLE`s) aren't actually pointers at all. – MooseBoys Jul 25 '17 at 20:05
  • 3
    With the deleter, all things are possible. If you supply a deleter (see my answer), the `shared_ptr` isn't going to `delete` -- instead, it's going to invoke the deleter, which allows you to tap into any deallocation method you need to. – Michael Gunter Jul 25 '17 at 20:08
  • @MichaelGunter While it's of course possible to construct a custom `shared_ptr` that doesn't actually hold a pointer, it's an abuse of the object's semantics. You could just as easily construct a `shared_ptr` that started playing audio through the system speakers, and stops the sound upon the last instance going out of scope. It doesn't mean it's a good idea... – MooseBoys Jul 25 '17 at 20:15
  • Most handles are just typedefs for `void*` so of course you can [use standard smart pointers to manage them](https://stackoverflow.com/a/9843284/7571258). – zett42 Jul 25 '17 at 20:17
  • Well, a `HANDLE` _is_ a pointer. Specifically, it's a `void*`. So a `shared_ptr` or `shared_ptr` would work. The only real argument against using them this way is that is loses some type safety -- given a `shared_ptr`, you can't just know whether the contained pointer represents an `HWND`, `HANDLE`, `HMODULE` or something else entirely like the result of `MapViewOfFile` – Michael Gunter Jul 25 '17 at 20:17
  • @MichaelGunter Semantic preservation is *precisely* the reason to not do this. There's a reason there are some 40 odd instances of `typedef HANDLE HSomeObject` in [tag:win32]; it's because they impart semantics to the object. – MooseBoys Jul 25 '17 at 20:23
  • Well, you can do the same for smart pointers: `using SharedHWND = std::shared_ptr`. – zett42 Jul 25 '17 at 20:26
  • 1
    The majority of handles in Win32 are actually typed struct pointers. There are relatively few handle types these days that are simply typedefs of HANDLEs. Contrary to my previous comment, an `HWND`, for example, is a `struct HWND__*`. So you could preserve type safety by using that struct -- an `HWND` would be stored in a `shared_ptr`. – Michael Gunter Jul 25 '17 at 20:29
  • 1
    @MichaelGunter That's an implementation detail. I think `std::shared_ptr::type>` would be more portable. – zett42 Jul 25 '17 at 20:49
  • @MichaelGunter that is only true when `STRICT` is defined, otherwise they are untyped `void*` pointers. – Remy Lebeau Jul 25 '17 at 21:53