f



Impossible Leak realloc

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! ]
0
Eitan
6/27/2003 11:01:37 AM
comp.lang.c++.moderated 10738 articles. 1 followers. allnor (8509) is leader. Post Follow

13 Replies
529 Views

Similar Articles

[PageSpeed] 30

"Eitan Michaelson" <m_eitan@netvision.net.il> wrote in message
news:bdgrml$drf$1@news2.netvision.net.il...
| Can any one tell me what's wrong with this code?
|
| It leaks (acceding to bound checker), when it attempts to reallocate
memory.
....
|          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 /////////

Where is the free() that will release pst_Test ?
I think bounds checker simply complains because the allocated
memory is not released before the end of the program (sometime
after the end of the loop).
Note also that:
 - you do not need a special case for the first iteration
   where pst_Test is NULL - realloc will do a malloc if
   its parameter is NULL.

 - the result of realloc should be checked, as it can be NULL.
   when this is the case, the original pst_Test remains a valid
   pointer to a memory block that will need to be freed.
   This could also be causing the complaint of bounds checker.

hth
--
 Ivan Vecerina, Dr. med.  <>  http://www.post1.com/~ivec
 Soft Dev Manger, xitact  <>  http://www.xitact.com
 Brainbench MVP for C++   <>  http://www.brainbench.com





0
ivec (54)
6/27/2003 11:40:03 AM
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! ]
0
Ken
6/27/2003 2:01:16 PM
In article <bdgrml$drf$1@news2.netvision.net.il>,
 on 27 Jun 2003 07:01:37 -0400,
 Eitan Michaelson <m_eitan@netvision.net.il> 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! ]
0
Andy
6/27/2003 2:03:59 PM
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 (cbfalconer@yahoo.com) (cbfalconer@worldnet.att.net)
   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! ]
0
CBFalconer
6/27/2003 2:04:23 PM
In article <bdgrml$drf$1@news2.netvision.net.il>,
Eitan Michaelson  <m_eitan@netvision.net.il> 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! ]
0
rbf
6/27/2003 2:11:20 PM
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.
> 
> 
> 
> 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>
Non-standard header. Use "#include <stdlib.h>".

> #include <string.h>
> 
> #define MAX_STRUCT_SIZE (50)
> struct st_Test
> {
>          char szChar[32];
>          int iInt;
>          void *pVoid;
> };
> void main(void)

OK. Say it with me: "main() returns int...main() returns int..."

> {
>          // Pointer to the structure
>          st_Test *pst_Test;
>          pst_Test = NULL; // pointer not valid yet

Why bother? And if you do want to set it to NULL, why initialize at the 
point of definition?

>          for(int i = 0;i <=MAX_STRUCT_SIZE;i++)
>          {
>                  if(!pst_Test) // not yet allocated
>                          pst_Test = (st_Test*)malloc(1 * sizeof(st_Test));

Lose the cast. It's not necessary.
A better way to write this is:
                            pst_Test = malloc( sizeof *pst_Test );

You should also check the return value of malloc(). Always!

>                  else // Resize by 1
>                          pst_Test = (st_Test*)realloc(pst_Test,(i+1)
> *sizeof(st_Test)); /// This is the Leak /////////

Yup. What if the realloc fails?
In that case, you no longer have access to the memory formerly pointed 
to by pst_Test. You should have written:
                    {
                        st_test * tmp = realloc(pst_Test,
                                                (i+1)*(sizeof *pst_test));
                        if ( tmp == NULL ) {
                           // take appropriate action
                        }
                        else
                           pst_Test = tmp;
                    }

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

Lose the cast.

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

>          }
> 
> ////////////////////////////////////////////////// End
> ///////////////////////////////////////////////////////////////////////
> 
> 
> 
> 
> 

HTH,
--ag
-- 
Artie Gold -- Austin, Texas



----== 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 =---
0
agold (23)
6/27/2003 4:17:56 PM
In <bdgrml$drf$1@news2.netvision.net.il> Eitan Michaelson
<m_eitan@netvision.net.il> 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: Dan.Pop@ifh.de

      [ See http://www.gotw.ca/resources/clcm.htm for info about ]
      [ comp.lang.c++.moderated.    First time posters: Do this! ]
0
Dan
6/27/2003 4:49:59 PM
Eitan Michaelson <m_eitan@netvision.net.il> wrote in message
news:<bdgrml$drf$1@news2.netvision.net.il>...
> 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! ]
0
ruse
6/27/2003 4:51:06 PM
Eitan Michaelson <m_eitan@netvision.net.il> wrote in message
news:<bdgrml$drf$1@news2.netvision.net.il>...
> 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! ]
0
vs_raghuvamshi
6/27/2003 4:51:30 PM
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! ]
0
bd
6/27/2003 5:54:11 PM
[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" <m_eitan@netvision.net.il> wrote in message
news:bdgrml$drf$1@news2.netvision.net.il...

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! ]
0
Balog
6/28/2003 12:28:31 PM
In 'comp.lang.c', Eitan Michaelson <m_eitan@netvision.net.il> wrote:

[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 <stdlib.h>

 > #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[i],'\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[i].iInt = 64738;

Ok.

 >                  strcpy(pst_Test[i].szChar,"poke 53280,1");

Ok, assuming the string is not too large.

                    *pst_Test[i].szChar = 0;
                    strncat(pst_Test[i].szChar
                            , "poke 53280,1"
                            , sizeof pst_Test[i].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[i].pVoid)

pVoid was not properly set to NULL.

 >                          pst_Test[i].pVoid = (char*)malloc(32);
 >                  strcpy((char*)pst_Test[i].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;
}

-- 
-ed- emdelYOURBRA@noos.fr [remove YOURBRA before answering me]
The C-language FAQ: http://www.eskimo.com/~scs/C-faq/top.html
C-library: http://www.dinkumware.com/htm_cl/index.html
FAQ de f.c.l.c : http://www.isty-info.uvsq.fr/~rumeau/fclc/

      [ See http://www.gotw.ca/resources/clcm.htm for info about ]
      [ comp.lang.c++.moderated.    First time posters: Do this! ]
0
Emmanuel
6/28/2003 12:37:37 PM
Eitan Michaelson <m_eitan@netvision.net.il> wrote in message
news:<bdgrml$drf$1@news2.netvision.net.il>...

 > 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[i],'\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[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)

This line contains undefined behavior.  Both in C and in C++.

 >                          pst_Test[i].pVoid = (char*)malloc(32);
 >                  strcpy((char*)pst_Test[i].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.

--
James Kanze             GABI Software             mailto:kanze@gabi-soft.fr
Conseils en informatique oriente objet/            http://www.gabi-soft.fr
                            Beratung in objektorientierter Datenverarbeitung
11 rue de Rambouillet, 78460 Chevreuse, France, Tl. : +33 (0)1 30 23 45 16

      [ See http://www.gotw.ca/resources/clcm.htm for info about ]
      [ comp.lang.c++.moderated.    First time posters: Do this! ]
0
kanze
6/30/2003 10:33:31 AM
Reply: