Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > problem with realloc

Reply
Thread Tools

problem with realloc

 
 
Igal
Guest
Posts: n/a
 
      05-03-2008
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;
}
 
Reply With Quote
 
 
 
 
vippstar@gmail.com
Guest
Posts: n/a
 
      05-03-2008
On May 3, 7:21 pm, Igal <(E-Mail Removed)> 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;
 
Reply With Quote
 
 
 
 
Guest
Posts: n/a
 
      05-03-2008

"Igal" <(E-Mail Removed)> wrote in message
news:(E-Mail Removed)...
> 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


 
Reply With Quote
 
Ulrich Eckhardt
Guest
Posts: n/a
 
      05-03-2008
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

 
Reply With Quote
 
Igal
Guest
Posts: n/a
 
      05-03-2008

> 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.
 
Reply With Quote
 
vippstar@gmail.com
Guest
Posts: n/a
 
      05-04-2008
On May 4, 2:53 am, Igal <(E-Mail Removed)> 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.
 
Reply With Quote
 
Barry Schwarz
Guest
Posts: n/a
 
      05-04-2008
On Sat, 3 May 2008 09:33:17 -0700 (PDT), http://www.velocityreviews.com/forums/(E-Mail Removed) wrote:

>On May 3, 7:21 pm, Igal <(E-Mail Removed)> 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
 
Reply With Quote
 
vippstar@gmail.com
Guest
Posts: n/a
 
      05-04-2008
On May 4, 7:51 am, Barry Schwarz <(E-Mail Removed)> wrote:
> On Sat, 3 May 2008 09:33:17 -0700 (PDT), (E-Mail Removed) wrote:
> >On May 3, 7:21 pm, Igal <(E-Mail Removed)> 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.


 
Reply With Quote
 
Ulrich Eckhardt
Guest
Posts: n/a
 
      05-13-2008
(E-Mail Removed) wrote:
> On May 4, 7:51 am, Barry Schwarz <(E-Mail Removed)> wrote:
>> On Sat, 3 May 2008 09:33:17 -0700 (PDT), (E-Mail Removed) 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

 
Reply With Quote
 
santosh
Guest
Posts: n/a
 
      05-15-2008
Ulrich Eckhardt wrote:

> (E-Mail Removed) wrote:
>> On May 4, 7:51 am, Barry Schwarz <(E-Mail Removed)> wrote:
>>> On Sat, 3 May 2008 09:33:17 -0700 (PDT), (E-Mail Removed) 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.

 
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
malloc and realloc problem ninboy C Programming 1 05-20-2008 01:24 PM
realloc problem, corrupt last item Igal C Programming 11 05-09-2008 09:54 AM
Pointer-to-pointer realloc problem marvinla C Programming 29 07-22-2007 07:43 PM
Problem with realloc () anirbid.banerjee@gmail.com C Programming 3 02-17-2007 12:00 PM
Problem with malloc, realloc, _msize and memcpy Bren C++ 8 09-03-2003 11:01 PM



Advertisments