Morten Baagøe
Morten Baagøe

Reputation: 81

malloc problems cause segmentation fault

We are currently working on a project where we need to work on some text, to do this we need to split the text up in smaller sections.

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

typedef struct paragraph{
   char **words;
}paragraph;

typedef struct text{
   char name[100];
   paragraph  *list;
}text;

void readFileContent(FILE *file, paragraph *pa, int size){

   char localString[100];

   pa->words = (char **)malloc(size * sizeof(char *));
   int i = 0, z;

   while(fscanf(file, "%s", localString) == 1 && i < size){
      z = strlen(localString);
      pa->words[i] = (char *)malloc(z + 1);

      strcpy(pa->words[i], localString);
      i++;
   }

}

void main(){
      int i = 0, n, z;
   FILE *file;
   text *localText;
   localText = (text *)malloc(sizeof(text));

   openFile(&file, "test.txt");
   i = countWords(file);

   i = i / 50 + 1; // calculate the number of section need for the text

   localText->list = calloc(sizeof(paragraph *), i);

   for(n = 0; n < i ; n++){
      printf("Paragraph - %d\n", n);
      readFileContent(file, &localText->list[i], 50);

   }

   for(n = 0; n < i ; n++){
      printf("Paragraph - %d", n);
      for(z = 0; z < 50; z++){
      printf("no. %d\n", z);
      printf("%s\n", localText->list[n].words[z]);
      }
   }

}

When I try to run the program, I get a segmentation fault on the print loop in the bottom. I think it is caused by some problem with allocating memory, but I can't figure out why.

Update 1 I have changed the code to use a 3 dimensional array to store the text segments, but I still get a segmentation fault when i try to allocate memory using malloc.

localText->list[i][n] = malloc(100 * sizeof(char));

her is the changed code.

typedef struct {
   char name[100];
   char  ***list;
}text;

int main(){
   int i = 0, n, z,wordCount, sections;
   FILE *file;
   text *localText;

   openFile(&file, "test.txt");
   wordCount = countWords(file);


   sections = (wordCount / 50) + 1;

   localText = malloc(sizeof(text));
   localText->list = malloc(sections * sizeof(char **));

   for(i = 0; i < sections; i++)
      localText->list[i] = malloc(50 * sizeof(char *));
      for(n = 0; n < 50; n++)
         localText->list[i][n] = malloc(100 * sizeof(char));

   readFileContent(file, localText->list, 50);

   freeText(localText);

   return 1;

}

Upvotes: 0

Views: 3207

Answers (3)

Lundin
Lundin

Reputation: 215330

There are lots of bugs in your code. Here are the most severe ones:

1) A pointer-to-pointer is not a multi-dimensional array. If you use pointer-to-pointer to access a multi-dimensional, dynamically allocated array, then that array needs to be allocated in a way that makes sense for a pointer-to-pointer.

It appears like you are trying to allocate an array of pointers dynamically, and then for each pointer in that array, allocate an array of data. However, your code does not do this, you have far too many levels of indirection for your code to make any sense. For example paragraph *list;, why would you need a pointer to a struct containing a pointer to a pointer?

You need to simplify your data structures. I propose to do like this instead:

typedef struct {
   char   name[100];
   char** list;
} text;

2) Don't name the typedef the same thing as the struct tag, this will get you in namespace conflict trouble sooner or later. You don't even need a struct tag when typedef:ing a struct, instead do as in my example above.

3) Never typecast the result of malloc/calloc in the C language. This hides away compiler warnings and bugs. Countless of detailed posts regarding the reasons why can be found here, on SO.

4) Since this is a hosted program running on an OS (I can tell by the use of file handling), main cannot return anything but int. Change your definition of main to int main() or it will not compile on a standard C compiler.

5) for(n = 0; n < i ; n++) ... list[i]. As you can tell by your own code, it is not a good idea to use the variable name i for anything but a loop iterator. (i actually stands for iterator). That's why you got a bug there.

6) You must close the open file when you are done with it, through fclose().

7) You must deallocate the dynamically allocated memory when you are done with it, through free().

Upvotes: 7

AndersK
AndersK

Reputation: 36092

seems you have done a typo here

for(n = 0; n < i ; n++){
      printf("Paragraph - %d\n", n);
      readFileContent(file, &localText->list[i], 50);

   }

shouldn't it be

for(n = 0; n < i ; n++){
      printf("Paragraph - %d\n", n);
      readFileContent(file, &localText->list[n], 50);

   }

Upvotes: 0

thiton
thiton

Reputation: 36059

 readFileContent(file, &localText->list[i], 50);

You are initializing the one-past-the-last-th element here, while not initializing all other list elements. Try list[n] instead.

Upvotes: 5

Related Questions