My first public solution

Started by
24 comments, last by sasho648 11 years, 6 months ago

You need to get rid of the space between delete and (.


This doesn't work for me. Can you give me an example code or explaining me more clearly.
Advertisement
Just a shot in the dark, but it's possible that the macro is expanding indefinitely, since the macro definition contains a reference to itself. It may be why memory leak detection in Visual Studio uses two macros instead of the one:


#define DBG_NEW new ( _NORMAL_BLOCK, __FILE__, __LINE__ )
#define new DBG_NEW


So your delete may look something like:

#define DBG_DELETE delete ( __FILE__, __LINE__ )
#define delete DBG_DELETE
This doesn't work. sad.png
I believe that you can overload the delete operator to take additional parameters but it cannot be called except for one particular situation: when the constructor fails for an object allocated with a new taking multiple parameters, the corresponding delete with multiple parameters are called. Otherwise, a delete with custom parameter cannot be called.

Thus, if you overload delete like you do, the only way to have it called is if new succeeds but the constructor of the allocated object fails.
Ok I found a way to pass the arguments in the delete operator by a global variables like this:

[source]#define DELETE_OLD delete
#define delete 0; while(bIsDeleteParamsPassing); bIsDeleteParamsPassing=true; chFile=(char*)__FILE__; inLineNo=__LINE__; DELETE_OLD[/source]

Here is the final version:

https://rapidshare.com/files/476519966/MemoryHandler.cpp

https://rapidshare.com/files/493613952/MemoryHandler.hpp

and example for used this (cmd) in main.cpp

https://rapidshare.com/files/1234352939/main.cpp
Ugh... rapidshare...


Ok I found a way to pass the arguments in the delete operator by a global variables like this:

[source]#define DELETE_OLD delete
#define delete 0; while(bIsDeleteParamsPassing); bIsDeleteParamsPassing=true; chFile=(char*)__FILE__; inLineNo=__LINE__; DELETE_OLD[/source]

Please don't.

#define DBG_DELETE delete(__FILE__, __LINE__)
#define delete DBG_DELETE

[s]should work[/s] (edit: turns out C++ doesn't support such syntax). If it doesn't, people can help you fix it... you didn't post the actual error when you said "This doesn't work." If you do, we can help you figure out what's up with it.

If you want some reasoning why I say the above code is bad, all you have to do is look at [font=courier new,courier,monospace]bIsDeleteParamsPassing[/font]... is that supposed to be some kind of mutex? You realize it doesn't work to protect your code in a multithreaded environment, right? I'm also unsure of why the [font=courier new,courier,monospace]0;[/font] is there...

And just a few more critiques on your code:

  • Be consistent with the std namespace. Sometimes you just say [font=courier new,courier,monospace]size_t[/font] instead of [font=courier new,courier,monospace]std::size_t[font=arial,helvetica,sans-serif]. You should always use the fully qualified [font=courier new,courier,monospace]std::[/font] prefix in header files (and also in source files, unless you do [font=courier new,courier,monospace]using namespace std[/font]).[/font][/font]
  • If you're going to use global variables (which you shouldn't be), put them in a namespace
  • Be consistent in your type names. Sometimes you use [font=courier new,courier,monospace]unsigned int[/font], sometimes [font=courier new,courier,monospace]UINT[/font]
  • Consider using proper types for booleans (i.e. use [font=courier new,courier,monospace]false[/font] instead of [font=courier new,courier,monospace]0[/font] when assigning to a [font=courier new,courier,monospace]bool[/font])
[size=2][ I was ninja'd 71 times before I stopped counting a long time ago ] [ f.k.a. MikeTacular ] [ My Blog ] [ SWFer: Gaplessly looped MP3s in your Flash games ]
Thank yoy very much for the critiques, this is exactly what I need because I learn c++ only from books and internet. The error behind "This doesn't work." is :
||=== Memw, Debug ===|
C:\Projects\Memw\main.cpp||In function 'int main()':|
C:\Projects\Memw\main.cpp|21|warning: left operand of comma operator has no effect [-Wunused-value]|
C:\Projects\Memw\main.cpp|21|error: type 'int' argument given to 'delete', expected pointer|
C:\Projects\Memw\main.cpp|21|error: expected ';' before 'my'|
||=== Build finished: 2 errors, 2 warnings ===|

at code:


using namespace std;
struct Some {
Some() {std::cout<<"constructor" <<std::endl;}
~Some() {std::cout<<"destructor" <<std::endl;}
unsigned long hash;
unsigned long hash1;
} ;
int main()
{
Some* my=new Some[2];
cout << "Hello world!" << endl;
delete my; //line 21
#ifdef DEBUG
EndOfProgramMemCheck();
#endif
system("PAUSE");
return 0;
}



with defined

#define DBG_DELETE delete(__FILE__, __LINE__)
#define delete DBG_DELETE


This means that the delete define expect pointer, not function call.

At Last: Can you give me better idea for the multithreaded environment importing (from bIsDeleteParamsPassing)?
So here is my last update of this solution. Now it really support multithreading ( I use __thread variables ). Here is the source:

MemoryHandler.cpp


#include <iostream>
#include <windows.h>
#include <cstring>
#include <cstdio>
struct MemoryAllocList {
char* File;
unsigned int Line;
MemoryAllocList* next;
void* MemoryPointer;
} ;
struct LineFile {
unsigned int nLineNo;
char* szFile;
} ;
MemoryAllocList* & GetMemoryList()
{
static __thread MemoryAllocList* pAllocList(0);
return pAllocList;
}
MemoryAllocList** & GetMemoryCurrPtrList()
{
static __thread MemoryAllocList** ppCurAllocList(0);
return ppCurAllocList;
}
LineFile & GetDeleteParams()
{
static __thread LineFile Obj;
return Obj;
}
void* operator new(size_t size, const char* szFile, unsigned int nLineNo) _GLIBCXX_THROW (std::bad_alloc)
{
void* pMem;
while(true) {
pMem=malloc(size ? size : 1);
if(!pMem)
{
#ifdef WIN32_MEM_LOG
MessageBoxA(0, "No more memory to use!", "Warning!", MB_OK | MB_ICONWARNING);
#endif
#ifndef WIN32_MEM_LOG
std::cerr << "No more memory to use!" << "Warning!" << std::endl;
#endif
std::new_handler cach=std::set_new_handler(0);
if(cach) (*cach)();
else throw std::bad_alloc();
std::set_new_handler(cach);
}
else
break;
}
MemoryAllocList** & ppCurAllocList=GetMemoryCurrPtrList();
if(!ppCurAllocList) //Thread first new call
{
ppCurAllocList=&GetMemoryList();
}
while(*ppCurAllocList);
(*ppCurAllocList)=(MemoryAllocList*)malloc(sizeof(MemoryAllocList));
if(!(*ppCurAllocList))
{
#ifdef WIN32_MEM_LOG
MessageBoxA(0, "No more memory to use for the debug version of MemoryHandler. Try compiling the app \
in release mode. It will exit now!", "Error!", MB_OK | MB_ICONERROR);
#endif
#ifndef WIN32_MEM_LOG
std::cerr << "No more memory to use for the debug version of MemoryHandler. Try compiling the app \
in release mode. It will exit now!" << "Error!" << std::endl;
#endif
exit(1);
}
(*ppCurAllocList)->Line=nLineNo;
(*ppCurAllocList)->File=(char*)szFile;
(*ppCurAllocList)->MemoryPointer=pMem;
(*ppCurAllocList)->next=0;
ppCurAllocList=&(*ppCurAllocList)->next;
return pMem;
}
void* operator new[](size_t size, const char* szFile, unsigned int nLineNo) _GLIBCXX_THROW (std::bad_alloc)
{
return operator new(size, szFile, nLineNo);
}
void operator delete[](void* pMem) _GLIBCXX_USE_NOEXCEPT
{
operator delete(pMem);
}
bool PassDeleteParams (const char* szFile, unsigned int nLineNo)
{
LineFile & Obj(GetDeleteParams());
Obj.nLineNo=nLineNo;
Obj.szFile=(char*)szFile;
return false;
}
void operator delete(void* pMem) _GLIBCXX_USE_NOEXCEPT
{
LineFile CurrPos(GetDeleteParams());
if(!pMem) return;
MemoryAllocList* sourc(GetMemoryList());
while(sourc&&sourc->MemoryPointer!=pMem)
sourc=sourc->next;
if(!sourc)
{
#ifdef WIN32_MEM_LOG
char str[80];
sprintf(str, "Invalid delete call at File: %s Line: %d", CurrPos.szFile,
CurrPos.nLineNo);
MessageBoxA(0, str, "Warning!", MB_OK | MB_ICONWARNING);
#endif
#ifndef WIN32_MEM_LOG
std::cerr << "Invalid delete call at File: " << CurrPos.szFile << " Line: " << CurrPos.nLineNo << " Warning!" << std::endl;
#endif
return;
}
sourc->MemoryPointer=0;
free(pMem);
return;
}
void EndThreadMemCheck() {
MemoryAllocList* pCurAllocTree(GetMemoryList());
while(pCurAllocTree) {
if(pCurAllocTree->MemoryPointer) {
#ifndef WIN32_MEM_LOG
std::cerr << "Object at line: " << pCurAllocTree->Line << " in file: "
<< pCurAllocTree->File << " wasn't deleted!" << std::endl;
#endif
#ifdef WIN32_MEM_LOG
char str[80];
sprintf(str, "Object at line: %d in file: %s wasn't deleted!", pCurAllocTree->Line,
pCurAllocTree->File);
MessageBoxA(0, str, "Memory leaked!", MB_OK | MB_ICONWARNING);
#endif
}
MemoryAllocList* cach(pCurAllocTree);
pCurAllocTree=pCurAllocTree->next;
free(cach);
}
}


MemoryHandler.hpp


#pragma once
/*Sasho 648 Memory leaks and inavlid delete calls finder (MultiThread support)
Set the MemoryHandler.cpp file to compile only in debug mode
and include the MemoryHandler.hpp only when the DEBUG symbol is defined. Then
at the end of a thread call EndThreadMemCheck() too when the DEBUG symbol is defined.
Define WIN32_MEM_LOG for Windows Messages.
*/
void* operator new(size_t size, const char* szFile, unsigned int nLineNo) _GLIBCXX_THROW (std::bad_alloc);
void* operator new[](size_t size, const char* szFile, unsigned int nLineNo) _GLIBCXX_THROW (std::bad_alloc);
void operator delete[](void* pMem) _GLIBCXX_USE_NOEXCEPT;
void operator delete(void* pMem) _GLIBCXX_USE_NOEXCEPT;
bool PassDeleteParams (const char* szFile, unsigned int nLineNo);
#define new new ( __FILE__ , __LINE__ )
#define DEL_OLD delete
#define delete if(PassDeleteParams(__FILE__, __LINE__)); else DEL_OLD
void EndThreadMemCheck() ;
You should wrap any statement-like macro into an single-looping do-while loop to ensure that the macro uses the exact same syntax and code paths as what it's trying to replace (the delete-statement in this case).

Consider things like this with your current code:

if(obj->isDead())
delete obj;
else
obj->update();


The standard procedure to wrap macros that require a semicolon at the end is like this:

#define FOO do { /* code here, single or multiline-code */ } while(false)

What this macro does is forcing the user to end the macro call with a semicolon, just like you would have to do with the normal delete statement. Leaving out the semicolon after the macro call will produce a compiler error; the semicolon is not optional anymore as with regular macros. Furthermore, it does not introduce the possibility of an additional code path, and can therefore be used safely in nested constructs like for-loops and if-statements without introducing unexpected and unintended code paths.

An extra bonus is that the do-while loop has its own "private" scope for you to define variables in without affecting outside scope.

Thank yoy very much for the critiques, this is exactly what I need because I learn c++ only from books and internet. The error behind "This doesn't work." is :
||=== Memw, Debug ===|
C:\Projects\Memw\main.cpp||In function 'int main()':|
C:\Projects\Memw\main.cpp|21|warning: left operand of comma operator has no effect [-Wunused-value]|
C:\Projects\Memw\main.cpp|21|error: type 'int' argument given to 'delete', expected pointer|
C:\Projects\Memw\main.cpp|21|error: expected ';' before 'my'|
||=== Build finished: 2 errors, 2 warnings ===|
[...]
with defined

#define DBG_DELETE delete(__FILE__, __LINE__)
#define delete DBG_DELETE



The first parameter for delete is a void*. That's what the message tells you. So the call to your overriden delete (which should probably look something like this:

void* operator delete(void*, const char* szFile, unsigned int nLineNo);

) you'd need to write

delete(myObject, __FILE__, __LINE__);

However your macro will do this:

delete myObject;
// will be expanded to
delete(__FILE__, __LINE__) myObject;

Apparently this is not correct.

I just checked, and even the

delete(myObject, __FILE__, __LINE__);

gives a compile error in visual studio ("Cannot delete objects that are no pointers") for whatever reason.

The code

delete(__FILE__, __LINE__, myObject);

Compiles but doesn't call my delete, but the default delete operator.

So anyway. Here's my suggestion:

#include <iostream>
class Foo {
public:
Foo() {}
};
void* operator new(size_t size, const char* f, unsigned int line) {
std::cout << "new: " << f << ":" << line << std::endl;
return malloc(size);
}
// just here to avoid the warning "'void *operator new(size_t,const char *,unsigned int)' : no matching operator delete found; memory will not be freed if initialization throws an exception"
void operator delete(void* ptr, const char* f, unsigned int line) {
std::cout << "delete: " << f << ":" << line << std::endl;
return free(ptr);
}
void Destroy(void* ptr, const char* f, unsigned int line) {
std::cout << "delete: " << f << ":" << line << std::endl;
return free(ptr);
}
#define DBG_NEW new(__FILE__, __LINE__)
#define new DBG_NEW
#define DBG_DELETE(p) Destroy(p, __FILE__, __LINE__)
#define delete(p) DBG_DELETE(p)
int main() {
Foo* myFoo = new Foo();
delete(myFoo);
return 0;
}

This topic is closed to new replies.

Advertisement