Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C++ > memory leaks

Reply
Thread Tools

memory leaks

 
 
Larry
Guest
Posts: n/a
 
      01-25-2010
"SG" <(E-Mail Removed)> ha scritto nel messaggio
news:(E-Mail Removed)...

> Yes. In this case it's fairly easy to determine: You used the "new"-
> operator three times but never the "delete"-operator and never passed
> on the responsibility for managing the object life times to other
> functions or objects.


Ok! do you think the following may still 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

 
Reply With Quote
 
 
 
 
Thomas J. Gritzan
Guest
Posts: n/a
 
      01-25-2010
Am 25.01.2010 19:42, schrieb Larry:
> struct Buffer
> {
> public:
> vector<char> vChar;
> int bufferLength;
> int bytesRecorded;
> int user;
> Buffer() : bytesRecorded(0), bufferLength(0), user(0) { };
> };
>
> int main()
> {
> circular_buffer<Buffer> cb(numbuff);
> circular_buffer<Buffer>::const_iterator it;
>
> // Insert elements
> for(int i = 0; i<10; i++)
> {
> // Get time
> char szTime[30]; getDateTime(szTime);


Use your constants:
char szTime[buflen];

> // Init Buff
> Buffer buff;
> copy(&szTime[0],&szTime[30],std::back_inserter(buff.vChar));


This also works:
buff.vChar.assign(szTime, szTime+buflen);

> //buff.vChar.assign(szTime, szTime+strlen(szTime));
> buff.bufferLength = buflen;
> buff.bytesRecorded = buflen;
> buff.user = i;
>
> printf("%d, %d, %s\n", buff.user, buff.bufferLength, szTime);
>
> 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].vChar);
> }

[...]
> in the second loop I get <null> when I try to print vChar


Of course. You try to output a std::vector through "%s", but a vector
isn't a C-style string. Learn to use C++ facilities:

std::cout << cb[i].user << ", " << cb[i].bufferLength << ", " <<
cb[i].vChar << std::endl;

This doesn't compile either, which is good, because is gives you an
error message, while the same with printf() just silently compiles and
yields undefined behaviour. So compiler-errors are good,
this-just-doesnt-work-and-I-dont-know-why is very bad.

Since you are storing a C-style string in the vector, you can do this to
output the string (works also with printf):
std::cout << &cb[i].vChar[0];

--
Thomas
 
Reply With Quote
 
 
 
 
Alf P. Steinbach
Guest
Posts: n/a
 
      01-25-2010
* Larry:
> "Alf P. Steinbach" <(E-Mail Removed)> ha scritto nel messaggio
> news:hjki9g$rc0$(E-Mail Removed)-september.org...
>> * Larry:
>>> "Alf P. Steinbach" <(E-Mail Removed)> ha scritto nel messaggio

>
>> However, if this is just raw data, consider
>>
>> typedef unsigned char Byte;
>> typedef vector<Byte> ByteVector;

>
> the new struct should look like this:
>
> struct Buffer
> {
> public:
> vector<unsigned char> vuChar;
> int bufferLength;
> int bytesRecorded;
> int user;
> Buffer() : bytesRecorded(0), bufferLength(0), vChar(NULL), user(0) { };
> };


More like


typedef unsigned char Byte;

struct Something
{
vector<Byte> bytes;
int userId;
// Constructor etc.
};

> what the difference between:
> --> vector<unsigned char> vuChar;
> and
> --> unsigne char buffer[]


The declaration

unsigned char buffer[51];

says that 'buffer' is a contiguous area of memory consisting of 51 elements each
of type 'unsigned char'.

The declaration

vector<unsigned char> v( 51 );

says that 'v' is a 'vector' which internally holds a pointer to a buffer, and
that on construction it should create a buffer of 51 elements. This can be
resized (if necessary the vector will discard the original buffer and copy
everything over to a and sufficiently larger buffer). And you can assign to 'v'
and generally copy 'v', and be sure that there is no memory leak.


Cheers & hth.,

- Alf
 
Reply With Quote
 
Larry
Guest
Posts: n/a
 
      01-25-2010
"Thomas J. Gritzan" <(E-Mail Removed)> ha scritto nel messaggio
news:hjkru0$o2i$(E-Mail Removed)...

>> struct Buffer
>> {
>> public:
>> vector<char> vChar;
>> int bufferLength;
>> int bytesRecorded;
>> int user;
>> Buffer() : bytesRecorded(0), bufferLength(0), user(0) { };
>> };


So I think the following is the only way not to have memory leaks:

#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 Buffer
{
public:
char payload[4096];
int bytesRecorded;
int user;
Buffer() : 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(static_cast<void*>(buff.payload), static_cast<void*>(szTime),
buflen);
buff.user = i;
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);
}

where payload is set to 4096 bytes long. Indeed it's going to hold less data
but this way I can make sure it can hold from 0 to 4095 bytes...the other
filed in the struct will store how long payload actually is!

thanks

 
Reply With Quote
 
Rolf Magnus
Guest
Posts: n/a
 
      01-26-2010
Larry wrote:

>> Yes. In this case it's fairly easy to determine: You used the "new"-
>> operator three times but never the "delete"-operator and never passed
>> on the responsibility for managing the object life times to other
>> functions or objects.

>
> Ok! do you think the following may still lead to memory leaks?


Since that code doesn't allocate memory anymore, it can't leak memory. But
you are making other severe memory access errors.

> #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;


Note that szTime is local to the loop. It stops existing once the current
loop iteration is done.

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


Here you're accessing objects that don't exist anymore. Anything can happen
here. Why are you using raw arrays of char instead of std::string?
Memory allocation is an advanced topic, so avoid doing it yourself and let
the standard classes handle it for you. That will make it a lot easier.

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


 
Reply With Quote
 
Nick Keighley
Guest
Posts: n/a
 
      01-26-2010
On 25 Jan, 16:14, "Alf P. Steinbach" <(E-Mail Removed)> wrote:
> * Larry:


> > #define _CRT_SECURE_NO_WARNINGS

>
> Don't do that.


why?

It suppresses some irritating MS warnings. Are you saying don't
suppress the warnings or don't embed such MSisms in your source (for
instance use something in the build system like
-D?)
 
Reply With Quote
 
Richard Herring
Guest
Posts: n/a
 
      01-26-2010
In message <4b5df6a2$0$1141$(E-Mail Removed)>, Larry
<(E-Mail Removed)> writes
>"Thomas J. Gritzan" <(E-Mail Removed)> ha scritto nel messaggio
>news:hjkru0$o2i$(E-Mail Removed)...
>
>>> struct Buffer
>>> {
>>> public:
>>> vector<char> vChar;
>>> int bufferLength;
>>> int bytesRecorded;
>>> int user;
>>> Buffer() : bytesRecorded(0), bufferLength(0), user(0) { };
>>> };

>
>So I think the following is the only way not to have memory leaks:


[...]
>
>struct Buffer
>{
>public:
>char payload[4096];


[...]
>
>where payload is set to 4096 bytes long. Indeed it's going to hold less
>data but this way I can make sure it can hold from 0 to 4095
>bytes...the other filed in the struct will store how long payload
>actually is!


<sigh>

And when you change the program so it needs 4097 bytes, you will get
undefined behaviour. What is so hard about using vector or string to
manage that memory, as everyone else is advising you?

USE A VECTOR.

--
Richard Herring
 
Reply With Quote
 
James Kanze
Guest
Posts: n/a
 
      01-27-2010
On Jan 25, 7:42 pm, "Alf P. Steinbach" <(E-Mail Removed)> wrote:
> * Larry:
> > "Alf P. Steinbach" <(E-Mail Removed)> ha scritto nel messaggio
> >news:hjki9g$rc0$(E-Mail Removed)-september.org...

[...]
> More like


> typedef unsigned char Byte;


> struct Something
> {
> vector<Byte> bytes;
> int userId;
> // Constructor etc.
> };


> > what the difference between:
> > --> vector<unsigned char> vuChar;
> > and
> > --> unsigne char buffer[]


> The declaration


> unsigned char buffer[51];


> says that 'buffer' is a contiguous area of memory consisting
> of 51 elements each of type 'unsigned char'.


> The declaration


> vector<unsigned char> v( 51 );


> says that 'v' is a 'vector' which internally holds a pointer
> to a buffer, and that on construction it should create a
> buffer of 51 elements. This can be resized (if necessary the
> vector will discard the original buffer and copy everything
> over to a and sufficiently larger buffer). And you can assign
> to 'v' and generally copy 'v', and be sure that there is no
> memory leak.


In fact, "vector< unsigned char >" is full-fledged type, which
behaves like any other type, where as "unsigned char buffer[ 51 ]"
creates an object of a second class type, which is fairly
broken, and doesn't behave normally.

Also, vector< unsigned char > will normally do bounds checking,
and other important things to avoid undefined behavior.

There are, of course, cases where something like "unsigned char
buffer[ 51 ]" is justified, but they aren't that common.

--
James Kanze
 
Reply With Quote
 
Alf P. Steinbach
Guest
Posts: n/a
 
      01-27-2010
* James Kanze:
> On Jan 25, 7:42 pm, "Alf P. Steinbach" <(E-Mail Removed)> wrote:
>> * Larry:
>>> "Alf P. Steinbach" <(E-Mail Removed)> ha scritto nel messaggio
>>> news:hjki9g$rc0$(E-Mail Removed)-september.org...

> [...]
>> More like

>
>> typedef unsigned char Byte;

>
>> struct Something
>> {
>> vector<Byte> bytes;
>> int userId;
>> // Constructor etc.
>> };

>
>>> what the difference between:
>>> --> vector<unsigned char> vuChar;
>>> and
>>> --> unsigne char buffer[]

>
>> The declaration

>
>> unsigned char buffer[51];

>
>> says that 'buffer' is a contiguous area of memory consisting
>> of 51 elements each of type 'unsigned char'.

>
>> The declaration

>
>> vector<unsigned char> v( 51 );

>
>> says that 'v' is a 'vector' which internally holds a pointer
>> to a buffer, and that on construction it should create a
>> buffer of 51 elements. This can be resized (if necessary the
>> vector will discard the original buffer and copy everything
>> over to a and sufficiently larger buffer). And you can assign
>> to 'v' and generally copy 'v', and be sure that there is no
>> memory leak.

>
> In fact, "vector< unsigned char >" is full-fledged type, which
> behaves like any other type, where as "unsigned char buffer[ 51 ]"
> creates an object of a second class type, which is fairly
> broken, and doesn't behave normally.


I probably agree with what you mean, but the wording above might give the wrong
impression to the OP or other readers.

The type of "unsigned char buffer[52]" is technically a full-fledged type. It's
just that, as you note, this type is broken, due to C compatibility. So with
respect to what one can do and with respect to type safety it's not quite up to par.

In particular, regarding technical type full fledgeness (not you don't know
this, but to avoid any misunderstanding), given

unsigned char buffer[51];

one might apply typeid(), or let some templated function infer the type,
whatever -- with respect to the C++ type system there are no not full fledged
types except incomplete types (types where sizeof cannot be applied) and perhaps
to some degree local classes because they don't have linkage and so do not go
well with templating.


> Also, vector< unsigned char > will normally do bounds checking,
> and other important things to avoid undefined behavior.
>
> There are, of course, cases where something like "unsigned char
> buffer[ 51 ]" is justified, but they aren't that common.


Yes, agreed.


Cheers,

- Alf
 
Reply With Quote
 
Richard Herring
Guest
Posts: n/a
 
      01-27-2010
In message <4b600a1d$0$828$(E-Mail Removed)>, io_x
<(E-Mail Removed)> writes
>
>"James Kanze" <(E-Mail Removed)> ha scritto nel messaggio
>news:(E-Mail Removed)...
>> Also, vector< unsigned char > will normally do bounds checking,
>> and other important things to avoid undefined behavior.

>
>this seems untrue for std::vector<T> data_;
>for T == struct Color3d{double r, g, b;};
>
>
>#include <iostream>
>#include <cstdlib>
>#include <stdint.h>
>#include <vector>
>
>#define u8 uint8_t
>#define i8 int8_t
>#define u32 uint32_t
>#define i32 int32_t
>// float 32 bits
>#define f32 float
>
>#define S sizeof
>#define R return
>#define P printf
>#define F for


Sorry, I stopped reading here. That kind of macro abuse merely makes the
rest of your code unreadable.

What point were you trying to make?

--
Richard Herring
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Memory Leaks in ASP.NET =?Utf-8?B?RnJhbmsxMjEz?= ASP .Net 12 03-17-2013 10:03 AM
Our memory leaks? James Hunter Ross ASP .Net 2 10-20-2005 08:25 PM
ASP.NET - Detecting memory leaks ASP.Confused ASP .Net 2 07-16-2004 04:24 PM
memory leaks with firebird? astromannix Firefox 2 07-26-2003 12:10 AM
do self references cause memory leaks in Java? Novice Java 28 07-22-2003 02:25 PM



Advertisments