Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C++ > Impossible Leak realloc

Reply
Thread Tools

Impossible Leak realloc

 
 
Eitan Michaelson
Guest
Posts: n/a
 
      06-27-2003
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[i],'\0',sizeof(st_Test));
// Add some data to the allocated struct
pst_Test[i].iInt = 64738;
strcpy(pst_Test[i].szChar,"poke 53280,1");
// allocate buffer for the void* element ////// This is
where bound checker found the problem ///
if(!pst_Test[i].pVoid)
pst_Test[i].pVoid = (char*)malloc(32);
strcpy((char*)pst_Test[i].pVoid,"commodre 64");
}

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





[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
 
Reply With Quote
 
 
 
 
Ken Hagan
Guest
Posts: n/a
 
      06-27-2003
Eitan Michaelson wrote:
> 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.)



[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
 
Reply With Quote
 
 
 
 
Andy Sawyer
Guest
Posts: n/a
 
      06-27-2003
In article <bdgrml$drf$(E-Mail Removed)>,
on 27 Jun 2003 07:01:37 -0400,
Eitan Michaelson <(E-Mail Removed)> wrote:

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

--
"Light thinks it travels faster than anything but it is wrong. No matter
how fast light travels it finds the darkness has always got there first,
and is waiting for it." -- Terry Pratchett, Reaper Man

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
 
Reply With Quote
 
CBFalconer
Guest
Posts: n/a
 
      06-27-2003
Eitan Michaelson wrote:
>
> 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?

--
Chuck F ((E-Mail Removed)) ((E-Mail Removed))
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home.att.net> USE worldnet address!


[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
 
Reply With Quote
 
Brett Frankenberger
Guest
Posts: n/a
 
      06-27-2003
In article <bdgrml$drf$(E-Mail Removed)>,
Eitan Michaelson <(E-Mail Removed)> wrote:
>
>#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[i],'\0',sizeof(st_Test));
> // Add some data to the allocated struct
> pst_Test[i].iInt = 64738;
> strcpy(pst_Test[i].szChar,"poke 53280,1");
> // allocate buffer for the void* element ////// This is
>where bound checker found the problem ///
> if(!pst_Test[i].pVoid)
> pst_Test[i].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[i].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[i].pVoid,"commodre 64");
> }


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

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
 
Reply With Quote
 
Dan Pop
Guest
Posts: n/a
 
      06-27-2003
In <bdgrml$drf$(E-Mail Removed)> Eitan Michaelson
<(E-Mail Removed)> writes:

>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
--
Dan Pop
DESY Zeuthen, RZ group
Email: http://www.velocityreviews.com/forums/(E-Mail Removed)

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
 
Reply With Quote
 
goose
Guest
Posts: n/a
 
      06-27-2003
Eitan Michaelson <(E-Mail Removed)> wrote in message
news:<bdgrml$drf$(E-Mail Removed)>...
> 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 ...

<snipped>

>
> #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[i],'\0',sizeof(st_Test));
> // Add some data to the allocated struct
> pst_Test[i].iInt = 64738;
> strcpy(pst_Test[i].szChar,"poke 53280,1");

<ot>
<grin>
this isn't sposed to run on a c64, right ?
</ot>
> // allocate buffer for the void* element ////// This

is
> where bound checker found the problem ///
> if(!pst_Test[i].pVoid)


what is this test supposed to achieve ?

> pst_Test[i].pVoid = (char*)malloc(32);


lose the casts.

> strcpy((char*)pst_Test[i].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

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
 
Reply With Quote
 
Vinayak Raghuvamshi
Guest
Posts: n/a
 
      06-27-2003
Eitan Michaelson <(E-Mail Removed)> wrote in message
news:<bdgrml$drf$(E-Mail Removed)>...
> 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[i],'\0',sizeof(st_Test));


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

-Vinayak

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
 
Reply With Quote
 
bd
Guest
Posts: n/a
 
      06-27-2003
On Fri, 27 Jun 2003 07:01:37 -0400, Eitan Michaelson wrote:

> 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[i],'\0',sizeof(st_Test));


This is not a standard way to clear it. How about:
pst_Test[i].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[i].iInt = 64738;
> strcpy(pst_Test[i].szChar,"poke 53280,1");
> // allocate buffer for the void* element ////// This

is
> where bound checker found the problem ///
> if(!pst_Test[i].pVoid)
> pst_Test[i].pVoid = (char*)malloc(32);


You never free that. Therefore, it's a leak. See below for the solution.

> strcpy((char*)pst_Test[i].pVoid,"commodre 64");
> }
>

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

--
Freenet distribution not available
"A verbal contract isn't worth the paper it's printed on."
- Samuel Goldwyn


[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
 
Reply With Quote
 
Balog Pal
Guest
Posts: n/a
 
      06-28-2003
[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}

"Eitan Michaelson" <(E-Mail Removed)> wrote in message
news:bdgrml$drf$(E-Mail Removed)...

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



[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
 
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
impossible to delete file because it is impossible to take ownership Devvie Nuis Computer Support 21 04-20-2009 02:07 AM
Making the simple impossible and the impossible unthinkable... xfx.publishing@gmail.com Perl Misc 5 06-30-2006 11:50 AM
Can realloc potentially cause a memory leak? Jeff Rodriguez C Programming 4 11-19-2003 08:53 AM
Impossible Leak - realloc (Resubmission) Mark McIntyre C Programming 11 07-05-2003 09:32 PM
Impossible Leak realloc Eitan Michaelson C Programming 13 06-30-2003 10:33 AM



Advertisments