problem with realloc

[Igal]

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;
}

 

[vippstar]

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;

 

[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;
}`

Say hey.

Jim

 

[Ulrich Eckhardt]

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

 

[Igal]

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.

 

[vippstar]

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.

 

[Barry Schwarz]

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

 

[vippstar]

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.

 

[Ulrich Eckhardt]

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

 

[santosh]

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.


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.