realloc homework help

For an assignment, part of what I have to do involves the use of malloc and realloc. I first create a 2D array of chars, the dimensions being the number of lines and the number of characters. I then use malloc to allocate enough memory to store 开发者_运维问答input from some file. Using fgets I read one line in at a time, and store it in the array. This part works fine (or so I think). The problem comes in when I try to reallocate memory for more lines if need be. The program flow is supposed to be like this:

Create a character array of 50 lines, with 80 characters per line (working)

Use fgets to read one line at a time and save it to the array (working)

When 50 lines have been read, reallocate the array to allow for 100 lines (not working)

Keep reallocating as need be (not working)

This is what I have so far (the core of it at least, I omitted irrelevant code):

#define NUMBER_OF_LINES 50
#define CHARACTERS_PER_LINE 80

FILE *inputFile = fopen("some.text", "r");

char **lines;
lines = malloc(NUMBER_OF_LINES * sizeof(*lines));
int i;
for (i = 0; i < NUMBER_OF_LINES; i++)
  *(lines+i) = malloc(CHARACTERS_PER_LINE * sizeof(char));

int linesRemaining = NUMBER_OF_LINES;
int reallocCount = 1;
i = 0;
while (!feof(inputFile)) {
  if (!linesRemaining) {
    reallocCount++;
    lines = realloc(lines, (NUM_OF_LINES * reallocCount) * sizeof(*lines));
    linesRemaining = NUM_OF_LINES;
  }
  fgets(*(lines+i), CHARS_PER_LINE, inputFile);
  i++;
  linesRemaining--;
}

My gut tells me the problem is with the realloc, so I'll explain what I think it's doing.

realloc(lines, (NUM_OF_LINES * reallocCount) * sizeof(*lines));

The first argument, lines, is the pointer I would like to reallocate a certain amount of memory. NUM_OF_LINES is the amount I would like to increase the size by. I multiply this by reallocLinesCount, which is a counter that keeps track of how many sets of 50 lines I ought to have. The sizeof(*lines) part is the size of a pointer to a char.

Thank you for reading and any help is greatly appreciated :)

EDIT: thank you all for the responses; I do not have time right now to read all of the answers right now, but all of your answers will be more thoroughly read and understood once this imminent deadline has passed :D

---

My motto is: "say what you mean". In your case, you MEAN to enlarge your array when it's not big enough to hold your data.

FILE *in;      // you fill this in
int nlines=50; // initial value
char **buffer=malloc(nlines * sizeof *buffer);
int i=0;

for(int i=0; !feof(in); ++i)
{
  if(i>=nlines)
    buffer=realloc(buffer, (nlines+=50)*sizeof *buffer);

  buffer[i]=malloc(80);
  fgets(buffer[i], 80, in);
}

---

realloc() will often find out that there is not enough available room to expand the existing array in-place; in that case, it will create an entirely new array of the specified size, copy the contents of the old array to the new one, deallocate the old array, and return a pointer to the new one. So you should write

char **oldLines = lines;
lines = realloc(...);

(the purpose of oldLines is to keep the original pointer in case realloc() runs out of memory and returns NULL, as per @Brian L's tip).

---

This is how you should realloc:

char **new_lines = realloc(lines, (NUM_OF_LINES * ++reallocLinesCount) * sizeof(*lines));
if (new_lines)
{
    lines = new_lines;
}
else
{
    // Memory allocation fails. Do some error handling.
}

Read realloc reference for details.

EDIT

You need more allocation for each new lines.

---

You are allocating more pointers to lines but not the lines themselves. It is in your code at the beginning:

for (i = 0; i < NUMBER_OF_LINES; i++)
   *(lines+i) = malloc(CHARACTERS_PER_LINE * sizeof(char));

So after you allocated your number of lines for each line you allocate the space for the line itself. You forgot to do this for the new lines when you reallocate.

---

Let's first see how realloc() works. It returns a pointer to new memory on success, and NULL on failure. On failure, it doesn't touch the old memory, and on success, it free()'s it, after copying your data to the new place.

So, the way to use realloc() safely is:

/* allocate memory using malloc() */
ptr = malloc(N * sizeof *ptr);
/* make sure malloc succeeded */
...
/* call realloc() */
new_ptr = realloc(ptr, M * sizeof *new_ptr);
/* see if it succeeded */
if (new_ptr) {
    /* okay, we can set ptr */
    ptr = new_ptr;
} else {
    /* realloc failed, old pointer still valid */
}

So, the first thing is that you are using realloc() incorrectly. You should never say x = realloc(x, ...);, because if realloc() fails, you assign x to NULL, and the old memory is lost. This is a memory leak.

Now, on to your problem. Let's say you have successfully read NUMBER_OF_LINES lines. Now you want to make room for an additional NUMBER_OF_LINES lines. You would do:

char **new_lines = realloc(lines, NUMBER_OF_LINES*reallocCount*sizeof *new_lines);
if (new_lines) {
    lines = new_lines;
} else {
    fprintf(stderr, "realloc failed!\n");
    return;
}

/* Now, lines[NUMBER_OF_LINES] to lines[2*NUMBER_OF_LINES-1] are
 * available to point someplace useful.  They don't point anywhere
 * useful yet.  We have to allocate memory for them, just like earlier */

start = NUMBER_OF_LINES*reallocCount;
for (i=0; i < NUMBER_OF_LINES; ++i) {
    /* You weren't allocating memory here, and were writing to
     * lines[0] through lines[NUMBER_OF_LINES-1], which is not what
     * you want. */
    lines[start+i] = malloc(CHARS_PER_LINE * sizeof *lines[start+i]);
    /* check the result of malloc here */
}
fgets(lines[start+i], CHARS_PER_LINE, inputFile);

One final note: it's almost always wrong to use while (!feof(fp)) to read lines from a file.