Impossible Leak realloc

E

Eitan Michaelson

Hi,

Can any one tell me what's wrong with this code?

It leaks (acceding to bound checker), when it attempts to reallocate memory.

The code is not pure C, but with minor adjustments any C compiler would
compile this.



Thank you

Eitan Michaelson.



////////////////////////////////////////////////// Code Starts Here
///////////////////////////////////////////////////////////////////////



// This code attempts to create a pointer to a structure,and then allocate a
buffer for that structure, so it would hold an array of structures.

// in each loop I’m trying to reallocate the structure array so it would
grow by 1, and then I'm trying to add data into it.

// one of the structure elements is a void*, which in turn is allocated by
malloc.




#include <stdio.h>
#include <malloc.h>
#include <string.h>

#define MAX_STRUCT_SIZE (50)
struct st_Test
{
char szChar[32];
int iInt;
void *pVoid;
};
void main(void)
{
// Pointer to the structure
st_Test *pst_Test;
pst_Test = NULL; // pointer not valid yet
for(int i = 0;i <=MAX_STRUCT_SIZE;i++)
{
if(!pst_Test) // not yet allocated
pst_Test = (st_Test*)malloc(1 * sizeof(st_Test));
else // Resize by 1
pst_Test = (st_Test*)realloc(pst_Test,(i+1)
*sizeof(st_Test)); /// This is the Leak /////////
// Reset the buffer we got
memset(&pst_Test,'\0',sizeof(st_Test));
// Add some data to the allocated struct
pst_Test.iInt = 64738;
strcpy(pst_Test.szChar,"poke 53280,1");
// allocate buffer for the void* element ////// This is
where bound checker found the problem ///
if(!pst_Test.pVoid)
pst_Test.pVoid = (char*)malloc(32);
strcpy((char*)pst_Test.pVoid,"commodre 64");
}

////////////////////////////////////////////////// End
///////////////////////////////////////////////////////////////////////
 
K

Ken Hagan

Eitan said:
Hi,

Can any one tell me what's wrong with this code?

It leaks (acceding to bound checker), when it attempts to reallocate
memory.

The code you posted never calls free() so presumably everything is
leaked. You need to free the pVoid members of all the elements and
then free pst_Text itself.

(Note: My newsreader couldn't find alt.comp.lang.c so I've dropped it.)
 
A

Andy Sawyer

Hi,

Can any one tell me what's wrong with this code?

It leaks (acceding to bound checker), when it attempts to reallocate
memory.

There isn't a single call to free in your code, so of course it leaks.
 
C

CBFalconer

Eitan said:
Can any one tell me what's wrong with this code?

It leaks (acceding to bound checker), when it attempts to
reallocate memory.

It contains invalid includes <malloc.h> and invalid (for C99, and
for display in a message) // comment forms. The indentation is
excessive. You should first convert it to a compilable form, in
standard C, and then post it. Ensure that lines do not exceed 65
characters in general, but anything of 80 or more characters does
not belong in newsgroups.

Why should we wade through a mess that you have created?

USE worldnet address!
 
B

Brett Frankenberger

#include <stdio.h>
#include <malloc.h>
#include <string.h>

#define MAX_STRUCT_SIZE (50)
struct st_Test
{
char szChar[32];
int iInt;
void *pVoid;
};
void main(void)
{
// Pointer to the structure
st_Test *pst_Test;
pst_Test = NULL; // pointer not valid yet
for(int i = 0;i <=MAX_STRUCT_SIZE;i++)
{
if(!pst_Test) // not yet allocated
pst_Test = (st_Test*)malloc(1 * sizeof(st_Test));
else // Resize by 1
pst_Test = (st_Test*)realloc(pst_Test,(i+1)
*sizeof(st_Test)); /// This is the Leak /////////
// Reset the buffer we got
memset(&pst_Test,'\0',sizeof(st_Test));
// Add some data to the allocated struct
pst_Test.iInt = 64738;
strcpy(pst_Test.szChar,"poke 53280,1");
// allocate buffer for the void* element ////// This is
where bound checker found the problem ///
if(!pst_Test.pVoid)
pst_Test.pVoid = (char*)malloc(32);


Why do you test pst_Test[1].pVoid for NULL before allocating memory for
it? Neither malloc or realloc makes any guarantees about the contents
of the memory it allocates. So pst_Test.pVoid is unitilialized.
The act of testing it for NULL in the if statement above is undefined.
If it happens to be initialized to NULL, then your program will
probably work; if it happens to be initialized to something else, then
the if will probably fail and the strcpy below will likely end up
copying to some random memory location.
strcpy((char*)pst_Test.pVoid,"commodre 64");
}


-- Brett (Still remembers what poke 53280,1 does on a Commodore 64)
 
D

Dan Pop

In <[email protected]> Eitan Michaelson
The code is not pure C, but with minor adjustments any C compiler would
compile this.

Then, why didn't *you* make those minor adjustments, so that any C
compiler would compile it?

Don't expect much help when deliberately posting broken C code. Not
from the C newsgroups, anyway.

Dan
 
G

goose

Eitan Michaelson said:
Hi,

Can any one tell me what's wrong with this code?

It leaks (acceding to bound checker), when it attempts to reallocate memory.

The code is not pure C, but with minor adjustments any C compiler would
compile this.

you know this and yet you still post ?
<grin>
heathen!!!

seriously, make it "pure C" and repost ... I'm sure that a lot more
people will look at it then ...

#include <stdio.h>
#include <malloc.h>

I dont know why you include malloc.h
#include <string.h>

#define MAX_STRUCT_SIZE (50)
struct st_Test
{
char szChar[32];
int iInt;
void *pVoid;
};
void main(void)

its supposed to be
int main (void)
{
// Pointer to the structure

comments of this form are illegal, use
/* comments like this */ (unless you are using
a c99 compiler)
st_Test *pst_Test;

should be
struct st_Test *pst_Test;
pst_Test = NULL; // pointer not valid yet
for(int i = 0;i <=MAX_STRUCT_SIZE;i++)
{
if(!pst_Test) // not yet allocated
pst_Test = (st_Test*)malloc(1 *
sizeof(st_Test));

why the cast ? did the compiler beat you over the head when you
left it out ? do you think perhaps that the cast is hiding problems ?

also, why do you have 1 * sizeof (st_Test), when sizeof st_Test
(or even better, sizeof *pst_Test) would do ?
else // Resize by 1

you forgot to make sure that malloc returned a pointer to valid
memory. what happens if there is no more memory ?
pst_Test = (st_Test*)realloc(pst_Test,(i+1)
*sizeof(st_Test)); /// This is the Leak /////////

a). maybe some clean indentation would help your (and others)
comprehension.
b). you are not making sure that realloc returned memory to you.
you must *always* check the return values of malloc/realloc/etc.
c). stop the casting.
d). those are not comments.
// Reset the buffer we got
memset(&pst_Test,'\0',sizeof(st_Test));
// Add some data to the allocated struct
pst_Test.iInt = 64738;
strcpy(pst_Test.szChar,"poke 53280,1");

<ot>
<grin>
this isn't sposed to run on a c64, right ?
// allocate buffer for the void* element ////// This is
where bound checker found the problem ///
if(!pst_Test.pVoid)


what is this test supposed to achieve ?
pst_Test.pVoid = (char*)malloc(32);


lose the casts.
strcpy((char*)pst_Test.pVoid,"commodre 64");
}

////////////////////////////////////////////////// End

///////////////////////////////////////////////////////////////////////

ok, so where is the code that frees all the memory ? no wonder
its leaking ...

how about you work on the code till it compiles as a clean
C proggy, then repost, and we'll try to spot the leak ?

goose,
professional layabout and arcade-game player,
poke 53281,0 :)
 
V

Vinayak Raghuvamshi

Eitan Michaelson said:
Hi,

Can any one tell me what's wrong with this code?

It leaks (acceding to bound checker), when it attempts to reallocate memory.

The code is not pure C, but with minor adjustments any C compiler would
compile this.


#include <stdio.h>
#include <malloc.h>
#include <string.h>

#define MAX_STRUCT_SIZE (50)
struct st_Test
{
char szChar[32];
int iInt;
void *pVoid;
};
void main(void)
{
// Pointer to the structure
st_Test *pst_Test;
pst_Test = NULL; // pointer not valid yet
for(int i = 0;i <=MAX_STRUCT_SIZE;i++)
{
if(!pst_Test) // not yet allocated
pst_Test = (st_Test*)malloc(1 * sizeof(st_Test));
else // Resize by 1
pst_Test = (st_Test*)realloc(pst_Test,(i+1)
*sizeof(st_Test)); /// This is the Leak /////////
// Reset the buffer we got
memset(&pst_Test,'\0',sizeof(st_Test));


Unless I am missing something, I think the leak is obvious.
where are you freeing ur mallocated memory ?

-Vinayak
 
B

bd

Hi,

Can any one tell me what's wrong with this code?

It leaks (acceding to bound checker), when it attempts to reallocate memory.

The code is not pure C, but with minor adjustments any C compiler would
compile this.

The only problem I see is void main - and this should be fixed.
Thank you

Eitan Michaelson.



////////////////////////////////////////////////// Code Starts Here
///////////////////////////////////////////////////////////////////////



// This code attempts to create a pointer to a structure,and then allocate a
buffer for that structure, so it would hold an array of structures.

// in each loop I’m trying to reallocate the structure array so it would
grow by 1, and then I'm trying to add data into it.

// one of the structure elements is a void*, which in turn is allocated by
malloc.




#include <stdio.h>
#include <malloc.h>
#include <string.h>

#define MAX_STRUCT_SIZE (50)
struct st_Test
{
char szChar[32];
int iInt;
void *pVoid;
};
void main(void)

int main(void)
{
// Pointer to the structure
st_Test *pst_Test;
pst_Test = NULL; // pointer not valid yet
for(int i = 0;i <=MAX_STRUCT_SIZE;i++)
{
if(!pst_Test) // not yet allocated
pst_Test = (st_Test*)malloc(1 * sizeof(st_Test));
else // Resize by 1
pst_Test = (st_Test*)realloc(pst_Test,(i+1)
*sizeof(st_Test)); /// This is the Leak /////////

You never free pst_Test.
// Reset the buffer we got
memset(&pst_Test,'\0',sizeof(st_Test));


This is not a standard way to clear it. How about:
pst_Test.pVoid = pst_Test.iInt = pst_Test.szChar[0] = 0;
It'll probably be faster, too. In fact, since you reset everything in
the
code below, it's totally unnecessary.
// Add some data to the allocated struct
pst_Test.iInt = 64738;
strcpy(pst_Test.szChar,"poke 53280,1");
// allocate buffer for the void* element ////// This is
where bound checker found the problem ///
if(!pst_Test.pVoid)
pst_Test.pVoid = (char*)malloc(32);


You never free that. Therefore, it's a leak. See below for the solution.
strcpy((char*)pst_Test.pVoid,"commodre 64");
}

for(int i = 0; i <= MAX_STRUCT_SIZE; i++){
free(pst_Test.pVoid);
}
free(pst_test);
return 0;
}
 
B

Balog Pal

[mods: why do you approve completely offtopic messages?]
{Sometimes we do by accident, but this thread is not one of those. The
OP's code is C++ even if it makes no use of specifically C++ rather than
C functionality. -mod}


Man, this code has nothing to do with C++. And even as C it looks pretty
ugly.
It leaks (acceding to bound checker), when it attempts to reallocate
memory.

You leak memory when you allocate a block and later you do not free it.
[using either free() or realloc(p, 0)]

Your code has not a single free operation, so how you expect it could *NOT*
leak memory?

Paul
 
E

Emmanuel Delahaye

[assuming C]
Can any one tell me what's wrong with this code?

It leaks (acceding to bound checker), when it attempts to reallocate
memory.

The code is not pure C, but with minor adjustments any C compiler would
compile this.


// This code attempts to create a pointer to a structure,and then
allocate a buffer for that structure, so it would hold an array of
structures.

// in each loop I'm trying to reallocate the structure array so it
would grow by 1, and then I'm trying to add data into it.

// one of the structure elements is a void*, which in turn is allocated
by malloc.


#include <stdio.h>
#include <malloc.h>

Non standard header. You want said:
#include <string.h>

#define MAX_STRUCT_SIZE (50)

Sounds more like an ARRAY_SIZE...
struct st_Test
{
char szChar[32];
int iInt;
void *pVoid;
};
void main(void)
{
// Pointer to the structure
st_Test *pst_Test;

st_Test is not defined. If it works with you, you are probably not using a C
compiler. You know, C and C++ are very different language, despite the
appearences.

struct st_Test *pst_Test;
pst_Test = NULL; // pointer not valid yet

In that case, why not doing it in one gulp:

struct st_Test *pst_Test = NULL;
for(int i = 0;i <=MAX_STRUCT_SIZE;i++)

Are you sure you are using a C compiler? You code is full of c++ism. It's
boring. (Note you can only use this construct in C with a C99 compiler).
{
if(!pst_Test) // not yet allocated
pst_Test = (st_Test*)malloc(1 *
sizeof(st_Test));

Again these casts are c++ism... Get rid of them. NEVER compile C code with a
compiler for another language.

#ifdef __cplusplus
#error Wrong compiler. Use a C one.
#endif

What is the point of multiplying something by 1 ? Note that realloc() with
NULL acts like malloc(). You don't need this 'special case'. realloc()
handkes it already.
else // Resize by 1
pst_Test = (st_Test*)realloc(pst_Test,(i+1)
*sizeof(st_Test)); /// This is the Leak /////////

Please avoid to write scary code. Stay simple:

pst_Test = realloc(pst_Test, (i + 1) * sizeof *pst_Test);

but as indicated in the FAQ, realloc() can fail. In that case, you need a
temporary pointer.

struct st_Test* p = realloc(pst_Test, (i + 1) * sizeof *pst_Test);

if (p)
{
pst_Test = p;
}
else
{
free (p);
}
// Reset the buffer we got
memset(&pst_Test,'\0',sizeof(st_Test));


memset(pst_Test + i, 0, sizeof *pst_Test);

Note that setting all bits to 0 os not the good wauy a setting the elements
of a structure to 0. You should use 'zero' reference structure of initialize
the elements one by one.
// Add some data to the allocated struct
pst_Test.iInt = 64738;
Ok.

strcpy(pst_Test.szChar,"poke 53280,1");


Ok, assuming the string is not too large.

*pst_Test.szChar = 0;
strncat(pst_Test.szChar
, "poke 53280,1"
, sizeof pst_Test.szChar - 1);

is safe (and easily 'macroizable' if you find it gory). No need for bound
checker now!
// allocate buffer for the void* element ////// This is
where bound checker found the problem ///
if(!pst_Test.pVoid)


pVoid was not properly set to NULL.
pst_Test.pVoid = (char*)malloc(32);
strcpy((char*)pst_Test.pVoid,"commodre 64");


Beware. The strcpy() should also be under the control of the if (). Always
use the {} with code structures. It helps readbility and maintenance.

missing

return 0;
}

Try that:

#include <stdlib.h>

#include <stdio.h>
#include <string.h>

#define MAX_STRUCT_SIZE (50)
struct st_Test
{
char szChar[32];
int iInt;
void *pVoid;
};

int main (void)
{
/* Pointer to the structure */
/* pointer not valid yet */
struct st_Test *pst_Test = NULL;
int i;

for (i = 0; i <= MAX_STRUCT_SIZE; i++)
{
/* Resize by 1 */
struct st_Test *p = realloc (pst_Test, (i + 1) * sizeof *pst_Test);
if (p)
{
pst_Test = p;

/* Reset the buffer we got */
{
static struct st_Test const z =
{
{0},
0,
0,
};

*p = z;
}

/* Add some data to the allocated struct */
p->iInt = 64738;

*p->szChar = 0;
strncat (p->szChar, "poke 53280,1", sizeof p->szChar - 1);

/* allocate buffer for the void* element */
if (!p->pVoid)
{
p->pVoid = malloc (32);
strcpy (p->pVoid, "commodre 64");
}
}
else
{
/* memory problem */
free (pst_Test);
break;
}
}

return 0;
}
 
K

kanze

Can any one tell me what's wrong with this code?
It leaks (acceding to bound checker), when it attempts to reallocate memory.
The code is not pure C, but with minor adjustments any C compiler
would compile this.

It's not legal C++ either, but with a fair number of adjustments...
// This code attempts to create a pointer to a structure,and then
// allocate a buffer for that structure, so it would hold an array of
// structures.
// in each loop I??m trying to reallocate the structure array so it
// would grow by 1, and then I'm trying to add data into it.
// one of the structure elements is a void*, which in turn is
// allocated by malloc.
#include <stdio.h>
#include <malloc.h>

There is no such header in C or in C++.
#include <string.h>
#define MAX_STRUCT_SIZE (50)
struct st_Test
{
char szChar[32];
int iInt;
void *pVoid;
};
void main(void)

In C++, at least, main *must* return an int. In C it normally returns
an int as well, but I think the C standard allows a conformant compiler
to accept a void return as an extension. As an extension -- "void main"
is not standard conforming C.
{
// Pointer to the structure
st_Test *pst_Test;

This line shouldn't pass a C complier, although it is legal C++. If you
wrote:

struct st_Test* pst_Test ;

it would be legal in both languages.
pst_Test = NULL; // pointer not valid yet
for(int i = 0;i <=MAX_STRUCT_SIZE;i++)
{
if(!pst_Test) // not yet allocated
pst_Test = (st_Test*)malloc(1 * sizeof(st_Test));
else // Resize by 1
pst_Test = (st_Test*)realloc(pst_Test,(i+1)
*sizeof(st_Test)); /// This is the Leak /////////

You don't need the test -- realloc is guaranteed to work correctly if
passed a NULL pointer.
// Reset the buffer we got
memset(&pst_Test,'\0',sizeof(st_Test));


I'm not sure what you are trying to do here, but you initialize the
pVoid element to a possibly invalid value, both in C and in C++.

If you want the equivalent of zero initialization, the standard idiom
would be to define a static object, and assign from it:

static struct st_Test zeroInitialized ;
pst_Test[ i ] = zeroInitialized ;
// Add some data to the allocated struct
pst_Test.iInt = 64738;
strcpy(pst_Test.szChar,"poke 53280,1");
// allocate buffer for the void* element ////// This is
where bound checker found the problem ///
if(!pst_Test.pVoid)


This line contains undefined behavior. Both in C and in C++.
pst_Test.pVoid = (char*)malloc(32);
strcpy((char*)pst_Test.pVoid,"commodre 64");
}

////////////////////////////////////////////////// End
///////////////////////////////////////////////////////////////////////

Of course, since you never free any memory, it is normal that you have a
memory leak. The code returns from main, and the only pointer to your
structures disappears, so any checker will call it a leak.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,904
Latest member
HealthyVisionsCBDPrice

Latest Threads

Top