-2

Hey i would like some help on this i am trying to load a dictionary on my c programm but i take segment fault. I would love some help. while debuging with gdb it says that it failed on line 63 which the command says : lines[i]=string_coppied

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

const int step=200;
char **loadfile();


int main()
{   
  char **words=loadfile();
  if (words==NULL) printf ("cant build structure");
  for (int i=0 ; i<100 ; i++) printf("%s \n" , words[i] );
}

char **loadfile()
{
  //We load our text file at the stack
  FILE * fpointer;
  fpointer = fopen ("word.txt" , "r");
  if (fpointer==NULL) printf ("file not loaded \n");
  else printf ("File loaded \n");

  int array_size=step;
  char ** lines=(char **)malloc(array_size*sizeof(char*));
  if (lines=NULL) printf ("cant allocate memory \n");

  char temp[100];
  int i=0;

  while (fgets(temp,100,fpointer))
  {
    //we check if the already allocated memory is full , if so we realloc
    if (i==array_size)
    { 
      array_size +=step;

      char **newlines= (char**)realloc(lines , array_size * sizeof(char*));
      if (newlines==NULL) //check if the memory was allocated
      {
         printf ("Cant reallocate memory , please try again \n ");
         return 0 ;
      }

      lines=newlines;
    }
    //now that we made sure that the memory was allocated we continue by copying the temp //
    temp[strlen(temp)-1]="\0" ;
    int length =strlen(temp);
    char * string_coppied=malloc((length+1) * sizeof(char));
    strcpy(string_coppied ,temp);
    lines[i]=string_coppied;
    i++;
  }
  return lines;

  free(lines);
  fclose(fpointer);
}
bruno
  • 32,421
  • 7
  • 25
  • 37
  • you already accepted my answer, but I edited my answer to add a proposal for your program – bruno Mar 18 '19 at 23:41
  • i tried to modify my programm with the notes you made but i was still getting core dumped while your proposal is working perfectly i will study yours and try to understand my mistakes. Sry i am inexperienced – Stelios V. Mar 18 '19 at 23:50
  • it is possible I missed something in your program, I just looked at it without trying to modify and make it running ;-) If you can use _valgrind_ run your program under it to check the memory accesses and more, _valgrind_ is a fantastic tool – bruno Mar 18 '19 at 23:54

1 Answers1

0

gdb it says that it failed on line 63 which the command says : lines[i]=string_coppied

this is because after

char **newlines= (char**)realloc(lines , array_size * sizeof(char*));

you cannot use lines after the realloc because if is potentially freed by realloc, and visibly this is what happened because gdb signals the problem


But you have more problems in your code

1) the compiler signal an error on the line temp[strlen(temp)-1]="\0" ; because temp is an array of char, not an array of char*, you certainly wanted temp[strlen(temp)-1] = 0 ;

2) in main :

 if (words==NULL) printf ("cant build structure");
 for (int i=0 ; i<100 ; i++) printf("%s \n" , words[i] );

an else is missing, if worlds is NULL you do the for and (try to) access to words[i] with an undefined behavior

3) in main

for (int i=0 ; i<100 ; i++) printf("%s \n" , words[i] );

you access to the 100 first entries of words to print them without knowing its size/how many entries it has, if it has are less than 100 entries you have an other undefined behavior, if it has more you do not print all of them

loadfile must return the number of entries, for instance through an output variable char **loadfile(size_t * n) allowing you to do for (int i=0 ; i<n ; i++) ...

4) in loadfile

 fpointer = fopen ("word.txt" , "r");
 if (fpointer==NULL) printf ("file not loaded \n");
 else printf ("File loaded \n");

if you cannot open file file you print a message and you continue the execution, you need to return NULL;

Note you are also optimistic because you already print File loaded while you just open the file

5) in loadfile

 char ** lines=(char **)malloc(array_size*sizeof(char*));
 if (lines=NULL) printf ("cant allocate memory \n");

if malloc returns NULL you print the message and you continue the execution, so you can do later lines[i]=string_coppied; while lines is NULL with an undefined behavior

6) in loadfile

As I said

 char **newlines= (char**)realloc(lines , array_size * sizeof(char*));
 if (newlines==NULL) //check if the memory was allocated
 {
     printf ("Cant reallocate memory , please try again \n ");
     return 0 ;
 }

 lines=newlines;

can be replaced by

  lines= (char**)realloc(lines , array_size * sizeof(char*));
  if (lines==NULL) //check if the memory was allocated
  {
      printf ("Cant reallocate memory , please try again \n ");
      return NULL;
  }

7) in loadfile

when doing (I changed "\0" to 0) :

temp[strlen(temp)-1]=0;
int length =strlen(temp);

you compute 2 times the length of the string while you know the second strlen values the first minus 1, do

int length = strlen(temp) - 1;

temp[length] = 0;

but there is an other problem, you do that to remove the newline, but

  • the last line of the file may not contain a new line and you lost the last character in that case.
  • when a line is greater than 100 characters it is cut on several, and the first sub lines do not contain a newline and you remove their last character

To not cut the lines, and to not have to duplicate them too, you can use getline

8) in loadfile

In

return lines;

free(lines);
fclose(fpointer);

the last two lines cannot be executed so you do not free lines (but that is better because you return it) and the file is not closed


A proposal :

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

const int step=200;
char **loadfile(ssize_t * n);

int main()
{   
  ssize_t n;
  char **words=loadfile(&n);

  if (words == NULL)
    puts("error when reading file");
  else {
    for (ssize_t i = 0 ; i < n ; i++) {
      puts(words[i]);
      free(words[i]);
    }
  }
}

char **loadfile(ssize_t * n)
{
  *n = 0;

  //We load our text file at the stack
  FILE * fpointer = fopen ("word.txt" , "r");

  if (fpointer==NULL) {
    printf ("file not loaded \n");
    return NULL;
  }
  puts ("load file");

  int array_size = 0;
  char ** lines = NULL;

  for (;;) {
    char * lineptr = NULL;
    size_t zero = 0;
    ssize_t sz = getline(&lineptr, &zero, fpointer);

    if (sz <= 0) {
      fclose(fpointer);
      return lines;
    }

    if (lineptr[sz - 1] == '\n')
      lineptr[sz - 1] = 0;

    if (*n == array_size)
    { 
      array_size += step;
      lines = realloc(lines , array_size * sizeof(char*));

      if (lines == NULL) //check if the memory was allocated
      {
         printf ("Cant reallocate memory , please try again \n ");
         return NULL;
      }
    }

    lines[*n] = lineptr;
    *n += 1;
  }
}
bruno
  • 32,421
  • 7
  • 25
  • 37