Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > Impossible Leak - realloc (Resubmission)

Reply
Thread Tools

Impossible Leak - realloc (Resubmission)

 
 
Mark McIntyre
Guest
Posts: n/a
 
      06-29-2003
On Sun, 29 Jun 2003 03:16:07 +0200, in comp.lang.c , "Eitan
Michaelson" <(E-Mail Removed)> wrote:

snipped

I've no idea what your problem is. realloc frees the pointer that was
malloced - yes, it often does, its supposed to, if there's not enough
memory at the original location. This is NOT a leak, its correctly
freed.

> struct st_Test *pst_Test = NULL;


> if(!pst_Test) /* not yet allocated */
> pst_Test = (struct st_Test*)malloc(1 * sizeof(struct st_Test));


fine, though you do not need the cast. Or rather, if you do, you're
compiling C++ not C, and you should be using new not malloc.

> else /* resize by 1 , Note: This is my problem!!!! */
> pst_Test = (struct st_Test*)realloc(pst_Test,(iCounter+1)
>*sizeof(struct st_Test));


This is a memory leak because if realloc fails, it returns NULL but
/without/ freeing the original pointer. You MUST use a temporary when
reallocing.
struct st_Test* tmp = realloc(ps_Test, iCounter+1);
if(!tmp)
// failed to find memory, handle the error
else
/// ok, proceed


> // check that malloc\realloc worked
> if(!pst_Test) return -1;


too late, if the realloc failed, you leaked.

--
Mark McIntyre
CLC FAQ <http://www.eskimo.com/~scs/C-faq/top.html>
CLC readme: <http://www.angelfire.com/ms3/bchambless0/welcome_to_clc.html>


----== Posted via Newsfeed.Com - Unlimited-Uncensored-Secure Usenet News==----
http://www.newsfeed.com The #1 Newsgroup Service in the World! >100,000 Newsgroups
---= 19 East/West-Coast Specialized Servers - Total Privacy via Encryption =---
 
Reply With Quote
 
 
 
 
Eitan Michaelson
Guest
Posts: n/a
 
      06-29-2003
Hello,

First I would like to thank all of you, who took the time to response to my
last submission,

Second I've reorganized my question to this group so it would please
everyone...

The code below (which is a C code that can be compiled with a C compiler)
demonstrates a memory leak with realloc.

I have a structure and a pointer to that structure, so far so good; with
realloc I'm allocating space for my pointer and then resizing it by 1.

(Once again, so far no problems).

Now, one of the structure elements is a void*, and this pointer gets
allocated in turn as well (with malloc). So far so good,

Then my program attempts to resize the structure array once again.... and
Ooops. !so good.

The void* pointer that was allocated by malloc, get's freed somehow by the
realloc, and now I have a memory leak.

This would never leak if had not included the void* element in my structure.



10x

Eitan Michaelson



See 4 your self:



/* begin code section */
/* was tested on Window$ platform + bound checker and it is defiantly
leaking */


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

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

#define MAX_STRUCT_SIZE (50)
#define CHAR_ARRAY_SIZE (32)

struct st_Test
{

char szChar[CHAR_ARRAY_SIZE];
int iInt;
void *pVoid;

};

int main(void)

{

/* Pointer to the structure */
struct st_Test *pst_Test = NULL;
char szAny[CHAR_ARRAY_SIZE];
int iCounter;

for(iCounter = 0;iCounter <=MAX_STRUCT_SIZE;iCounter++)
{

if(!pst_Test) /* not yet allocated */
pst_Test = (struct st_Test*)malloc(1 * sizeof(struct st_Test));

else /* resize by 1 , Note: This is my problem!!!! */
pst_Test = (struct st_Test*)realloc(pst_Test,(iCounter+1)
*sizeof(struct st_Test));

// check that malloc\realloc worked
if(!pst_Test) return -1;

/* Add some data to the allocated struct */
pst_Test[iCounter].iInt = iCounter;
sprintf(szAny,"szChar = %d",iCounter);
strncpy(pst_Test[iCounter].szChar,szAny,CHAR_ARRAY_SIZE);

/* allocate buffer for the void* element */
pst_Test[iCounter].pVoid = (char*)malloc(CHAR_ARRAY_SIZE);

/* check for a valid pointer after malloc */
if(!pst_Test) return -1;
sprintf(szAny,"pVoid = %d",iCounter);
strncpy((char*)pst_Test[iCounter].pVoid,szAny,CHAR_ARRAY_SIZE);
}

/* free all structure elements */

if(pst_Test) /* we don't really need this ... */
{

for(iCounter = 0;iCounter <=MAX_STRUCT_SIZE;iCounter++)
{
free(pst_Test[iCounter].pVoid);
pst_Test[iCounter].pVoid = NULL; /* just to be on z safe side */
}

free(pst_Test);
pst_Test = NULL;

}

return 0;
}

/* end code section */


 
Reply With Quote
 
 
 
 
Michaelson Eitan
Guest
Posts: n/a
 
      06-29-2003
So basically, I can't use a void* element in a structure that will be
resized later in my program, since realloc will free all the allocated
buffer before creating the new one.
by freeing all the structure elements it will destroy any memory that
was allocated to that void* ?
Is that true?
Is there any other way 4 doing this?
Is there a "better" realloc?


Mark McIntyre <(E-Mail Removed)> wrote in message news:<(E-Mail Removed)>. ..
> On Sun, 29 Jun 2003 03:16:07 +0200, in comp.lang.c , "Eitan
> Michaelson" <(E-Mail Removed)> wrote:
>
> snipped
>
> I've no idea what your problem is. realloc frees the pointer that was
> malloced - yes, it often does, its supposed to, if there's not enough
> memory at the original location. This is NOT a leak, its correctly
> freed.
>
> > struct st_Test *pst_Test = NULL;

>
> > if(!pst_Test) /* not yet allocated */
> > pst_Test = (struct st_Test*)malloc(1 * sizeof(struct st_Test));

>
> fine, though you do not need the cast. Or rather, if you do, you're
> compiling C++ not C, and you should be using new not malloc.
>
> > else /* resize by 1 , Note: This is my problem!!!! */
> > pst_Test = (struct st_Test*)realloc(pst_Test,(iCounter+1)
> >*sizeof(struct st_Test));

>
> This is a memory leak because if realloc fails, it returns NULL but
> /without/ freeing the original pointer. You MUST use a temporary when
> reallocing.
> struct st_Test* tmp = realloc(ps_Test, iCounter+1);
> if(!tmp)
> // failed to find memory, handle the error
> else
> /// ok, proceed
>
>
> > // check that malloc\realloc worked
> > if(!pst_Test) return -1;

>
> too late, if the realloc failed, you leaked.
>
> --
> Mark McIntyre
> CLC FAQ <http://www.eskimo.com/~scs/C-faq/top.html>
> CLC readme: <http://www.angelfire.com/ms3/bchambless0/welcome_to_clc.html>
>
>
> ----== Posted via Newsfeed.Com - Unlimited-Uncensored-Secure Usenet News==----
> http://www.newsfeed.com The #1 Newsgroup Service in the World! >100,000 Newsgroups
> ---= 19 East/West-Coast Specialized Servers - Total Privacy via Encryption =---

 
Reply With Quote
 
Richard Heathfield
Guest
Posts: n/a
 
      06-29-2003
Michaelson Eitan wrote:

> So basically, I can't use a void* element in a structure that will be
> resized later in my program, since realloc will free all the allocated
> buffer before creating the new one.


Can't you?

> by freeing all the structure elements it will destroy any memory that
> was allocated to that void* ?


No, realloc will retain as much data as possible. If you resize from, say,
1000 bytes to 800, those 800 bytes will retain their value. If you resize
from 1000 to 1200, all 1000 bytes will retain their value.

In general, when reallocating a block of objects of some arbitrary object
type T, do it like this:

size_t newsize = oldsize + n;
T *tmp = realloc(tptr, newsize * sizeof *tmp);
if(tmp != NULL)
{
tptr = tmp;
tmp = NULL;
newsize = oldsize + n;
You can use your new memory now. All the old data, as far as is possible,
is still there.
}
else
{
at least you still have your old memory
}


--
Richard Heathfield : http://www.velocityreviews.com/forums/(E-Mail Removed)
"Usenet is a strange place." - Dennis M Ritchie, 29 July 1999.
C FAQ: http://www.eskimo.com/~scs/C-faq/top.html
K&R answers, C books, etc: http://users.powernet.co.uk/eton
 
Reply With Quote
 
Malcolm
Guest
Posts: n/a
 
      06-29-2003

"Michaelson Eitan" <(E-Mail Removed)> wrote in message
> So basically, I can't use a void* element in a structure that will be
> resized later in my program, since realloc will free all the allocated
> buffer before creating the new one.
> by freeing all the structure elements it will destroy any memory that
> was allocated to that void* ?
> Is that true?
>

No, your program was fine. The pointer is moved to a new position but
retains its value - it still points to the allocated block of memory.
Of course if you shrink a memory block using realloc() you will destroy any
pointers in the lost part of memory - the blocks the pointers point to won't
be freed, but unless you have another pointer to them they will be orphaned.
I suspect that realloc() is fooling boundschecker and it is reporting a
memory leak where there isn't one.



 
Reply With Quote
 
Mark McIntyre
Guest
Posts: n/a
 
      06-29-2003
On 28 Jun 2003 23:32:13 -0700, in comp.lang.c , (E-Mail Removed)
(Michaelson Eitan) wrote:

>So basically, I can't use a void* element in a structure that will be
>resized later in my program, since realloc will free all the allocated
>buffer before creating the new one.


realloc frees up the old memory only if it cannot extend the original
block enough for some reason. So you _might_ get back the same pointer
you sent in. Or you night not. You should not rely on either
behaviour.

>by freeing all the structure elements it will destroy any memory that
>was allocated to that void* ?


if you read the fn description, you'll see that it will copy any data
you currently have to the new location. Nothing is lost, unless you
use it unsafely (ie as you originally did).

>Is there a "better" realloc?


whats wrong with realloc the way it is? It does what it says on the
box.

Also, please don't top post. If you are not using the material in the
previous post, then snip it all away.
--
Mark McIntyre
CLC FAQ <http://www.eskimo.com/~scs/C-faq/top.html>
CLC readme: <http://www.angelfire.com/ms3/bchambless0/welcome_to_clc.html>


----== Posted via Newsfeed.Com - Unlimited-Uncensored-Secure Usenet News==----
http://www.newsfeed.com The #1 Newsgroup Service in the World! >100,000 Newsgroups
---= 19 East/West-Coast Specialized Servers - Total Privacy via Encryption =---
 
Reply With Quote
 
Tim Woodall
Guest
Posts: n/a
 
      06-30-2003
On Sun, 29 Jun 2003 07:43:24 +0000 (UTC),
Richard Heathfield <(E-Mail Removed)> wrote:
> Michaelson Eitan wrote:
>
>> So basically, I can't use a void* element in a structure that will be
>> resized later in my program, since realloc will free all the allocated
>> buffer before creating the new one.

>
> Can't you?
>
>> by freeing all the structure elements it will destroy any memory that
>> was allocated to that void* ?

>
> No, realloc will retain as much data as possible. If you resize from, say,
> 1000 bytes to 800, those 800 bytes will retain their value. If you resize
> from 1000 to 1200, all 1000 bytes will retain their value.
>


I couldn't be bothered to understand the original code posted but the OPs
problem might be that if you have pointers inside your malloc()ed memory,
e.g. a linked list contained within a single malloc() then realloc can
break the pointers because the entire block of memory can move.

(OT for c.l.c. Similar problems occur when using shared memory - shmat()
doesn't necessarily return the same pointer to all processes)

In order to avoid this problem you have to use offsets (usually to the start
of the memory block) rather then explicit pointers.

Regards,

Tim.



--
God said, "div D = rho, div B = 0, curl E = - @B/@t, curl H = J + @D/@t,"
and there was light.

http://tjw.hn.org/ http://www.locofungus.btinternet.co.uk/
 
Reply With Quote
 
goose
Guest
Posts: n/a
 
      06-30-2003
"Eitan Michaelson" <(E-Mail Removed)> wrote in message news:<bdlb3c$866$(E-Mail Removed)>...
<snipped>
>
> Then my program attempts to resize the structure array once again.... and
> Ooops. !so good.


what happens ?

>
> The void* pointer that was allocated by malloc, get's freed somehow by the
> realloc, and now I have a memory leak.


I dont understand. where exactly *is* the void* being tested, other
than when you free it.

iow, how do you know that the pointer gets freed (or corrupted) ?

>
> This would never leak if had not included the void* element in my structure.
>


I also dont understand that.

>
>
> 10x
>
> Eitan Michaelson
>
>
>
> See 4 your self:
>
>
>
> /* begin code section */
> /* was tested on Window$ platform + bound checker and it is defiantly
> leaking */
>
>
> #ifdef __cplusplus
> #error Wrong compiler. Use a C one.
> #endif
>
> #include <stdlib.h>
> #include <stdio.h>
> #include <string.h>
>
> #define MAX_STRUCT_SIZE (50)
> #define CHAR_ARRAY_SIZE (32)
>
> struct st_Test
> {
>
> char szChar[CHAR_ARRAY_SIZE];
> int iInt;
> void *pVoid;
>
> };
>
> int main(void)
>
> {
>
> /* Pointer to the structure */
> struct st_Test *pst_Test = NULL;
> char szAny[CHAR_ARRAY_SIZE];
> int iCounter;
>
> for(iCounter = 0;iCounter <=MAX_STRUCT_SIZE;iCounter++)
> {
>
> if(!pst_Test) /* not yet allocated */
> pst_Test = (struct st_Test*)malloc(1 * sizeof(struct st_Test));
>
> else /* resize by 1 , Note: This is my problem!!!! */


yup ....
> pst_Test = (struct st_Test*)realloc(pst_Test,(iCounter+1)
> *sizeof(struct st_Test));
>
> // check that malloc\realloc worked
> if(!pst_Test) return -1;


you just return ? why not free all the memory you have already
allocated ?

>
> /* Add some data to the allocated struct */
> pst_Test[iCounter].iInt = iCounter;
> sprintf(szAny,"szChar = %d",iCounter);
> strncpy(pst_Test[iCounter].szChar,szAny,CHAR_ARRAY_SIZE);
>
> /* allocate buffer for the void* element */
> pst_Test[iCounter].pVoid = (char*)malloc(CHAR_ARRAY_SIZE);
>
> /* check for a valid pointer after malloc */
> if(!pst_Test) return -1;


here again, you just return. dont you think it would be a good idea
to free all the memory you have *already* allocated ?

> sprintf(szAny,"pVoid = %d",iCounter);
> strncpy((char*)pst_Test[iCounter].pVoid,szAny,CHAR_ARRAY_SIZE);
> }
>


so after the loop finishes, at this point, all you pst_Test[].pVoid
pointers are intact, right ?

> /* free all structure elements */
>
> if(pst_Test) /* we don't really need this ... */
> {
>
> for(iCounter = 0;iCounter <=MAX_STRUCT_SIZE;iCounter++)
> {
> free(pst_Test[iCounter].pVoid);
> pst_Test[iCounter].pVoid = NULL; /* just to be on z safe side */
> }
>
> free(pst_Test);
> pst_Test = NULL;
>
> }
>
> return 0;
> }
>
> /* end code section */


so where exactly is pVoid getting corrupt ?


(ps, lose the casts)

goose,
still dont know what you think your problem is. code lacks only
"recovery in case of malloc/realloc failure" feature. that possibly
*is* a memory leak.
 
Reply With Quote
 
goose
Guest
Posts: n/a
 
      06-30-2003
Nick Austin <(E-Mail Removed)> wrote in message news:<(E-Mail Removed)>. ..
> On Sun, 29 Jun 2003 03:16:07 +0200, "Eitan Michaelson"
> <(E-Mail Removed)> wrote:


<snipped>

> >struct st_Test
> >{
> >
> > char szChar[CHAR_ARRAY_SIZE];
> > int iInt;
> > void *pVoid;
> >
> >};
> >
> >int main(void)
> >
> >{
> >
> > /* Pointer to the structure */
> > struct st_Test *pst_Test = NULL;


<snipped>

> >
> > else /* resize by 1 , Note: This is my problem!!!! */
> > pst_Test = (struct st_Test*)realloc(pst_Test,(iCounter+1)
> >*sizeof(struct st_Test));

>


the only thing wrong with that is that he discards the old memory,

> This is a bug but it is harmless. You allocate way too much memory.
> It should be:
>
> pst_Test = realloc(pst_Test,
> (iCounter*sizeof(void*))+sizeof(struct st_Test));


this, otoh, isn't. (look carefully, and tell me why sizeof (void*) is
needed at all, nevermind the multiplication).

this is faulty information.

(unless i *really* am missing something, in which case feel free to
inform me).

> > pst_Test[iCounter].iInt = iCounter;


thats right!

>
> What??? pst_Test is not an array! You just want:
>
> pst_Test->iInt = iCounter;


thats *wrong*!!!!!

are you purposefully dispensing wrong information ?

>
> > sprintf(szAny,"szChar = %d",iCounter);
> > strncpy(pst_Test[iCounter].szChar,szAny,CHAR_ARRAY_SIZE);

>
> Ditto.
>
> > /* allocate buffer for the void* element */
> > pst_Test[iCounter].pVoid = (char*)malloc(CHAR_ARRAY_SIZE);

>
> Also wrong.


no it isn't!

> I think what you want is probably:
>
> (&pst_Test->pVoid)[iCounter] = malloc(CHAR_ARRAY_SIZE);


this is an overly complex way to refer to pst_Test[iCounter].pVoid.

>
> > /* check for a valid pointer after malloc */
> > if(!pst_Test) return -1;

>
> You alreay tested this pointer.
>


thats the only correct thing you've said thus far.

<snipped lots of other misleading info>

hth
goose,
 
Reply With Quote
 
Brian MacBride
Guest
Posts: n/a
 
      06-30-2003

"Nick Austin" <(E-Mail Removed)> wrote in message
news:(E-Mail Removed)...
> On Sun, 29 Jun 2003 03:16:07 +0200, "Eitan Michaelson"
> <(E-Mail Removed)> wrote:
>
> >#include <stdlib.h>
> >#include <stdio.h>
> >#include <string.h>
> >
> >#define MAX_STRUCT_SIZE (50)
> >#define CHAR_ARRAY_SIZE (32)
> >
> >struct st_Test
> >{
> >
> > char szChar[CHAR_ARRAY_SIZE];
> > int iInt;
> > void *pVoid;
> >
> >};
> >
> >int main(void)
> >
> >{
> >
> > /* Pointer to the structure */
> > struct st_Test *pst_Test = NULL;
> > char szAny[CHAR_ARRAY_SIZE];
> > int iCounter;
> >
> > for(iCounter = 0;iCounter <=MAX_STRUCT_SIZE;iCounter++)
> > {
> >
> > if(!pst_Test) /* not yet allocated */
> > pst_Test = (struct st_Test*)malloc(1 * sizeof(struct

st_Test));
> >
> > else /* resize by 1 , Note: This is my problem!!!! */
> > pst_Test = (struct st_Test*)realloc(pst_Test,(iCounter+1)
> >*sizeof(struct st_Test));

>
> This is a bug but it is harmless. You allocate way too much memory.
> It should be:
>
> pst_Test = realloc(pst_Test,
> (iCounter*sizeof(void*))+sizeof(struct st_Test));
>
> > // check that malloc\realloc worked
> > if(!pst_Test) return -1;
> >
> > /* Add some data to the allocated struct */
> > pst_Test[iCounter].iInt = iCounter;

>
> What??? pst_Test is not an array! You just want:
>
> pst_Test->iInt = iCounter;
>
> > sprintf(szAny,"szChar = %d",iCounter);
> > strncpy(pst_Test[iCounter].szChar,szAny,CHAR_ARRAY_SIZE);

>
> Ditto.
>
> > /* allocate buffer for the void* element */
> > pst_Test[iCounter].pVoid = (char*)malloc(CHAR_ARRAY_SIZE);

>
> Also wrong. I think what you want is probably:
>
> (&pst_Test->pVoid)[iCounter] = malloc(CHAR_ARRAY_SIZE);
>
> > /* check for a valid pointer after malloc */
> > if(!pst_Test) return -1;

>
> You alreay tested this pointer.
>
> > sprintf(szAny,"pVoid = %d",iCounter);
> > strncpy((char*)pst_Test[iCounter].pVoid,szAny,CHAR_ARRAY_SIZE);

>
> Replace pst_Test[iCounter].pVoid with (&pst_Test->pVoid)[iCounter]
>
> > }
> >
> > /* free all structure elements */
> >
> > if(pst_Test) /* we don't really need this ... */
> > {
> >
> > for(iCounter = 0;iCounter <=MAX_STRUCT_SIZE;iCounter++)
> > {
> > free(pst_Test[iCounter].pVoid);

>
> Ditto.
>
> > pst_Test[iCounter].pVoid = NULL; /* just to be on z safe side

*/
>
> Ditto.
>
> > }
> >
> > free(pst_Test);
> > pst_Test = NULL;
> >
> > }
> >
> > return 0;
> >}

>
> Nick.
>


I think the OP wanted something like this but dynamically allocated...

#define MAX_STRUCT_SIZE 50
#define CHAR_ARRAY_SIZE 32

struct st_Test {
char szChar1[CHAR_ARRAY_SIZE];
int iInt;
char szChar2[CHAR_ARRAY_SIZE];
} pst_Test[MAX_STRUCT_SIZE];

your 'solution' gets the OP the equivalent of...

struct st_Test {
char szChar1[CHAR_ARRAY_SIZE];
int iInt;
char szChar2[CHAR_ARRAY_SIZE][MAX_STRUCT_SIZE];
} pst_Test;


Regards

Brian

 
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 Eitan Michaelson C Programming 13 06-30-2003 10:33 AM
Impossible Leak realloc Eitan Michaelson C++ 11 06-30-2003 10:33 AM



Advertisments