allocating memory as the program runs

Question

Hi there I am trying to write a program that will create memory during run time, such that the program could run continuously as it generates memory to store strings until the user decides to quit. However, when I run the program I can enter the strings properly, however printing them out is another story. I believe the issue is that in the loop I am overwriting the memory that was created through each iteration. Here is what I have thus far:

int main(){
    char **lines = NULL;
    char sentence[1000];
    int numOfLines = 1;
    int i;
    int j;

    printf("Enter the sentence:\n");
    lines = (lines, numOfLines * sizeof *lines); 

    for (i=0; i <= numOfLines; i++){

        fgets(sentence, 1000, stdin);
        lines[i] = malloc (sizeof(char) * strlen(sentence) +1);
        if (strcmp(sentence, ".\n") == 0){ //exits loops if entered string is "."
              break;
        }
        strcpy(lines[i], sentence); 
        numOfLines++;
        printf("%s", lines[i]); // attempt at a Debug statement
    }
   numOfLines = numOfLines - 1;

   for (j = numOfLines; j>=0; j--){ //prints out the lines in reverse
       printf("%s\n", lines[j]);
   }
   return 0;

}

I might add that I get a segmentation fault when the user exits the loop.

Answer 1

lines[i] = malloc (sizeof(char) * strlen(sentence + 1));

This is a problem. Should be

lines[i] = malloc (sizeof(char) * strlen(sentence) + 1);

Answer 2

Issues

You do this on each loop

lines = malloc(sizeof(char *) * numOfLines); 

If you exit early lines not used are full of random gunk

Try (Note: FIX the NYI's)

#include <stdio.h>
#include <strings.h>
#include <string.h>
#include <stdlib.h>

int main(){
    char sentence[1000]; // NYI - fix magic
    int numOfLines = 0;
    int maxNumOfLines = 10;
    char **lines=malloc(sizeof(char *) * maxNumOfLines); // NYI - calloc

    bzero(lines, sizeof(char *) * maxNumOfLines); // NYI - calloc

    printf("Enter the sentence:\n");

    for (int i=0; i < maxNumOfLines; i++) { 
      fgets(sentence, 1000, stdin);
      lines[i] = malloc (sizeof(char) * strlen(sentence) + 1);

      if (strcmp(sentence, ".\n") == 0){
        strcpy(lines[i], ".\n");
        break;
      }

      strcpy(lines[i], sentence);
      numOfLines++;
      //printf("%s", lines[i]); // intial Debug statement
    }

    for (int j = numOfLines-1; j>=0; j--){
      printf("%s\n", lines[j]);
    }
    return 0;

}

Note that you need to either set a maximum and exit the loop before that max is reached or get the user to enter the maximum and set it dynamically.

Answer 3

 lines = malloc(sizeof(char *) * numOfLines); 

On each iteration, you're allocating a completely new array, and the old one is lost (not freed, and no longer addressable). What you want here is realloc(3).

 lines[i] = malloc (sizeof(char) * strlen(sentence + 1));

As Andrew Jenkins mentions, you want strlen(sentence) + 1. Your code is allocating 2 fewer bytes than you need. (Consider what sentence + 1 is.) A less error-prone idiom is 1 + strlen(sentence).

Posix defines a more convenient function, strdup(3), that helps avoid this kind of fencepost error.

One word of advice, if I may: You're not checking that your allocations return a valid value. Even though memories are big nowadays (and, on Linux, frequently bigger than actual) correct logic deals with errors from any function call.

Answer 4

Another way to fix your code, not mentioned yet, is to change:

lines = malloc(sizeof(char *) * numOfLines); 

to:

lines = realloc(lines, numOfLines * sizeof *lines);

(Note, I use the recommended idiom for sizeof to increase code robustness). And have before the loop, char **lines = NULL;.

Then the memory block containing the line pointers will be increased in size as necessary.

Note that you should check the return value of malloc and realloc and take appropriate action if it returned NULL. If you want to be able to recover the program in the case of realloc failure, see here.