using realloc multiple times question...

[Mischa]

Hello,

I am trying to use realloc multiple times to extend an array of
doubles but unfortunatly it keeps failing. I think I am mixing up the
size to which the old memory block needs to be extended. So if it's
already 128 in size it needs to be realloc'd to (ORIGSIZE + NEWSIZE)
right ? I have provided a small example below, maybe that will make my
describtion somewhat clearer...

Thank you for any suggestions..

Mischa
#include <stdio.h>
#include <stdlib.h>

#define MAXLINE 1024
#define CHUNK 128

int main(void) {
char line[MAXLINE];
double num = 0.0;
double *array = NULL;
double *tmp = NULL;
int idx = 0;
int trck = 0;

array = malloc(CHUNK * sizeof(double));
if(array == NULL) {
fprintf(stderr, "malloc error \n");
return 1;
}

while(fgets(line, MAXLINE, stdin) != NULL) {
num = strtod(line, (char **)NULL);
array[idx++] = num;
trck++;

if(trck == CHUNK) {
tmp = realloc(array, (idx + CHUNK + 1) * sizeof(double));
if(tmp != NULL) {
array = tmp;
} else {
fprintf(stderr, "realloc error\n");
free(array);
return 1;
}
}
}

return 0;
}

[Jordan Abel]

Mischa wrote:

Hello,

I am trying to use realloc multiple times to extend an array of
doubles but unfortunatly it keeps failing. I think I am mixing up the
size to which the old memory block needs to be extended. So if it's
already 128 in size it needs to be realloc'd to (ORIGSIZE + NEWSIZE)
right ? I have provided a small example below, maybe that will make my
describtion somewhat clearer...

Thank you for any suggestions..

Mischa
#include <stdio.h>
#include <stdlib.h>

#define MAXLINE 1024
#define CHUNK 128

int main(void) {
char line[MAXLINE];
double num = 0.0;
double *array = NULL;
double *tmp = NULL;
int idx = 0;
int trck = 0; array = malloc(CHUNK * sizeof(double));
if(array == NULL) {
fprintf(stderr, "malloc error \n");
return 1;
}
that's a bit verbose for my taste - could have been

if(!(array = malloc(CHUNK*sizeof *array))) {
perror("malloc"); /* this way it'll tell you why malloc failed */
exit(1);
}
while(fgets(line, MAXLINE, stdin) != NULL) {
num = strtod(line, (char **)NULL);
array[idx++] = num;
trck++;
what's trck for?
if(trck == CHUNK) {
....oh - you should really be tracking size and capacity, and assigning
capacity each time you realloc
tmp = realloc(array, (idx + CHUNK + 1) * sizeof(double));
if you're doing it this way, you'll need to reset trck to 0. but see
above for a better way to keep track of things
if(tmp != NULL) {
array = tmp;
} else {
fprintf(stderr, "realloc error\n");
free(array);
return 1;
}
}
}

return 0;
}

[slebetman]

Mischa wrote:

it needs to be realloc'd to (ORIGSIZE + NEWSIZE) right ?
correct
<snip>

while(fgets(line, MAXLINE, stdin) != NULL) {
num = strtod(line, (char **)NULL);
array[idx++] = num;
trck++;

if(trck == CHUNK) {
tmp = realloc(array, (idx + CHUNK + 1) * sizeof(double));
if(tmp != NULL) {
array = tmp;
} else {
fprintf(stderr, "realloc error\n");
free(array);
return 1;
}
}
}

Either you if statement is logically wrong or you forgot to reset trck:

/*** change to >=, this is why you didn't catch your bug ***/
if(trck >= CHUNK) {
tmp = realloc(array, (idx + CHUNK + 1) * sizeof(double));
if(tmp != NULL) {
array = tmp;
trck = 0; /*** reset trck here otherwise the next time round
*** it will always be larger than CHUNK
***/
} else {
fprintf(stderr, "realloc error\n");
free(array);
return 1;
}
}
}

Personally, I'd prefer writing something like:

if(idx >= total_size) {
tmp = realloc(array, (total_size + CHUNK) * sizeof(double));
if(tmp != NULL) {
total_size += CHUNK;
} else {
perror("realloc");
free(array);
return 1;
}
}
}

[slebetman]

Mischa wrote:

it needs to be realloc'd to (ORIGSIZE + NEWSIZE) right ?
correct
<snip>

while(fgets(line, MAXLINE, stdin) != NULL) {
num = strtod(line, (char **)NULL);
array[idx++] = num;
trck++;

if(trck == CHUNK) {
tmp = realloc(array, (idx + CHUNK + 1) * sizeof(double));
if(tmp != NULL) {
array = tmp;
} else {
fprintf(stderr, "realloc error\n");
free(array);
return 1;
}
}
}

Either you if statement is logically wrong or you forgot to reset trck:

/*** change to >=, this is why you didn't catch your bug ***/
if(trck >= CHUNK) {
tmp = realloc(array, (idx + CHUNK + 1) * sizeof(double));
if(tmp != NULL) {
array = tmp;
trck = 0; /*** reset trck here otherwise the next time round
*** it will always be larger than CHUNK
***/
} else {
fprintf(stderr, "realloc error\n");
free(array);
return 1;
}
}
}

Personally, I'd prefer writing something like:

if(idx >= total_size) {
tmp = realloc(array, (total_size + CHUNK) * sizeof(double));
if(tmp != NULL) {
array = tmp;
total_size += CHUNK;
} else {
perror("realloc");
free(array);
return 1;
}
}
}

[slebetman]

Mischa wrote:

it needs to be realloc'd to (ORIGSIZE + NEWSIZE) right ?
correct
<snip>

while(fgets(line, MAXLINE, stdin) != NULL) {
num = strtod(line, (char **)NULL);
array[idx++] = num;
trck++;

if(trck == CHUNK) {
tmp = realloc(array, (idx + CHUNK + 1) * sizeof(double));
if(tmp != NULL) {
array = tmp;
} else {
fprintf(stderr, "realloc error\n");
free(array);
return 1;
}
}
}

Either you if statement is logically wrong or you forgot to reset trck:

/*** change to >=, this is why you didn't catch your bug ***/
if(trck >= CHUNK) {
tmp = realloc(array, (idx + CHUNK + 1) * sizeof(double));
if(tmp != NULL) {
array = tmp;
trck = 0; /*** reset trck here otherwise the next time round
*** it will always be larger than CHUNK
***/
} else {
fprintf(stderr, "realloc error\n");
free(array);
return 1;
}
}
}

Personally, I'd prefer writing something like:

if(idx >= total_size) {
tmp = realloc(array, (total_size + CHUNK) * sizeof(double));
if(tmp != NULL) {
array = tmp;
total_size += CHUNK;
} else {
perror("realloc");
free(array);
return 1;
}
}
}

[Christian Bau]

In article <9c********************************@4ax.com>,
Mischa <Mi******@provided.com> wrote:

Hello,

I am trying to use realloc multiple times to extend an array of
doubles but unfortunatly it keeps failing. I think I am mixing up the
size to which the old memory block needs to be extended. So if it's
already 128 in size it needs to be realloc'd to (ORIGSIZE + NEWSIZE)
right ? I have provided a small example below, maybe that will make my
describtion somewhat clearer...

Thank you for any suggestions..

Mischa
#include <stdio.h>
#include <stdlib.h>

#define MAXLINE 1024
#define CHUNK 128

int main(void) {
char line[MAXLINE];
double num = 0.0;
double *array = NULL;
double *tmp = NULL;
int idx = 0;
int trck = 0;
I would write:
int allocated_items = CHUNK;

and forget about trck. What does "trck" mean? allocated_items counts how
many items have been allocated (don't call it allocated_size because
then nobody knows if it is size in elements or in bytes; you can call it
allocated_bytes and make it count bytes).
array = malloc(CHUNK * sizeof(double));
if(array == NULL) {
fprintf(stderr, "malloc error \n");
return 1;
}

while(fgets(line, MAXLINE, stdin) != NULL) {
num = strtod(line, (char **)NULL);
array[idx++] = num;
trck++;

if(trck == CHUNK) { if (idx >= allocated_items) {
allocated_items += CHUNK + 1;
tmp = realloc (array, allocated_items * sizeof (double));
tmp = realloc(array, (idx + CHUNK + 1) * sizeof(double));
if(tmp != NULL) {
array = tmp;
} else {
fprintf(stderr, "realloc error\n");
free(array);
return 1;
}
}
}

return 0;
}

Try to use variable names that cannot be misunderstood. Try to use
variables that express things as simple as possible. Typically when I
have an array that can be resized I will have one pointer to the array,
one variable "allocated_elements" and one variable "used_elements".
Obviously used_elements counts how many elements are used,
allocated_elements counts how many elements have been allocated.

I wouldn't allocate the array initially; just start with

array = NULL;
used_elements = 0;
allocated_elements = 0;

and increase the size of the array just before another element is
stored.

[Chuck F.]

Mischa wrote:

I am trying to use realloc multiple times to extend an array of
doubles but unfortunatly it keeps failing. I think I am mixing up
the size to which the old memory block needs to be extended. So
if it's already 128 in size it needs to be realloc'd to (ORIGSIZE
+ NEWSIZE) right ? I have provided a small example below, maybe
that will make my describtion somewhat clearer...

You have various suggestions from others. Now I suggest you take a
look at the coding of ggets, which also uses realloc to open up
storage for fgets. See:

<http://cbfalconer.home.att.net/download/ggets.zip>

--
"If you want to post a followup via groups.google.com, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers." - Keith Thompson
More details at: <http://cfaj.freeshell.org/google/>

[Stef]

Guys thank you for all the great suggestions. It made things clearer
especially regarding the tracking of size instead of counting items.

Thanks..

Mischa

On Sat, 17 Dec 2005 00:40:50 +0000, Christian Bau
<ch***********@cbau.freeserve.co.uk> wrote:

In article <9c********************************@4ax.com>,
Mischa <Mi******@provided.com> wrote:

Hello,

I am trying to use realloc multiple times to extend an array of
doubles but unfortunatly it keeps failing. I think I am mixing up the
size to which the old memory block needs to be extended. So if it's
already 128 in size it needs to be realloc'd to (ORIGSIZE + NEWSIZE)
right ? I have provided a small example below, maybe that will make my
describtion somewhat clearer...

Thank you for any suggestions..

Mischa
#include <stdio.h>
#include <stdlib.h>

#define MAXLINE 1024
#define CHUNK 128

int main(void) {
char line[MAXLINE];
double num = 0.0;
double *array = NULL;
double *tmp = NULL;
int idx = 0;
int trck = 0;

I would write:
int allocated_items = CHUNK;

and forget about trck. What does "trck" mean? allocated_items counts how
many items have been allocated (don't call it allocated_size because
then nobody knows if it is size in elements or in bytes; you can call it
allocated_bytes and make it count bytes).

array = malloc(CHUNK * sizeof(double));
if(array == NULL) {
fprintf(stderr, "malloc error \n");
return 1;
}

while(fgets(line, MAXLINE, stdin) != NULL) {
num = strtod(line, (char **)NULL);
array[idx++] = num;
trck++;

if(trck == CHUNK) {

if (idx >= allocated_items) {
allocated_items += CHUNK + 1;
tmp = realloc (array, allocated_items * sizeof (double));

tmp = realloc(array, (idx + CHUNK + 1) * sizeof(double));

if(tmp != NULL) {
array = tmp;
} else {
fprintf(stderr, "realloc error\n");
free(array);
return 1;
}
}
}

return 0;
}

Try to use variable names that cannot be misunderstood. Try to use
variables that express things as simple as possible. Typically when I
have an array that can be resized I will have one pointer to the array,
one variable "allocated_elements" and one variable "used_elements".
Obviously used_elements counts how many elements are used,
allocated_elements counts how many elements have been allocated.

I wouldn't allocate the array initially; just start with

array = NULL;
used_elements = 0;
allocated_elements = 0;

and increase the size of the array just before another element is
stored.