returning pointer to a struct, how can I free memory automatically?

Question

To return a big struct "MODULEENTRY32" from WINAPI I want to use a pointer, but need to allocate memory in the heap inside the function without deleting it. Then, out of the function when I don't want to use that struct anymore I know that I should use the keyword delete to free memory.

#include <iostream>
#include <cstring>
#include <Windows.h>
#include <TlHelp32.h>

MODULEENTRY32* GetModuleEntry(const char *module_name, int PID)
{
    HANDLE moduleSnapshot = CreateToolhelp32Snapshot(TH32CS_SNAPMODULE | TH32CS_SNAPMODULE32, PID);

    if (moduleSnapshot != INVALID_HANDLE_VALUE)
    {
        MODULEENTRY32 *moduleEntry = new MODULEENTRY32;             // Remember to delete if don't want leak
        moduleEntry->dwSize = sizeof(MODULEENTRY32);

        if (Module32First(moduleSnapshot, moduleEntry)) {
            do {
                if (strcmp(moduleEntry->szModule, module_name) == 0) {
                    return moduleEntry;
                }
            } while (Module32Next(moduleSnapshot, moduleEntry));
        }

        CloseHandle(moduleSnapshot);
    }

    return nullptr;
}

int main()
{
    int myRandomPID = 123;

    MODULEENTRY32* p = GetModuleEntry("mymodule.dll", myRandomPID);

    if (!p) {
        std::cout << "Obviously you didn't found your random module of your random PID " << std::endl;
    }

    delete p;   // I just don't want to do this

    return 0;
}

How could I avoid having to free memory in main function? unique_ptr?

EDIT: Possible solution

#include <iostream>
#include <cstring>
#include <Windows.h>
#include <TlHelp32.h>

bool GetModuleEntry(const char *module_name, int PID, MODULEENTRY32* moduleEntry)
{
    HANDLE moduleSnapshot = CreateToolhelp32Snapshot(TH32CS_SNAPMODULE | TH32CS_SNAPMODULE32, PID);

    if (moduleSnapshot != INVALID_HANDLE_VALUE)
    {
        moduleEntry->dwSize = sizeof(MODULEENTRY32);

        if (Module32First(moduleSnapshot, moduleEntry)) {
            do {
                if (strcmp(moduleEntry->szModule, module_name) == 0) {
                    CloseHandle(moduleSnapshot);
                    return true;
                }
            } while (Module32Next(moduleSnapshot, moduleEntry));
        }

        CloseHandle(moduleSnapshot);
    }

    return false;
}

int main()
{
    int myRandomPID = 123;

    MODULEENTRY32 moduleEntry;

    if (!GetModuleEntry("mymodule.dll", 123, &moduleEntry)) {
        std::cout << "Obviously you didn't find your random module of your random PID " << std::endl;
    }

    return 0;
}

Yes, `unique_ptr`. That's exactly the sole reason for its existence. Maybe `shared_ptr`, too. — Sam Varshavchik, Aug 07 '19 at 00:36
A struct is copyable as-is. There is no need in resorting to pointers -- just return the `struct`. Let me guess -- you're calling a function that requires a pointer to a `MODULEENTRY32`, so you assumed you *must* declare a pointer and use it, am I right? — PaulMcKenzie, Aug 07 '19 at 00:37
[`std::optional`](https://en.cppreference.com/w/cpp/utility/optional) is a useful addition to @PaulMcKenzie 's suggestion. For more reading, [What are copy elision and return value optimization?](https://stackoverflow.com/questions/12953127/what-are-copy-elision-and-return-value-optimization) — user4581301, Aug 07 '19 at 00:42
@OP [Are you making this mistake?](https://stackoverflow.com/questions/24472174/c-uninitialized-local-variable/24472249#24472249). Other than the link uses `DWORD`, it seems you're making every mistake pointed out in that answer. — PaulMcKenzie, Aug 07 '19 at 00:44
@PaulMcKenzie do you mean pass the struct as out parameter via pointer? — , Aug 07 '19 at 00:47
Note that the [Microsoft documentation](https://learn.microsoft.com/en-us/windows/win32/toolhelp/traversing-the-module-list) even shows to do without _creating_ a pointer — Tas, Aug 07 '19 at 00:48
@PaulMcKenzie Well really... how I didn't guess that? thanks I usually pass a lot of out parameters via pointer but this time I only wonder about return the pointer. — , Aug 07 '19 at 00:50
Do not return the pointer -- actually return the full struct. A `struct` is copyable. Or return a type (such as pointed out by @user4581301) that contains a full struct (a `std::pair` or similar). Contrary to what many newbies believe, you can pass and return `structs` around by value -- all the copy semantics will hold. — PaulMcKenzie, Aug 07 '19 at 00:52
Also, it looks like you left the function without closing the handle. Sounds like a reason to also use RAII. — PaulMcKenzie, Aug 07 '19 at 00:59
@PaulMcKenzie yes but woudn't be better to declare a struct MODULEENTRY32 in main, pass it's address to the function GetModuleEntry("mymodule.dll", myRandomPID, &module) and then work directly with that inside the function and returning a bool or nothing (void). — , Aug 07 '19 at 01:00
@PaulMcKenzie yes I forget CloseHandle(moduleSnapshot) inside the conditional. — , Aug 07 '19 at 01:02
If the function returns a `bool`, you have to return `true` or `false` -- there is no "void" return if the function returns `bool`. — PaulMcKenzie, Aug 07 '19 at 01:03
Please see my answer. Changing the function parameters is a matter of taste, but the bottom line is that you do not need dynamic allocation whatsoever. — PaulMcKenzie, Aug 07 '19 at 01:20
@PaulMcKenzie ok Thanks Paul I didn't know anyting about **copy elision**, it seems not exensive at all. The only think that I don't like is the std::pair. — , Aug 07 '19 at 01:29
@EduardoG The `std::pair` technique is used in a few STL functions. For example `std::map::insert`, `std::unordered_map::insert`, and a few others utilize this technique of returning a success code and an item. — PaulMcKenzie, Aug 07 '19 at 01:31
With C++17 structured bindings the `std::pair` thing gets even nicer. You can do something like `auto [success, moduleEntry] = GetModuleEntry(...);` — Miles Budnek, Aug 07 '19 at 03:30
@MilesBudnek, I don't like C++11, 14 and 17, prefer to stay close to C-Style. — , Aug 07 '19 at 06:05

score 0 · Accepted Answer · answered Aug 07 '19 at 01:15

I suggest that you refrain from pointers, and simply return the full MODULEENTRY32 struct by value.

I also suggest that you can wrap the return type in a std::pair<bool, MODULEENTRY32> or similar type, so that you can record an error return if necessary.

Lastly, you are not closing the handle if you find the module name. A little bit of RAII could be used to ensure that the HANDLE is closed.

#include <iostream>
#include <cstring>
#include <Windows.h>
#include <TlHelp32.h>
#include <utility>

// Helper struct to automatically close the handle
struct ModuleRAII
{
    HANDLE *h;
    ModuleRAII(HANDLE* h_) : h(h_){}
    ~ModuleRAII() { CloseHandle(h); }
};

std::pair<bool, MODULEENTRY32> GetModuleEntry(const char *module_name, int PID)
{
    HANDLE moduleSnapshot = CreateToolhelp32Snapshot(TH32CS_SNAPMODULE | TH32CS_SNAPMODULE32, PID);

    if (moduleSnapshot != INVALID_HANDLE_VALUE)
    {
        // This will close the handle automatically
        ModuleRAII raii(&moduleSnapshot);

        MODULEENTRY32 moduleEntry = {};
        moduleEntry.dwSize = sizeof(MODULEENTRY32);
        if (Module32First(moduleSnapshot, &moduleEntry)) {
        do {
            if (strcmp(moduleEntry.szModule, module_name) == 0) 
                  return {true, moduleEntry};
           } while (Module32Next(moduleSnapshot, &moduleEntry));
        }
    }
    return {false, MODULEENTRY32()};
}

int main()
{
    int myRandomPID = 123;
    auto retVal = GetModuleEntry("mymodule.dll", myRandomPID);
    MODULEENTRY32& p = retVal.second;    
    if (!retVal.first) {
        std::cout << "Obviously you didn't found your random module of your random PID " << std::endl;
    }

    return 0;
}

This basically is a fail-safe implementation of the function. It hasn't been tested, but shows the various techniques used, all without dynamic allocation.

Note that the function returns a std::pair<bool, MODULEENTRY>, so basically the parameters remain the same. The only difference is that you obtain the information by accessing either first (to test whether the function failed or not), and second, for the actual MODULEENTRY.

score 0 · Answer 2 · answered Aug 17 '19 at 19:58

Solution:

Return a unique_ptr from the getModuleEntry function itself.

unique_ptr<MODULEENTRY32> GetModuleEntry(const char *module_name, int PID)
{
    HANDLE moduleSnapshot = CreateToolhelp32Snapshot(TH32CS_SNAPMODULE | TH32CS_SNAPMODULE32, PID);

    if (moduleSnapshot != INVALID_HANDLE_VALUE)
    {
        unique_ptr<MODULEENTRY32> moduleEntry = make_unique<MODULEENTRY32>();             // Remember to delete if don't want leak
        moduleEntry->dwSize = sizeof(MODULEENTRY32);

        if (Module32First(moduleSnapshot, moduleEntry.get())) {
            do {
                if (strcmp(moduleEntry->szModule, module_name) == 0) {
                    return moduleEntry;
                }
            } while (Module32Next(moduleSnapshot, moduleEntry.get());
        }

        CloseHandle(moduleSnapshot);
    }

    return nullptr;
}
int main()
{
    int myRandomPID = 123;
    std::unique_ptr<MODULEENTRY32> s_ptr = GetModuleEntry("mymodule.dll", myRandomPID);

    if(s_ptr.get()==nullptr)
    {
        std::cout << "Obviously you didn't found your random module of your random PID " << std::endl;
    }

    return 0;
}

returning pointer to a struct, how can I free memory automatically?

2 Answers2