problem with realloc

Discussion in 'C Programming' started by Igal, May 3, 2008.

  1. Igal

    Igal Guest

    hay, i'm doing this program. having problem wiht realloc in the
    function that reads data structures into array (pointer - bp2), this
    happens after reading the second record. when call to realloc.
    i can't figure out what's wrong, think it's soming got to do with
    freeing bp2.
    and something called "corruption of the heap".

    book* LoadBookData(unsigned *size)
    {
    FILE* fp;
    int n = 0;
    book *bp2 = NULL;

    //open book data file
    fp=fopen("book.bin","rb");
    if (fp == NULL)
    {
    bp2 = (book*)calloc(0, sizeof(book));
    return bp2;
    }
    <<<<<<<<<<<<<<<HERE>>>>>>>>>>>>>>
    bp2 = realloc(bp2, sizeof(book));
    if (bp2 == NULL) { perror("ERROR [realloc - LoadBookData()]");
    exit(ERR_REALOC); }

    //read data from file
    while(fread(bp2,sizeof(book),1,fp) == 1)
    {
    if(ferror(fp)) { perror("ERROR [book.bin - read - LoadBookData()]");
    exit(ERR_FREAD); }
    bp2 = bp2 - n;
    n++;

    bp2 = realloc(bp2, sizeof(book));
    if (bp2 == NULL) { perror("ERROR [realloc - LoadBookData()]");
    exit(ERR_REALOC); }

    bp2 = bp2 + n;
    }

    //close book data file
    if (fclose(fp)==EOF)
    { perror("ERROR [book.bin - close - LoadBookData()]]");
    exit(ERR_FCLOSE); }

    *size = n;

    return bp2;
    }
     
    Igal, May 3, 2008
    #1
    1. Advertising

  2. Igal

    Guest

    On May 3, 7:21 pm, Igal <> wrote:
    > hay, i'm doing this program. having problem wiht realloc in the
    > function that reads data structures into array (pointer - bp2), this
    > happens after reading the second record. when call to realloc.
    > i can't figure out what's wrong, think it's soming got to do with
    > freeing bp2.
    > and something called "corruption of the heap".
    >
    > book* LoadBookData(unsigned *size)
    > {
    > FILE* fp;
    > int n = 0;
    > book *bp2 = NULL;
    >
    > //open book data file
    > fp=fopen("book.bin","rb");
    > if (fp == NULL)
    > {
    > bp2 = (book*)calloc(0, sizeof(book));
    > return bp2;

    Don't cast calloc. I'm not sure what the results of calloc(0, N) are,
    but if they are similar of malloc(), then that pointer is not really
    reliable.
    You most likely want something like this:
    return calloc(0, sizeof book);
    > }
    > <<<<<<<<<<<<<<<HERE>>>>>>>>>>>>>>

    Comment your code with /* */
    > bp2 = realloc(bp2, sizeof(book));

    That's actually a malloc(), since bp2 was initialized to NULL before,
    and realloc(NULL, N) is equal to malloc(N)
    > if (bp2 == NULL) { perror("ERROR [realloc - LoadBookData()]");
    > exit(ERR_REALOC); }

    Are you sure you want to exit here? And what is the value of
    ERR_REALOC?
    >
    > //read data from file
    > while(fread(bp2,sizeof(book),1,fp) == 1)
    > {
    > if(ferror(fp)) { perror("ERROR [book.bin - read - LoadBookData()]");

    fread() can't return the count (third parameter) if an error in the
    stream occurs.
    > exit(ERR_FREAD); }

    Memory leak here, you don't free `bp2'.
    > bp2 = bp2 - n;
    > n++;

    Why?
    >
    > bp2 = realloc(bp2, sizeof(book));

    If `bp2' doesn't point to a pointer returned by realloc, malloc or
    calloc, realloc() will produce undefined results (unless `bp2's value
    is NULL)
    But I *see* what you are trying to do, here's the actual logic:
    size_t n = 0;
    back:
    if(fread(&bp2[n], sizeof bp2[0], 1, fp) != 1) { /* handle error or EOF
    */ }
    n++;
    bp2 = realloc(bp2, n+1 * sizeof bp2[0]);
    if(bp2 == NULL) { /* ... */ }
    goto back;
     
    , May 3, 2008
    #2
    1. Advertising

  3. Igal

    Guest Guest

    "Igal" <> wrote in message
    news:...
    > hay, i'm doing this program. having problem wiht realloc in the
    > function that reads data structures into array (pointer - bp2), this
    > happens after reading the second record. when call to realloc.
    > i can't figure out what's wrong, think it's soming got to do with
    > freeing bp2.
    > and something called "corruption of the heap".
    >
    > book* LoadBookData(unsigned *size)
    > {
    > FILE* fp;
    > int n = 0;
    > book *bp2 = NULL;
    >
    > //open book data file
    > fp=fopen("book.bin","rb");
    > if (fp == NULL)
    > {
    > bp2 = (book*)calloc(0, sizeof(book));
    > return bp2;
    > }
    > <<<<<<<<<<<<<<<HERE>>>>>>>>>>>>>>
    > bp2 = realloc(bp2, sizeof(book));
    > if (bp2 == NULL) { perror("ERROR [realloc - LoadBookData()]");
    > exit(ERR_REALOC); }
    >
    > //read data from file
    > while(fread(bp2,sizeof(book),1,fp) == 1)
    > {
    > if(ferror(fp)) { perror("ERROR [book.bin - read - LoadBookData()]");
    > exit(ERR_FREAD); }
    > bp2 = bp2 - n;
    > n++;
    >
    > bp2 = realloc(bp2, sizeof(book));
    > if (bp2 == NULL) { perror("ERROR [realloc - LoadBookData()]");
    > exit(ERR_REALOC); }
    >
    > bp2 = bp2 + n;
    > }
    >
    > //close book data file
    > if (fclose(fp)==EOF)
    > { perror("ERROR [book.bin - close - LoadBookData()]]");
    > exit(ERR_FCLOSE); }
    >
    > *size = n;
    >
    > return bp2;
    > }`




    Say hey.

    Jim
     
    Guest, May 3, 2008
    #3
  4. Igal wrote:

    > hay, i'm doing this program. having problem wiht realloc in the
    > function that reads data structures into array (pointer - bp2), this
    > happens after reading the second record. when call to realloc.
    > i can't figure out what's wrong, think it's soming got to do with
    > freeing bp2.
    > and something called "corruption of the heap".
    >
    > book* LoadBookData(unsigned *size)
    > {
    > FILE* fp;
    > int n = 0;
    > book *bp2 = NULL;
    >
    > //open book data file
    > fp=fopen("book.bin","rb");
    > if (fp == NULL)
    > {
    > bp2 = (book*)calloc(0, sizeof(book));
    > return bp2;
    > }


    * Don't cast calloc/malloc/realloc etc.
    * Why assign to bp2 at all?
    * There is no way that your code can signal an error here, i.e. the calling
    code can't distinguish between a non-existant and empty file.

    > bp2 = realloc(bp2, sizeof(book));
    > if (bp2 == NULL) { perror("ERROR [realloc - LoadBookData()]");
    > exit(ERR_REALOC); }


    Here, you exit on realloc() failure, above, you return NULL when calloc()
    fails. I'd suggest using some kind of xalloc().

    > //read data from file
    > while(fread(bp2,sizeof(book),1,fp) == 1)


    Note: the binary layout of structures depends on the compiler and system. It
    is for sure not easily portable to store binary dumps of structures in a
    file like that.

    > {
    > if(ferror(fp)) { perror("ERROR [book.bin - read - LoadBookData()]");
    > exit(ERR_FREAD); }
    > bp2 = bp2 - n;
    > n++;
    >
    > bp2 = realloc(bp2, sizeof(book));


    Problem here: the pointer you pass to realloc() _MUST_ have been acquired by
    a former call to realloc()/malloc(). Suggestion:

    book tmp;
    while(read_book( &tmp, fp)) {
    bp2 = xrealloc( bp2, (n+1)*(sizeof *bp2));
    bp2[n++] = tmp;
    }

    cheers

    Uli
     
    Ulrich Eckhardt, May 3, 2008
    #4
  5. Igal

    Igal Guest


    > If `bp2' doesn't point to a pointer returned by realloc, malloc or
    > calloc, realloc() will produce undefined results (unless `bp2's value
    > is NULL)
    > But I *see* what you are trying to do, here's the actual logic:
    > size_t n = 0;
    > back:
    > if(fread(&bp2[n], sizeof bp2[0], 1, fp) != 1) { /* handle error or EOF
    > */ }
    > n++;
    > bp2 = realloc(bp2, n+1 * sizeof bp2[0]);
    > if(bp2 == NULL) { /* ... */ }
    > goto back;


    thanks, this helped a lot. i just didn't know i can use bp2 just as
    normal array.
     
    Igal, May 4, 2008
    #5
  6. Igal

    Guest

    On May 4, 2:53 am, Igal <> wrote:
    > > If `bp2' doesn't point to a pointer returned by realloc, malloc or
    > > calloc, realloc() will produce undefined results (unless `bp2's value
    > > is NULL)
    > > But I *see* what you are trying to do, here's the actual logic:
    > > size_t n = 0;
    > > back:
    > > if(fread(&bp2[n], sizeof bp2[0], 1, fp) != 1) { /* handle error or EOF
    > > */ }
    > > n++;
    > > bp2 = realloc(bp2, n+1 * sizeof bp2[0]);
    > > if(bp2 == NULL) { /* ... */ }
    > > goto back;

    >
    > thanks, this helped a lot. i just didn't know i can use bp2 just as
    > normal array.


    Depends, in this case you can.
    Read section 6 of the FAQ <http://c-faq.com/> which explains arrays/
    pointers and then read the rest of the FAQ.
     
    , May 4, 2008
    #6
  7. On Sat, 3 May 2008 09:33:17 -0700 (PDT), wrote:

    >On May 3, 7:21 pm, Igal <> wrote:
    >> hay, i'm doing this program. having problem wiht realloc in the
    >> function that reads data structures into array (pointer - bp2), this
    >> happens after reading the second record. when call to realloc.
    >> i can't figure out what's wrong, think it's soming got to do with
    >> freeing bp2.
    >> and something called "corruption of the heap".
    >>
    >> book* LoadBookData(unsigned *size)
    >> {
    >> FILE* fp;
    >> int n = 0;
    >> book *bp2 = NULL;
    >>
    >> //open book data file
    >> fp=fopen("book.bin","rb");
    >> if (fp == NULL)
    >> {
    >> bp2 = (book*)calloc(0, sizeof(book));
    >> return bp2;

    >Don't cast calloc. I'm not sure what the results of calloc(0, N) are,
    >but if they are similar of malloc(), then that pointer is not really
    >reliable.
    >You most likely want something like this:
    > return calloc(0, sizeof book);


    book is a type. The parentheses are required. Other than the cast
    and parentheses, your code is identical to the OP's. Surely not your
    intent.



    Remove del for email
     
    Barry Schwarz, May 4, 2008
    #7
  8. Igal

    Guest

    On May 4, 7:51 am, Barry Schwarz <> wrote:
    > On Sat, 3 May 2008 09:33:17 -0700 (PDT), wrote:
    > >On May 3, 7:21 pm, Igal <> wrote:
    > >> hay, i'm doing this program. having problem wiht realloc in the
    > >> function that reads data structures into array (pointer - bp2), this
    > >> happens after reading the second record. when call to realloc.
    > >> i can't figure out what's wrong, think it's soming got to do with
    > >> freeing bp2.
    > >> and something called "corruption of the heap".

    >
    > >> book* LoadBookData(unsigned *size)
    > >> {
    > >> FILE* fp;
    > >> int n = 0;
    > >> book *bp2 = NULL;

    >
    > >> //open book data file
    > >> fp=fopen("book.bin","rb");
    > >> if (fp == NULL)
    > >> {
    > >> bp2 = (book*)calloc(0, sizeof(book));
    > >> return bp2;

    > >Don't cast calloc. I'm not sure what the results of calloc(0, N) are,
    > >but if they are similar of malloc(), then that pointer is not really
    > >reliable.
    > >You most likely want something like this:
    > > return calloc(0, sizeof book);

    >
    > book is a type. The parentheses are required. Other than the cast
    > and parentheses, your code is identical to the OP's. Surely not your
    > intent.

    Yes you are right, thanks for pointing that out.
    return calloc(1, sizeof bp2);
    `book' is confusing as a type. Maybe book_t or Book. Regardless, my
    mistake.
     
    , May 4, 2008
    #8
  9. wrote:
    > On May 4, 7:51 am, Barry Schwarz <> wrote:
    >> On Sat, 3 May 2008 09:33:17 -0700 (PDT), wrote:
    >> >You most likely want something like this:
    >> > return calloc(0, sizeof book);

    >>
    >> book is a type. The parentheses are required. Other than the cast
    >> and parentheses, your code is identical to the OP's. Surely not your
    >> intent.

    > Yes you are right, thanks for pointing that out.
    > return calloc(1, sizeof bp2);


    Still wrong, bp2 is a pointer. You probably meant *bp2. However, that then
    spoils the whole relation because it isn't used to initialise bp2. No, in
    the elided original source, bp2 can well be removed completely, so using
    sizeof (book) should work.

    > `book' is confusing as a type. Maybe book_t or Book. Regardless, my
    > mistake.


    I beg to differ. I also don't need int_t or Integer. YMMV.

    Uli
     
    Ulrich Eckhardt, May 13, 2008
    #9
  10. Igal

    santosh Guest

    Ulrich Eckhardt wrote:

    > wrote:
    >> On May 4, 7:51 am, Barry Schwarz <> wrote:
    >>> On Sat, 3 May 2008 09:33:17 -0700 (PDT), wrote:
    >>> >You most likely want something like this:
    >>> > return calloc(0, sizeof book);
    >>>
    >>> book is a type. The parentheses are required. Other than the cast
    >>> and parentheses, your code is identical to the OP's. Surely not
    >>> your intent.

    >> Yes you are right, thanks for pointing that out.
    >> return calloc(1, sizeof bp2);

    >
    > Still wrong, bp2 is a pointer. You probably meant *bp2. However, that
    > then spoils the whole relation because it isn't used to initialise
    > bp2. No, in the elided original source, bp2 can well be removed
    > completely, so using sizeof (book) should work.
    >
    >> `book' is confusing as a type. Maybe book_t or Book. Regardless, my
    >> mistake.

    >
    > I beg to differ. I also don't need int_t or Integer. YMMV.


    Also identifiers ending with a '_t' are reserved by POSIX. A pretty wide
    sweep if you ask me.
     
    santosh, May 15, 2008
    #10
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Bren
    Replies:
    8
    Views:
    2,033
    Stephen Howe
    Sep 4, 2003
  2. Problem with realloc ()

    , Feb 17, 2007, in forum: C Programming
    Replies:
    3
    Views:
    376
    Richard Heathfield
    Feb 17, 2007
  3. marvinla

    Pointer-to-pointer realloc problem

    marvinla, Jul 4, 2007, in forum: C Programming
    Replies:
    29
    Views:
    911
    David Thompson
    Jul 22, 2007
  4. Igal

    realloc problem, corrupt last item

    Igal, May 8, 2008, in forum: C Programming
    Replies:
    11
    Views:
    564
    Eligiusz Narutowicz
    May 9, 2008
  5. ninboy

    malloc and realloc problem

    ninboy, May 19, 2008, in forum: C Programming
    Replies:
    1
    Views:
    332
    ninboy
    May 20, 2008
Loading...

Share This Page