memory leaks

[THEARTOFWEB]

Ok. I have given up vectors for the moment...I'd liek to know if the following code may lead to memory leaks:

#include <windows.h>
#include <vector>
#include <cstdlib>
#include <ctime>
#include <cstdio>
#include <boost/circular_buffer.hpp>
using namespace std;
using namespace boost;

void getDateTime(char * szTime);
const int numbuff = 5;
const int buflen  = 30;

struct Buffer
{
public:
	char * payload;
	int bufferLength;
	int bytesRecorded;
	int user;
	Buffer() : bytesRecorded(0), bufferLength(0), user(0), payload(NULL) { };
};

int main()
{
	circular_buffer<Buffer> cb(numbuff);

	// Insert elements
	for(int i = 0; i<10; i++)
	{
		// Get time
		char szTime[30]; getDateTime(szTime);

		// Init Buff
		Buffer buff;
		buff.user          = i;
		buff.payload       = szTime;
		buff.bufferLength  = buflen;
		buff.bytesRecorded = buflen;

		cb.push_back(buff);
	}

	// Show elements:
	for(int i = 0; i<(int)cb.size(); i++)
	{
		printf("%d, %d, %s\n", cb[i].user, cb[i].bufferLength, cb[i].payload);
	}

	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);
}

thanks

[VictorN]

No, because you don't allocate anything in the heap (neither with new nor with malloc nor with any other means)

[hoxsiew]

No memory leaks that I see, but certainly undefined behavior as your "char szTime" goes out of scope when your for loop finishes.

Why not declare your "struct Buffer" with a fixed size "payload":

struct Buffer
{
public:
	char payload[30];
	int bufferLength;
	int bytesRecorded;
	int user;
	Buffer() : bytesRecorded(0), bufferLength(0), user(0), payload(NULL) { };
};

Edit: remove the "payload(NULL)" form your constructor too.

[THEARTOFWEB]

Ok, so all I can do is the following:

#include <windows.h>
#include <vector>
#include <cstdlib>
#include <ctime>
#include <cstdio>
#include <boost/circular_buffer.hpp>
using namespace std;
using namespace boost;

void getDateTime(char * szTime);
const int numbuff = 3;
const int buflen  = 30;

struct Buffer
{
public:
	char payload[30];
	int bufferLength;
	int bytesRecorded;
	int user;
	Buffer() : bufferLength(0), bytesRecorded(0), user(0) { }
};

int main()
{
	circular_buffer<Buffer> cb(numbuff);

	// Insert elements
	printf("Push elements:\n");
	for(int i = 0; i<5; i++)
	{
		// Get time
		char szTime[30]; getDateTime(szTime);

		// Init Buff
		Buffer buff;
		ZeroMemory(&buff, sizeof(Buffer));

		memcpy((void*)buff.payload, szTime, buflen);
		buff.user          = i;
		buff.bufferLength  = buflen;
		buff.bytesRecorded = buflen;

		cb.push_back(buff);

		printf("%s\n", buff.payload);
		Sleep(1000);
	}

	// Show elements:
	printf("Show elements:\n");
	for(int i = 0; i<(int)cb.size(); i++)
	{
		printf("%s\n", cb[i].payload);
	}

	system("pause");
	return EXIT_SUCCESS;
}

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);
}

so I will set payload to contains a loto of chars like:

char payload[4096]

then if payload is less the 4096 bytes long I will zero fill or read only a part of that

[hoxsiew]

I have no idea what it is you are doing. I thought the time data was the payload. If your payload is variable with each buffer, then it is pretty inefficient to use a fixed size payload.

[Joeman]

Looks like you aren't taking advantage of c++ at all. this looks like c with classes approach with boost?

#include <windows.h>

#include <ctime>
#include <string>
#include <iostream>
#include <boost/circular_buffer.hpp>
#include <boost/foreach.hpp>

using namespace std;
using namespace boost;

string getDateTime()
{
	time_t rawtime = time(NULL);
	tm* timeinfo = gmtime(&rawtime);
	char szTime[30];
	strftime(szTime, 30, "%a, %d %b %Y %X GMT", timeinfo);
	return szTime;
}
 
struct Buffer 
{
	string payload;
	int user;
	Buffer() : payload(), user() { }
	Buffer( const string& payload, int user ) : payload(payload), user(user) { }
};
 
int main()
{
	const int numbuff = 3;
	circular_buffer<Buffer> cb(numbuff);
 
	// Insert elements
	cout << "Push elements:" << endl;
 
	for( int i = 0; i < 5; i++ )
	{
		cb.push_back( Buffer( getDateTime(), i ) );
		Sleep(1000);
	}
 
	// Show elements:
	cout << "Show elements:" << endl;
 
	BOOST_FOREACH( Buffer Record, cb )
	cout << Record.payload << endl;
 
	system("pause");
	return 0;
}

This is more c++ than the c approach , but understanding how and why to use string, constructors, containers and etc will take time. If this isn't what you require, just state what is wrong with the above code.

[THEARTOFWEB]

I have no idea what it is you are doing. I thought the time data was the payload. If your payload is variable with each buffer, then it is pretty inefficient to use a fixed size payload.

even if payload is big enough (fixed) to store a variable size?

[THEARTOFWEB]

This is more c++ than the c approach , but understanding how and why to use string, constructors, containers and etc will take time. If this isn't what you require, just state what is wrong with the above code.

I might deal with /char/ (or unsigned char) data returned by some other functions....

I know I could cast to char to string...Yet, I'd like to stick with char for the moment

[THEARTOFWEB]

I mean, binary string mostly (unsigned char) the getDateTime() is just a way to fill the buffer with something for the moment...in the near future I'll be dealing with binary string only in my application

[THEARTOFWEB]

This is more c++ than the c approach , but understanding how and why to use string, constructors, containers and etc will take time. If this isn't what you require, just state what is wrong with the above code.

I think for the moment I'll use this:

#include <windows.h>
#include <cstdlib>
#include <ctime>
#include <cstdio>
#include <cstring>
#include <boost/circular_buffer.hpp>
using namespace std;
using namespace boost;

void getDateTime(char * szTime);
const int numbuff = 3;
const int buflen  = 30;

struct cBuffer
{
	char data[1024];
	int bytesRecorded;
	bool flag;
	cBuffer(char * data_, int bytesRecorded_, bool flag_) :
	bytesRecorded(bytesRecorded_), flag(flag_)
	{
		memcpy(static_cast<void *>(data), static_cast<void *>(data_), bytesRecorded);
	}
};


int main()
{
	circular_buffer<cBuffer> cb(numbuff);

	// Insert elements
	printf("Push elements:\n");
	for(int i = 0; i < 5; i++)
	{
		char szTime[30]; getDateTime(szTime);

		cb.push_back(cBuffer(szTime, 30, true));

		printf("%s\n", szTime);

		Sleep(1000);
	}

	// Show elements:
	printf("Show elements:\n");
	for(int i = 0; i<(int)cb.size(); i++)
	{
		printf("%s\n", cb[i].data);
	}

	system("pause");
	return EXIT_SUCCESS;
}

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);
}

this time I think I won't be having any memory leaks!

[Joeman]

memcpy(static_cast<void *>(data), static_cast<void *>(data_), bytesRecorded);

This line can be wrong if compiled when char != 1.

memcpy(static_cast<void *>(data), static_cast<void *>(data_), bytesRecorded * sizeof(char));

Binary String? you mean like "01010101". Shouldn't that fit into chars? you can also cast char to unsigned char as well.

typedef basic_string<unsigned char> ustring;

I think for the moment I'll use this:

At least it looks better