Search code examples
c++stackcorruption

C++ What's wrong with these functions?


This is a class I'm doing to organize every memory block I allocate using malloc/calloc, so I can easily dispose of them later. The following is the definition of this class:

#include "memmanager.h"

std::vector<void*> MemoryManager::GarbageCollector(0);

void MemoryManager::AddToGC(void* myThing)
{
    if(__DEBUG__MODE__)
        MainDebugger.Log("Adding 1 element to GC: %p", "Info", myThing);
    MemoryManager::GarbageCollector.push_back(myThing);
    return;
}

void MemoryManager::MultiAddToGC(int args, void* myThing, ...)
{
    if(args < 1)
        return;

#ifdef _DEBUG
        MainDebugger.Log("Adding %i element to GC", "Info", args);
#endif

    va_list chrstr;
    va_start(chrstr, args);

    for(int x = 0; x < args; x++)
        MemoryManager::GarbageCollector.push_back(va_arg(chrstr, void*));

    va_end(chrstr);
    return;
}

void MemoryManager::Flush()
{
    int lasterror = 0;

#ifdef _DEBUG
        MainDebugger.Log("Call to MemoryManager::Flush() with %i items in the GC", "Info", MemoryManager::GarbageCollector.size());
#endif

    for(unsigned int x = 0; x < MemoryManager::GarbageCollector.size(); x++)
    {
        errno = lasterror = 0;
        free(GarbageCollector[x]);
        lasterror = errno;
        if(lasterror > 0)
            MainDebugger.Log("MemoryManager::Flush() Error: %s (%i : %p)", "Error", strerror(lasterror), x, GarbageCollector[x]);
    }
    GarbageCollector.clear();
    return;
}

The problem seems to be with the function "MultiAddToGC". When I do this in my main file:

MemoryManager::MultiAddToGC(3,tst,mfile,testfile);
MemoryManager::Flush();

It works fine if I'm in Debug mode (I'm doing this in VC++ 2010). But if I change to Release mode, it gives me an error inside MemoryManager::Flush() while calling the free() function (something about the stack being corrupted). I can continue, if I continue, I get the following in my log:

15:12:26 f 00 (0 fps) | Error > MemoryManager::Flush() Error: Invalid argument (2 : 00D44784)

However, if I do this:

MemoryManager::AddToGC(tst);
MemoryManager::AddToGC(mfile);
MemoryManager::AddToGC(testfile);
MemoryManager::Flush();

It works both in Debug, and release. No errors. So I'm assuming the error is in MultiAddToGC(), but I can't find it. The following code is the header, "memmanager.h":

#ifndef __MEMMANAGER_H__
#define __MEMMANAGER_H__

#include <vector>
#include <malloc.h>
#include <stdarg.h>
#include "..\core.h"
#include "..\DEBUGGER\debugger.h"

extern bool __DEBUG__MODE__;
extern GameDebugger MainDebugger;

class MemoryManager {

public:
    static void MemoryManager::AddToGC(void*);
    static void MemoryManager::MultiAddToGC(int, void*,...);
    static void MemoryManager::Flush();
private:
    static std::vector<void*> GarbageCollector;

protected:

};

#endif

Any help/tips/advise is wellcome.


Solution

  • In va_start(chrstr, args);, the second parameter should be the last named parameter, so myThing (but you probably want to remove myThing in fact).

    but I would write

    template <typename...Ts>
    void MemoryManager::MultiAddToGC(Ts*... myThings)
    {
        if(sizeof...(myThings) < 1)
            return;
    
    #ifdef _DEBUG
        MainDebugger.Log("Adding %i elements to GC", "Info", sizeof...(myThings));
    #endif
        int dummy[] = {(MemoryManager::GarbageCollector.push_back(myThings), 0)...};
        (void) dummy; // avoid warning for unused variable
    }
    

    or simply

    void MemoryManager::MultiAddToGC(const std::initializer_list<void*>& myThings)
    {
        if(myThings.size() < 1)
            return;
    
    #ifdef _DEBUG
        MainDebugger.Log("Adding %i elements to GC", "Info", myThings.size());
    #endif
        MemoryManager::GarbageCollector.insert(MemoryManager::GarbageCollector.begin(),
                                               myThings.begin(), myThings.end());
    }