Impossible Leak - realloc (Resubmission)

Discussion in 'C Programming' started by Mark McIntyre, Jun 29, 2003.

  1. On Sun, 29 Jun 2003 03:16:07 +0200, in comp.lang.c , "Eitan

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

    too late, if the realloc failed, you leaked.
     
    Mark McIntyre, Jun 29, 2003
    #1
    1. Advertisements

  2. 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 */
     
    Eitan Michaelson, Jun 29, 2003
    #2
    1. Advertisements

  3. 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?
     
    Michaelson Eitan, Jun 29, 2003
    #3
  4. Can't you?
    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, Jun 29, 2003
    #4
  5. Mark McIntyre

    Malcolm Guest

    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.
     
    Malcolm, Jun 29, 2003
    #5
  6. 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.
    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).
    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, Jun 29, 2003
    #6
  7. Mark McIntyre

    Tim Woodall Guest

    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.
     
    Tim Woodall, Jun 30, 2003
    #7
  8. Mark McIntyre

    goose Guest

    what happens ?
    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) ?
    I also dont understand that.
    yup ....
    you just return ? why not free all the memory you have already
    allocated ?
    here again, you just return. dont you think it would be a good idea
    to free all the memory you have *already* allocated ?
    so after the loop finishes, at this point, all you pst_Test[].pVoid
    pointers are intact, right ?
    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.
     
    goose, Jun 30, 2003
    #8
  9. Mark McIntyre

    goose Guest

    the only thing wrong with that is that he discards the old memory,
    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).
    thats right!
    thats *wrong*!!!!!

    are you purposefully dispensing wrong information ?
    no it isn't!
    this is an overly complex way to refer to pst_Test[iCounter].pVoid.
    thats the only correct thing you've said thus far.

    <snipped lots of other misleading info>

    hth
    goose,
     
    goose, Jun 30, 2003
    #9
  10. 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
     
    Brian MacBride, Jun 30, 2003
    #10
  11. Mark McIntyre

    Al Bowers Guest


    Since the variable iCounter is being used to keep a count
    on a dynamically array of the struct object, I find it
    better to encapsulate the pointer to the array along with
    the counter in another struct object. See the example below.


    This if-else statement is not needed. One feature of function realloc
    that you have is that it pst_Test is NULL, which it is in the initialized
    state, function realloc behaves like function malloc for the specified
    size.


    Both realloc statements are 'buggy'. Should realloc return NULL then
    the original allocated space, if any, is now hopelessly lost and
    cannot be freed because pst_Test would have the value of NULL. The
    realloc statement using 'sizeof((void *)' is allocating the wrong
    size space.


    The OP is allocating space for an array of the struct objects.
    The [] operator is valid.


    Here is an example of what I believe the OP is testing:
    a dynamically allocated array of the struct st_Test.

    // Test.c
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>

    typedef struct st_Test
    {
    char szChar[32];
    int iInt;
    void *pVoid;
    }st_Test;

    typedef struct st_Array
    {
    st_Test *pst_Test;
    unsigned pst_Count;
    }st_Array;

    st_Test *addtoArray(st_Array *p, int a);
    void printArray(const st_Array *p);
    void freeArray(st_Array *p);

    int main(void)
    {
    st_Array test = {NULL,0}; // pointer not valid yet
    int i;

    for(i = 2222; i < 2232 ;i++)
    if(NULL == addtoArray(&test,i))
    {
    freeArray(&test);
    puts("Memory Allocation error");
    exit(EXIT_FAILURE);
    }
    puts("The array contents");
    printArray(&test);
    freeArray(&test);
    return 0;
    }

    st_Test *addtoArray(st_Array *p,int a)
    {
    st_Test *temp;
    unsigned cnt = p->pst_Count;

    temp = realloc(p->pst_Test,(cnt+1)*sizeof(*p->pst_Test));
    if(temp == NULL) return NULL;
    sprintf(temp[cnt].szChar,"%d",a);
    if((temp[cnt].pVoid = malloc(strlen(temp[cnt].szChar)+1)) == NULL)
    return NULL;
    strcpy(temp[cnt].pVoid,temp[cnt].szChar);
    temp[cnt].iInt = a;
    p->pst_Test = temp;
    p->pst_Count++;
    return p->pst_Test;
    }

    void printArray(const st_Array *p)
    {
    unsigned i;

    for(i = 0; i < p->pst_Count;i++)
    printf("\tszChar: %s\n\tiInt: %d\n\tpVoid: %s\n\n",
    p->pst_Test.szChar, p->pst_Test.iInt,
    (char *)p->pst_Test.pVoid);
    return;
    }

    void freeArray(st_Array *p)
    {
    unsigned i;

    for(i = 0;i < p->pst_Count;i++)
    free(p->pst_Test.pVoid);
    free(p->pst_Test);
    p->pst_Test = NULL;
    p->pst_Count = 0;
    }



    --
    Al Bowers
    Tampa, FL. USA
    email: (remove the x)
    http://www.geocities.com/abowers822
    comp.lang.c
     
    Al Bowers, Jun 30, 2003
    #11
  12. Mark McIntyre

    m_eitan Guest

    Hello,
    After doing some readings I've came to the conclusion that there was
    nothing wrong neither with my code nor realloc. It appears that bound
    checker was unable to handle this correctly.
    The pointer that was pointing the *pvoid struct member, was moved to a
    new location by realloc, and for some reason boundchecker identified
    this as a leak. When I played with the code, and actually wrote a
    simple "home made" realloc insted of using the library function, no
    leaks where discovered.
    So I guess if anyone from Compuware is reading the posts to this group
    he\she has a little bug to fix....
    By the way, purify and insure++ did not see any leak.

    Eitan.


     
    m_eitan, Jul 5, 2003
    #12
    1. Advertisements

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 (here). After that, you can post your question and our members will help you out.