-
January 25th, 2010, 10:50 AM
#1
memory leaks
Hi,
do you think the following code could lead to memory leaks?
Code:
#define _CRT_SECURE_NO_WARNINGS
#include <windows.h>
#include <vector>
#include <cstdlib>
#include <ctime>
#include <cstdio>
#include <boost/circular_buffer.hpp>
using namespace std;
using namespace boost;
char * getDateTime(void);
const short numbuff = 5;
const short buflen = 30;
typedef struct
{
unsigned char * pData;
unsigned short bufferLength;
unsigned short bytesRecorded;
bool flag;
} Buffer;
int main()
{
circular_buffer<Buffer*> cb(numbuff);
circular_buffer<Buffer*>::const_iterator it;
// fill buffer
for(int i = 0; i<numbuff; i++)
{
// set up buffer
Buffer *buff = new Buffer;
ZeroMemory(buff, sizeof(Buffer));
buff->bufferLength = buflen;
buff->bytesRecorded = buflen;
buff->flag = true;
buff->pData = new unsigned char[buflen];
buff->pData = reinterpret_cast<unsigned char *>(getDateTime());
// push buffer
cb.push_back(buff);
Sleep(1000);
}
// show elements
for(int i = 0; i<(int)cb.size(); i++)
{
printf("%s\n", cb[i]->pData);
}
system("pause");
return EXIT_SUCCESS;
}
// getDateTime (Fri, 10 Oct 2008 14:41:59 GMT)
char * getDateTime(void)
{
time_t rawtime;
struct tm * timeinfo;
time(&rawtime);
timeinfo = gmtime(&rawtime);
char * buffer = new char[30];
strftime(buffer,30,"%a, %d %b %Y %X GMT",timeinfo);
return buffer;
}
I cannot delete "buff" since its scope is inside the first for loop...
-
January 25th, 2010, 11:06 AM
#2
Re: memory leaks
Hi,
do you think the following code could lead to memory leaks? it creates a
circualr buffer made up of 5 element then it pushes 10 elements (overwriting
the first 5) when overwriting , do you think the eldest pointed memory will
be overwritten, or a new pointer in memory will be written?
Code:
#define _CRT_SECURE_NO_WARNINGS
#include <windows.h>
#include <vector>
#include <cstdlib>
#include <ctime>
#include <cstdio>
#include <boost/circular_buffer.hpp>
using namespace std;
using namespace boost;
char * getDateTime(void);
const short numbuff = 5;
const short buflen = 30;
typedef struct
{
unsigned char * pData;
unsigned short bufferLength;
unsigned short bytesRecorded;
bool flag;
} Buffer;
int main()
{
circular_buffer<Buffer*> cb(numbuff);
circular_buffer<Buffer*>::const_iterator it;
printf("Push elements:\n");
// fill buffer
for(int i = 0; i<10; i++)
{
// set up buffer
Buffer *buff = new Buffer;
ZeroMemory(buff, sizeof(Buffer));
buff->bufferLength = buflen;
buff->bytesRecorded = buflen;
buff->flag = true;
buff->pData = new unsigned char[buflen];
buff->pData = reinterpret_cast<unsigned char *>(getDateTime());
printf("%s\n", buff->pData);
// push buffer
cb.push_back(buff);
Sleep(1000);
}
printf("\nShow elements:\n");
// show elements
for(int i = 0; i<static_cast<int>(cb.size()); i++)
{
printf("%s\n", cb[i]->pData);
}
system("pause");
return EXIT_SUCCESS;
}
// getDateTime (Fri, 10 Oct 2008 14:41:59 GMT)
char * getDateTime(void)
{
time_t rawtime;
struct tm * timeinfo;
time(&rawtime);
timeinfo = gmtime(&rawtime);
char * buffer = new char[30];
strftime(buffer,30,"%a, %d %b %Y %X GMT",timeinfo);
return buffer;
}
// thanks
-
January 25th, 2010, 11:15 AM
#3
Re: memory leaks
buff->pData = new unsigned char[buflen];
buff->pData = reinterpret_cast<unsigned char *>(getDateTime());
This leaks. pData is overwritten the 2nd time so the memory you created the 1st time is gone.
Also... who frees the memory that 'getDateTime' has created.
-
January 25th, 2010, 11:20 AM
#4
Re: memory leaks
All that you newed you *must* delete. Period.
If you don't delete what you have newed then you produce memory leaks.
Victor Nijegorodov
-
January 25th, 2010, 11:24 AM
#5
Re: memory leaks
Originally Posted by Skizmo
This leaks. pData is overwritten the 2nd time so the memory you created the 1st time is gone.
Also... who frees the memory that 'getDateTime' has created.
by the way, how can I change getDateTime() to return char[30] ?? I mean the return type...
thanks
-
January 25th, 2010, 11:25 AM
#6
Re: memory leaks
Originally Posted by VictorN
All that you newed you *must* delete. Period.
If you don't delete what you have newed then you produce memory leaks.
Ok. As I was suggested in this very forum...I am going to use vector to collect chars...is that ok? and no pointers anymore...
-
January 25th, 2010, 11:29 AM
#7
Re: memory leaks
Originally Posted by THEARTOFWEB
by the way, how can I change getDateTime() to return char[30] ?? I mean the return type...
thanks
For example:
Code:
void getDateTime( char *buffer)
{
time_t rawtime;
struct tm * timeinfo;
time(&rawtime);
timeinfo = gmtime(&rawtime);
strftime(buffer,30,"%a, %d %b %Y %X GMT",timeinfo);
}
Now you can:
Code:
char buffer[30];
getDateTime(buffer);
Victor Nijegorodov
-
January 25th, 2010, 11:30 AM
#8
Re: memory leaks
Originally Posted by THEARTOFWEB
...I am going to use vector to collect chars...is that ok? and no pointers anymore...
Yes, it is definetly a very good idea!
Victor Nijegorodov
-
January 25th, 2010, 11:48 AM
#9
Re: memory leaks
For instance...do you think the following could be better approach?
Code:
struct Buffer
{
public:
vector<unsigned char> vChar;
unsigned int bufferLength;
unsigned int bytesRecorded;
Buffer() : bytesRecorded(0), bufferLength(0), vChar(NULL) { };
};
int main()
{
circular_buffer<Buffer> cb(numbuff);
circular_buffer<Buffer>::const_iterator it;
(...)
}
thanks
-
January 25th, 2010, 12:06 PM
#10
Re: memory leaks
anyway, I have a problem with assign a char to a vector:
Code:
void getDateTime(char * szTime);
const int numbuff = 5;
const int buflen = 30;
struct Buffer
{
public:
vector<char> vChar;
unsigned int bufferLength;
unsigned int bytesRecorded;
Buffer() : bytesRecorded(0), bufferLength(0), vChar(NULL) { };
};
int main()
{
circular_buffer<Buffer> cb(numbuff);
circular_buffer<Buffer>::const_iterator it;
for(int i = 0; i<10; i++)
{
// Get time
char szTime[30]; getDateTime(szTime);
// Init Buff
Buffer buff;
ZeroMemory(&buff, sizeof(Buffer));
buff.vChar.resize(buflen);
buff.vChar = szTime;
buff.bufferLength = buflen;
buff.bytesRecorded = buflen;
printf("%s\n", buff.vChar);
}
system("pause");
return EXIT_SUCCESS;
}
// getDateTime (Fri, 10 Oct 2008 14:41:59 GMT)
void getDateTime(char * szTime)
{
time_t rawtime = time(NULL);
struct tm timeinfo;
gmtime_s(&timeinfo, &rawtime);
strftime(szTime, 30, "%a, %d %b %Y %X GMT", &timeinfo);
}
the code fails here:
Code:
buff.vChar = szTime;
why?
thanks
-
January 25th, 2010, 12:08 PM
#11
Re: memory leaks
Because szTime is not a char
Victor Nijegorodov
-
January 25th, 2010, 12:12 PM
#12
Re: memory leaks
do you mean szTime is several chars ??
-
January 25th, 2010, 12:15 PM
#13
Re: memory leaks
No, it is a pointer to a NULL-terminated string (character array)
Victor Nijegorodov
-
January 25th, 2010, 12:29 PM
#14
Re: memory leaks
well, I thought this was a stand-alone string (chars array) not a pointer:
Code:
char szTime[30]; getDateTime(szTime);
I tried using memcpy but it's not working...
-
January 25th, 2010, 12:38 PM
#15
Re: memory leaks
Originally Posted by THEARTOFWEB
well, I thought this was a stand-alone string (chars array) not a pointer:
Code:
char szTime[30]; getDateTime(szTime);
I tried using memcpy but it's not working...
First, change the function getDateTime() to return a string, not a char*.
Code:
// getDateTime (Fri, 10 Oct 2008 14:41:59 GMT)
std::string getDateTime(void)
{
time_t rawtime;
struct tm * timeinfo;
time(&rawtime);
timeinfo = gmtime(&rawtime);
char buffer[30];
strftime(buffer,30,"%a, %d %b %Y %X GMT",timeinfo);
return buffer;
}
There really isn't a reason to resort to char* and new[] to create strings in a C++ application.
Regards,
Paul McKenzie
Posting Permissions
- You may not post new threads
- You may not post replies
- You may not post attachments
- You may not edit your posts
-
Forum Rules
|
Click Here to Expand Forum to Full Width
|