realloc problem, corrupt last item

Re: realloc problem, corrupt last item

Antoninus Twink <nospam@nospam. invalidwrites:

On 9 May 2008 at 5:32, Richard Heathfield wrote:

>The OP replied to the above, quoting a little of it, and then Chuck replied
>to that reply:

>>>
>> if (tmp = realloc(bp, sizeof book) bp = tmp; /* ok, update */
>> else {
>> /* tmp == NULL, realloc failed, do whatever is needed */
>> /* Note that the original content of *bp still exists */
>> }

>>
>I'm curious to know what information Chuck thinks he added here.

>
He added a syntax error in the if line, and gave the OP an example of
hideous C style: his idiotic inistence on one-line if statements that
makes code hard-to-read and debugging even harder, plus a completely
vacuous comment.

Does he really help people? But I must be in agreement with you that
this style would not be allowed on my projects. It is horrible and very
amateur. Possibly Chuck is a new C programmer and learning though it is
better to be patient.

Re: realloc problem, corrupt last item

On 9 May 2008 at 5:32, Richard Heathfield wrote:

The OP replied to the above, quoting a little of it, and then Chuck replied
to that reply:

>>
> if (tmp = realloc(bp, sizeof book) bp = tmp; /* ok, update */
> else {
> /* tmp == NULL, realloc failed, do whatever is needed */
> /* Note that the original content of *bp still exists */
> }

>
I'm curious to know what information Chuck thinks he added here.

He added a syntax error in the if line, and gave the OP an example of
hideous C style: his idiotic inistence on one-line if statements that
makes code hard-to-read and debugging even harder, plus a completely
vacuous comment.

Re: realloc problem, corrupt last item

<snip>

I wrote:
"You have not provided sufficient information for me to help you
effectively. On general principles, lose the cast, replace sizeof(book)
with sizeof *bp, use a temp to catch the result of realloc in case it
fails and returns NULL, and make sure you have #included <stdlib.h>. Note
that bp is a copy of the calling function's pointer; the original will not
be updated automatically, so if you want to keep the new value you must
provide a mechanism for the caller to assign this new value to the
original pointer."

The OP replied to the above, quoting a little of it, and then Chuck replied
to that reply:

Nor should you cast the output from realloc. It just hides error
messages. Also NEVER receive the result from realloc in the
original pointer. Create a temporary, and do:
>
if (tmp = realloc(bp, sizeof book) bp = tmp; /* ok, update */
else {
/* tmp == NULL, realloc failed, do whatever is needed */
/* Note that the original content of *bp still exists */
}
>
The above doesn not address problems with the 'sizeof book' that
you pass to realloc.

I'm curious to know what information Chuck thinks he added here.

--
Richard Heathfield <http://www.cpax.org.uk >
Email: -http://www. +rjh@
Google users: <http://www.cpax.org.uk/prg/writings/googly.php>
"Usenet is a strange place" - dmr 29 July 1999

Re: realloc problem, corrupt last item

"K. Jennings" wrote:

Richard Heathfield wrote:

>Igal said:
>>

.... snip ...

>>

>>book* AddBook(book *bp, unsigned *size) { ...
>>// then i use realloc to allocate space for the new item in the
>>// bp pointer
>>bp = (book*)realloc( bp, sizeof(book)); ...

>>
>You have not provided sufficient information for me to help you
>effectively. On general principles, lose the cast, replace
>sizeof(book) with sizeof *bp,

>
Why? Under what circumstances they would not return the same value?

Nor should you cast the output from realloc. It just hides error
messages. Also NEVER receive the result from realloc in the
original pointer. Create a temporary, and do:

if (tmp = realloc(bp, sizeof book) bp = tmp; /* ok, update */
else {
/* tmp == NULL, realloc failed, do whatever is needed */
/* Note that the original content of *bp still exists */
}

The above doesn not address problems with the 'sizeof book' that
you pass to realloc.

--
[mail]: Chuck F (cbfalconer at maineline dot net)
[page]: <http://cbfalconer.home .att.net>
Try the download section.


** Posted from http://www.teranews.com **

Re: realloc problem, corrupt last item

K. Jennings wrote:

On Thu, 08 May 2008 14:27:23 +0000, Richard Heathfield wrote:
>

>Igal said:
>>

>>hay, i have this werid problem with my book adding function, this how
>>it looks
>>>
>>book* AddBook(book *bp, unsigned *size) { ...
>>//then i use realloc to allocate space for the new item in the bp
>>pointer
>>bp = (book*)realloc( bp, sizeof(book)); ...
>>}

>You have not provided sufficient information for me to help you
>effectively. On general principles, lose the cast, replace sizeof(book)
>with sizeof *bp,

>
Why? Under what circumstances they would not return the same
value?

When bp turns out not to be a book* but a book** or a
booklet* or a Book* or ...

Suppose that the compiler raises no objections to

ptr = malloc(sizeof(s truct messagedetail)) ;

Can you tell whether the malloc() call requests the correct
amount of memory for ptr to point at? Not until you scroll
around for a while to find the declaration of ptr to make
sure it's not a `struct messagedigest*' instead. But if
it's written as

ptr = malloc(sizeof *ptr);

.... you can tell at a glance that it's correct.

--
Eric.Sosman@sun .com

Re: realloc problem, corrupt last item

"K. Jennings" wrote:

>
On Thu, 08 May 2008 14:27:23 +0000, Richard Heathfield wrote:

[...]

bp = (book*)realloc( bp, sizeof(book)); ...
}

You have not provided sufficient information for me to help you
effectively. On general principles, lose the cast, replace sizeof(book)
with sizeof *bp,

>
Why? Under what circumstances they would not return the same
value?

In six months, when bp is changed to "book_v2 *". :-)

And, removing the unnecessary cast can uncover a hidden problem if
the proper header file isn't included.

--
+-------------------------+--------------------+-----------------------+
| Kenneth J. Brody | www.hvcomputer.com | #include |
| kenbrody/at\spamcop.net | www.fptech.com | <std_disclaimer .h|
+-------------------------+--------------------+-----------------------+
Don't e-mail me at: <mailto:ThisIsA SpamTrap@gmail. com>

Re: realloc problem, corrupt last item

On Thu, 08 May 2008 14:27:23 +0000, Richard Heathfield wrote:

Igal said:
>

>hay, i have this werid problem with my book adding function, this how
>it looks
>>
>book* AddBook(book *bp, unsigned *size) { ...
>//then i use realloc to allocate space for the new item in the bp
>pointer
>bp = (book*)realloc( bp, sizeof(book)); ...
>}

>
You have not provided sufficient information for me to help you
effectively. On general principles, lose the cast, replace sizeof(book)
with sizeof *bp,

Why? Under what circumstances they would not return the same
value?

Re: realloc problem, corrupt last item

Igal wrote:
[...]

bp = (book*)realloc( bp, sizeof(book));

Here, you allocate room for one book.

I assume you really want:

bp = realloc(bp, sizeof(*bp) * n);

which will allocate room for n books.

[...]

bp[n-1].cn = temp.cn;
bp[n-1].Units = temp.Units;
bp[n-1].Year = temp.Year;
bp[n-1].Price.CostPric e = temp.Price.Cost Price;
bp[n-1].Price.RetailPr ice = temp.Price.Reta ilPrice;

Unless n==1, all of the above invoke UB by acessing memory
that does not belong to bp.

[...]

--
+-------------------------+--------------------+-----------------------+
| Kenneth J. Brody | www.hvcomputer.com | #include |
| kenbrody/at\spamcop.net | www.fptech.com | <std_disclaimer .h|
+-------------------------+--------------------+-----------------------+
Don't e-mail me at: <mailto:ThisIsA SpamTrap@gmail. com>

Re: realloc problem, corrupt last item

Igal wrote:

On May 8, 5:36 pm, "Jim Langston" <tazmas...@rock etmail.comwrote :

>Igal wrote:

>>hay, i have this werid problem with my book adding function, this
>>how it looks

>>

>>book* AddBook(book *bp, unsigned *size) {
>>...
>>//then i use realloc to allocate space for the new item in the bp
>>pointer
>>bp = (book*)realloc( bp, sizeof(book));

>>
>realloc accepts the pointer to the prevoiusly allocated block, and
>the new size. Since you are passing sizeof( book ) you are only
>allocating enough space for 1 item.
>>
>You probably meant something like:
>bp = realloc( bp, sizeof( book ) * *size );
>or
>bp = realloc( bp, sizeof( book ) * (*size + 1) )
>or something. I'm not sure how many additional items you want to
>allocate for nor what size represents (new size or old size).

>
in this function i need to reallocate only space for one new item.
*size is the number of items i have in my array.
correct me if i'm wrong, but if i realloc the size of my array+1,
won't this allocate + n items to the original array?

No, realloc accpets the *new size* Total new size. How big you want the
array to be. Reguardless of how big it currently is. Which is current size
+ 1, or in your case (*size + 1) or with the source you gave before, n.

--
Jim Langston
tazmaster@rocke tmail.com

Re: realloc problem, corrupt last item

Igal wrote:

>You have not provided sufficient information for me to help you
>effectively. On general principles, lose the cast, replace
>sizeof(book) with sizeof *bp, use a temp to catch the result of
>realloc in case it fails and returns NULL, and make sure you have
>#included <stdlib.h>. Note that bp is a copy of the calling
>function's pointer; the original will not be updated automatically,
>so if you want to keep the new value you must provide a mechanism
>for the caller to assign this new value to the original pointer.

>
here's the full function.
i call it like this:
>
bp = AddBook(bp, &BOOK_SIZE);
>
/*############## ###############
# Add Book Function #
############### ##############*/
book* AddBook(book *bp, unsigned *size)
{
book temp;
unsigned n = *size; //n = size of book array
bool check = FALSE;
char str_cn[80];
int i;
>
printf("%d", (int)(sizeof(bp )/sizeof(bp[0])));

bp is a book*. Therefore, it's size is the size of a pointer (most likely 4
bytes). This is most likely not giving you what you want. You need to keep
track of the size for dynamically allocated arrays.

n++;
bp = (book*)realloc( bp, sizeof(book));

And here you are allocating space for *one* book. You need to allocate size
+ 1 (which is in n at this point) so this line should become:

bp = realloc( bp, sizeof(book) * n );

if (bp == NULL) { perror("ERROR [realloc - AddBook()]");
exit(ERR_REALOC ); }
>
printf("\n[Add Book to Catalog]\n");
>
//get catalog number
while(check != TRUE)
{
printf("Book Catalog Number: ");
_flushall();
gets(str_cn);
>
//check if string is longer then 9 digits. if longer then error.
if(strlen(str_c n) 9) { check = FALSE; printf("[E] catalog number
too long, 9 digit max.\n"); continue; }
>
//check each letter in string if a digit, if all are then check =
TRUE, else check = FALSE
i=0;
while(str_cn[i])
{
if(isdigit(str_ cn[i])) { check = TRUE; i++; continue; }
else { check = FALSE; printf("[E] catalog number not valid, try
again.\n"); break; }
}
>
if(check == TRUE) { temp.cn = atol(str_cn); } //if all OK, put value
in str_cn
}
check = FALSE;
>
... here i get data into temp, and check validation ...
>
*size = n;
>
bp[n-1].cn = temp.cn;
bp[n-1].Units = temp.Units;
bp[n-1].Year = temp.Year;
bp[n-1].Price.CostPric e = temp.Price.Cost Price;
bp[n-1].Price.RetailPr ice = temp.Price.Reta ilPrice;
>
strcpy(bp[n-1].Publisher, temp.Publisher) ;
strcpy(bp[n-1].BookName, temp.BookName);
strcpy(bp[n-1].Author.Author1 , temp.Author.Aut hor1);
strcpy(bp[n-1].Author.Author2 , temp.Author.Aut hor2);
strcpy(bp[n-1].Author.Author3 , temp.Author.Aut hor3);
>
return bp;
}

--
Jim Langston
tazmaster@rocke tmail.com

Re: realloc problem, corrupt last item

On May 8, 5:36 pm, "Jim Langston" <tazmas...@rock etmail.comwrote :

Igal wrote:

hay, i have this werid problem with my book adding function, this how
it looks

>

book* AddBook(book *bp, unsigned *size) {
...
//then i use realloc to allocate space for the new item in the bp
pointer
bp = (book*)realloc( bp, sizeof(book));

>
realloc accepts the pointer to the prevoiusly allocated block, and the new
size. Since you are passing sizeof( book ) you are only allocating enough
space for 1 item.
>
You probably meant something like:
bp = realloc( bp, sizeof( book ) * *size );
or
bp = realloc( bp, sizeof( book ) * (*size + 1) )
or something. I'm not sure how many additional items you want to allocate
for nor what size represents (new size or old size).

in this function i need to reallocate only space for one new item.
*size is the number of items i have in my array.
correct me if i'm wrong, but if i realloc the size of my array+1,
won't this allocate + n items to the original array?

Jim Langston
tazmas...@rocke tmail.com

Re: realloc problem, corrupt last item

You have not provided sufficient information for me to help you

effectively. On general principles, lose the cast, replace sizeof(book)
with sizeof *bp, use a temp to catch the result of realloc in case it
fails and returns NULL, and make sure you have #included <stdlib.h>. Note
that bp is a copy of the calling function's pointer; the original will not
be updated automatically, so if you want to keep the new value you must
provide a mechanism for the caller to assign this new value to the
original pointer.

here's the full function.
i call it like this:

bp = AddBook(bp, &BOOK_SIZE);

/*############## ###############
# Add Book Function #
############### ##############*/
book* AddBook(book *bp, unsigned *size)
{
book temp;
unsigned n = *size; //n = size of book array
bool check = FALSE;
char str_cn[80];
int i;

n++;
bp = (book*)realloc( bp, sizeof(book));
if (bp == NULL) { perror("ERROR [realloc - AddBook()]");
exit(ERR_REALOC ); }

.... here i get data into temp, and check validation ...

*size = n;

bp[n-1].cn = temp.cn;
bp[n-1].Units = temp.Units;
bp[n-1].Year = temp.Year;
bp[n-1].Price.CostPric e = temp.Price.Cost Price;
bp[n-1].Price.RetailPr ice = temp.Price.Reta ilPrice;

strcpy(bp[n-1].Publisher, temp.Publisher) ;
strcpy(bp[n-1].BookName, temp.BookName);
strcpy(bp[n-1].Author.Author1 , temp.Author.Aut hor1);
strcpy(bp[n-1].Author.Author2 , temp.Author.Aut hor2);
strcpy(bp[n-1].Author.Author3 , temp.Author.Aut hor3);

return bp;
}

Re: realloc problem, corrupt last item

Igal wrote:

hay, i have this werid problem with my book adding function, this how
it looks
>
book* AddBook(book *bp, unsigned *size) {
...
//then i use realloc to allocate space for the new item in the bp
pointer
bp = (book*)realloc( bp, sizeof(book));

realloc accepts the pointer to the prevoiusly allocated block, and the new
size. Since you are passing sizeof( book ) you are only allocating enough
space for 1 item.

You probably meant something like:
bp = realloc( bp, sizeof( book ) * *size );
or
bp = realloc( bp, sizeof( book ) * (*size + 1) )
or something. I'm not sure how many additional items you want to allocate
for nor what size represents (new size or old size).

...
}
>
when i do this, the next them is added in this right space. but let's
say *bp hold 2 book type items, first item will be ok, second one will
get garbage content, then the third item (the new reallocated one)
will get the new data.
i just have no idea will the last item gets corruped, this also
happens with more items in array.
>
i know that to use linked lists is a lot better solution, but i still
dunno how to use them right just yet.

--
Jim Langston
tazmaster@rocke tmail.com

Re: realloc problem, corrupt last item

You have not provided sufficient information for me to help you

effectively. On general principles, lose the cast, replace sizeof(book)
with sizeof *bp, use a temp to catch the result of realloc in case it
fails and returns NULL, and make sure you have #included <stdlib.h>. Note
that bp is a copy of the calling function's pointer; the original will not
be updated automatically, so if you want to keep the new value you must
provide a mechanism for the caller to assign this new value to the
original pointer.

here's the full function.
i call it like this:

bp = AddBook(bp, &BOOK_SIZE);

/*############## ###############
# Add Book Function #
############### ##############*/
book* AddBook(book *bp, unsigned *size)
{
book temp;
unsigned n = *size; //n = size of book array
bool check = FALSE;
char str_cn[80];
int i;

printf("%d", (int)(sizeof(bp )/sizeof(bp[0])));

n++;
bp = (book*)realloc( bp, sizeof(book));
if (bp == NULL) { perror("ERROR [realloc - AddBook()]");
exit(ERR_REALOC ); }


printf("\n[Add Book to Catalog]\n");

//get catalog number
while(check != TRUE)
{
printf("Book Catalog Number: ");
_flushall();
gets(str_cn);

//check if string is longer then 9 digits. if longer then error.
if(strlen(str_c n) 9) { check = FALSE; printf("[E] catalog number
too long, 9 digit max.\n"); continue; }

//check each letter in string if a digit, if all are then check =
TRUE, else check = FALSE
i=0;
while(str_cn[i])
{
if(isdigit(str_ cn[i])) { check = TRUE; i++; continue; }
else { check = FALSE; printf("[E] catalog number not valid, try
again.\n"); break; }
}

if(check == TRUE) { temp.cn = atol(str_cn); } //if all OK, put value
in str_cn
}
check = FALSE;

... here i get data into temp, and check validation ...

*size = n;

bp[n-1].cn = temp.cn;
bp[n-1].Units = temp.Units;
bp[n-1].Year = temp.Year;
bp[n-1].Price.CostPric e = temp.Price.Cost Price;
bp[n-1].Price.RetailPr ice = temp.Price.Reta ilPrice;

strcpy(bp[n-1].Publisher, temp.Publisher) ;
strcpy(bp[n-1].BookName, temp.BookName);
strcpy(bp[n-1].Author.Author1 , temp.Author.Aut hor1);
strcpy(bp[n-1].Author.Author2 , temp.Author.Aut hor2);
strcpy(bp[n-1].Author.Author3 , temp.Author.Aut hor3);

return bp;
}

Re: realloc problem, corrupt last item

Igal said:

hay, i have this werid problem with my book adding function, this how
it looks
>
book* AddBook(book *bp, unsigned *size) {
...
//then i use realloc to allocate space for the new item in the bp
pointer
bp = (book*)realloc( bp, sizeof(book));
...
}

You have not provided sufficient information for me to help you
effectively. On general principles, lose the cast, replace sizeof(book)
with sizeof *bp, use a temp to catch the result of realloc in case it
fails and returns NULL, and make sure you have #included <stdlib.h>. Note
that bp is a copy of the calling function's pointer; the original will not
be updated automatically, so if you want to keep the new value you must
provide a mechanism for the caller to assign this new value to the
original pointer.

For more specific help, post more specific code. "..." doesn't say anything
useful, and you haven't shown the calling code.

<snip>

--
Richard Heathfield <http://www.cpax.org.uk >
Email: -http://www. +rjh@
Google users: <http://www.cpax.org.uk/prg/writings/googly.php>
"Usenet is a strange place" - dmr 29 July 1999