Dynamic Memory Allocation for string of arrays from input file

Question

I am a relatively new programmer in C and I'm trying to simply print out the content from an input file onto my screen. I have to use dynamic memory allocation and the issue I am facing is that if the number of letters in my string is >8 it overwrites it.

int main(){
FILE *input = fopen("inpit.txt","r");
int b;
char **aPtr;
int i = 0;
int j = 0;
fscanf(input,"%d",&b); //takes first value from input file which tells me number of strings in the file
aPtr = (char **)malloc(sizeof(char *)*b); 
for(i=0;i<b;i++) {
    aPtr[i]=(char *)malloc(sizeof(char));
}
for(i = 0;i < b;i++){
    fscanf(input,"%s",&aPtr[i]);
}
for(i = 0;i < b;i++){
    printf("Address %d = %d\n",i,&aPtr[i]);
}

for(i = 0;i < b;i++){
    printf("%s\n",(aPtr+i));
}
return 0; }

My input into the file inpit1.txt is:

5
grapefruit
apple
Banana
monkey
orange

If I run the file. Everything will print out fine except grapefruit. Which will be overwritten to grapefruapple.

Any help would be appreciated. Thank you in advance.

You might have gotten better results if you had not looked for C++ documentation in a C project! Do not add tags for foreign languages! And do not cast the result of `malloc` & friends in C! — too honest for this site, Nov 03 '15 at 22:27
Oh, and indent your code properly! And the signature of `main` is wrong! Write 100 times: "C is not C++ is not C!" — too honest for this site, Nov 03 '15 at 22:28
@Olaf Write 100 times: "C is not C++ is not C!" haha nice :) — LBes, Nov 03 '15 at 22:38

score 3 · Answer 1 · edited May 23 '17 at 11:44

You have a problem with your malloc which is only allocating for one char here:

aPtr[i]=(char *)malloc(sizeof(char));

Try to add a given size to it:

aPtr[i]=(char *)malloc(sizeof(char)*20);

And it should work better

Note on the casting of malloc(): Casting malloc() is not necessary in c (unless dealing with extra-old standards [pre-1989]) and can hide errors. void* is automatically promoted to any other pointer type. However it is useful if compiling as C++, since your question included both C and C++ I thought it would be good to tell you about it.

EDIT: For further information on the cast of malloc() I just found this protected question here. Feel free to check it out.

Also you need to check that malloc() was successful (the result is !=NULL)

Finally your printf is not correct and should be printf("%s\n", aPtr[i]);

score 3 · Answer 2 · answered Nov 03 '15 at 22:17

Problems I see:

You are not allocating enough memory for aPtr[i].

aPtr[i]=(char *)malloc(sizeof(char));

allocates memory for only one char. It's not enough to hold a string. You need something like:

int arraySize = 20; // Make it large enough
aPtr[i] = malloc(arraySize); // No need to use sizeof(char).
                             // It is always 1

Make sure that when you read the string, you don't overflow the array size. Instead of:

fscanf(input,"%s",&aPtr[i]);

use:

fscanf(input,"%19s", aPtr[i]);
//                  ^^^ Remove the & operator. That is wrong.
//            ^^^ Add size to prevent overflow.

You are using the wrong argument to the printf function. Instead of:

printf("%s\n",(aPtr+i));

use

printf("%s\n", *(aPtr+i));
//            ^^^ Missing pointer dereferencing operator

or

printf("%s\n", aPtr[i]);

score 0 · Answer 3 · answered Nov 03 '15 at 22:25

The first comment on your question is right at the heart of the problem.
malloc(sizeof(char)) is really just malloc(1)

What you have done is...

Allocate an array of N character pointers

aPtr = (char **)malloc(sizeof(char *)*b);

Allocate a single character to each pointer in the list

for(i=0;i<b;i++) {
  aPtr[i]=(char *)malloc(sizeof(char));
}

Copy a string of unknown length into the memory allocated to each pointer in the list.

for(i = 0;i < b;i++){
  fscanf(input,"%s",&aPtr[i]);
}

The memory allocated to each character pointer is likely to be close or contiguous to the memory allocated to its predecessor. Subsequent writes are likely to overwrite some portion of the preceding string. The the case of your example, there appears to be a sufficient gap between consecutively allocated blocks of memory that only the 'grapefruit' entry is long enough to be obviously trampled by it's neighbor.

Lukas · Answer 4 · 2015-11-03T23:46:12.780

There you go :) (I tried to explain in the comments)

int main() {

    FILE * pFile;
    char * buffer = NULL;
    size_t size = 0;
    ssize_t line_length;

    pFile = fopen("inpit.txt", "r");
    if (pFile != NULL) {
        int number_of_lines;
        fscanf(pFile, "%d", &number_of_lines);

        //create charcter pointers array to hold each line
        char* strings[number_of_lines];

        int i = -1;
        while ((line_length = getline(&buffer, &size, pFile)) != -1) {
            if (i != -1) { // skips first line because its number of lines (5)
                strings[i] = malloc(line_length * sizeof(char)); //allocate memory for the line
                sprintf(strings[i], "%s", buffer); //copy line from buffer to allocated space 
            }
            //incase file has more than it said
            if (i++ >= number_of_lines) { break; }
        }

        //Test print
        for (i = 0; i <= number_of_lines; i++) {
            printf("%s", strings[i]);
        }
        printf("\n");

        fclose(pFile);
        if (buffer) { free(buffer); }
    }
}

Dynamic Memory Allocation for string of arrays from input file

4 Answers4