Segmantation Fault in C

Question

This program will create link list from text alphabetically.
It is case-sensitive and it will eliminate the marks.

When I run the program, it gives a segmentation fault. I can't find where the problem is. I added the printf() in order to find the mistake but i can't.

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

typedef struct NODE {
    char *word;
    int count;
    struct NODE *next;
}NODE;

char *get_word(FILE *fp){
     printf("getWord");
     char *str = (char*)malloc(sizeof(char)*100);

     char c;
     do {
        c = fgetc(fp);
        if (c == EOF) 
            return 0;
        } while (!isalpha(c));
        do {
             printf("getWord");

        *str++ = tolower(c);
        c = fgetc(fp);
        printf("Word");

        } while (isalpha(c));

        return str;
        }

void insert(NODE* sortedList, char *word) { 
   printf("INSERT ");

   char *str = (char*)malloc(sizeof(char)*100);
   if (sortedList == NULL || word < sortedList->word) {

      NODE *ekle;
      ekle=(NODE*)malloc(sizeof(NODE));
      strcpy(ekle->word,word);
      ekle->count = 1;
      ekle->next = sortedList;
      sortedList = ekle;
   }
   else {
        // 
      NODE *current = sortedList->next;
      NODE *pre = sortedList;
      while (current != NULL && word > current->word) { 
         pre = current;
         current = current->next;
      }
      if (current != NULL && word == current->word) {

         (current->count)++;
      }
      else {

         NODE *ekle;
         ekle=(NODE*)malloc(sizeof(NODE));
         strcpy(ekle->word,word);
         ekle->count = 1;
         ekle->next = current;
         pre->next = ekle;
      }
   }
}

void createList(FILE* fp,NODE *n) {
   printf("CREATELIST ");
   char *word;
   strcpy(word,get_word(fp));
   puts(word);
   while (strcmp(word,"")) {
      printf("Create_LİST2");
      insert(n,word);
      word = get_word(fp);
   }
}

NODE *head;


int main(){
    NODE *list=NULL;;
    FILE *fp;
    fp=fopen( "text.txt", "r" );
    head=list;

    while(!feof(fp)){

                     createList(fp,list);

                     }
    while(list->next != NULL){
                     printf("%s", list->word);
                     }
    return 0;
}

tried to use a debugger? – Peter Miehle Apr 23 '14 at 09:37 — Peter Miehle, Apr 23 '14 at 09:37
always check the result of fopen(). ALWAYS – Peter Miehle Apr 23 '14 at 09:38 — Peter Miehle, Apr 23 '14 at 09:38
you have no boundarycheck in your get_word() while(getc()) – Peter Miehle Apr 23 '14 at 09:39 — Peter Miehle, Apr 23 '14 at 09:39
Where does the segmentation fault happen? – The Archetypal Paul Apr 23 '14 at 09:49 — The Archetypal Paul, Apr 23 '14 at 09:49

score 3 · Accepted Answer · edited May 23 '17 at 12:08

A major problem is this line

*str++ = tolower(c);

This changes the pointer str, so when you return str from the function it actually points beyond the string. A string which you, by the way, do not terminate.

Another major problem are these lines:

NODE *ekle;
ekle=(NODE*)malloc(sizeof(NODE));
strcpy(ekle->word,word);

Here you allocate a NODE structure, but you do not allocate memory for ekle->word, so it points to indeterminate memory. You have the above code in two places.

Equal to the above allocation problem, you have

char *word;
strcpy(word,get_word(fp));

Here too you don't allocate memory for word, so you have a pointer to indeterminate memory.

Also, in C you should not cast the return of malloc. You should also look out for warnings from the compiler, and if you don't get any from the code you have then you need to enable more warnings. Compiler warnings are often a sign of undefined behavior which is what all of the above leads to. And finally, next to the compiler I would argue that a debugger is a developers best tool. Learn to use it, it would have helped you with some of the above problems.

@user3546593 All strings in C must be terminated by the character `'\0'`. For the allocation problems look at the [`strdup`](http://man7.org/linux/man-pages/man3/strdup.3.html) function. — Some programmer dude, Apr 23 '14 at 09:54
@user3546593 Also be careful, what if the string you read from the file is longer than 99 (100 with terminator) characters? — Some programmer dude, Apr 23 '14 at 09:55

unwind · Answer 2 · 2014-04-23T09:58:47.510

2

Here's one problem:

char c;
do {
    c = fgetc(fp);
    if (c == EOF) 
        return 0;

This is wrong; fgetc() returns int, since EOF does not fit in a char. The first line should therefore be:

int c;

edited Apr 23 '14 at 09:58

answered Apr 23 '14 at 09:40

unwind

391,730
64
469
606

will just return fix this problem? – user3546593 Apr 23 '14 at 09:50

score 0 · Answer 3 · edited Apr 23 '14 at 10:32

0

not the segfault, but: you malloc str, without using or freeing it

void insert(NODE* sortedList, char *word) { 
  printf("INSERT ");
  char *str = (char*)malloc(sizeof(char)*100);

edited Apr 23 '14 at 10:32

The Archetypal Paul

41,321
20
104
134

answered Apr 23 '14 at 09:41

Peter Miehle

5,984
2
38
55

i fixed there.it should be *word – user3546593 Apr 23 '14 at 09:49
No, it shouldn't. Hint: "*word" is a _char_ and you're assigning a pointer to it. Reverted your edit – The Archetypal Paul Apr 23 '14 at 10:32

score 0 · Answer 4 · answered Apr 23 '14 at 09:51

0

Fist you have to verify if the file is correctly open. Then AFAIK the strcpy requires that destination has enough space to store the data (line 74), instead of "char *word" use "char word[255]" for instance (if you know the size limit).

answered Apr 23 '14 at 09:51

pmarques

31
1

barak manos · Answer 5 · 2014-04-23T10:05:49.383

Your main problem is here:

*str++ = tolower(c);

First of all, once you increment str, you no longer hold a pointer to the dynamically allocated memory. Therefore, you will not be able to release that memory at a later point in the execution of your program, which will eventually lead to memory leaks. Second, when you return str at the end of the function, you are not returning a pointer to that string as you're probably hoping to.

Additional problems are:

You are not making sure that no more than 99 characters are stored.
You are not terminating the string pointed by str with a null-character.
You are not de-allocating the string pointed by str if an EOF is encountered.
You are not using an int in order to store the return value of function fgetc.

Here is how your function should look like:

#define MAX_WORD_LEN 101

char* get_word(FILE* fp)
{
    char* str = (char*)malloc(sizeof(char)*MAX_WORD_LEN);
    int c,i;
    do
    {
        c = fgetc(fp);
        if (c == EOF)
        {
            free(str);
            return 0;
        }
    }
    while (!isalpha((char)c));
    i = 0;
    do
    {
        str[i++] = tolower((char)c);
        c = fgetc(fp);
    }
    while (isalpha((char)c) && i < MAX_WORD_LEN-1);
    str[i] = 0;
    return str;
}

Please note that if a word in your file is longer than MAX_WORD_LEN-1 characters, then you will essentially "lose" the last character that was read, because it will not be stored anywhere.

score 0 · Answer 6 · answered Apr 23 '14 at 10:37

Here i write code which may helps you to understand problem. It's not exact what as your program does but somewhat similar and easier to understand and also find your all problems solution from below code:

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

typedef struct NODE
{
   char word[100];
   int count;
   struct NODE *next;
}NODE;

NODE *head = NULL;
NODE *list = NULL;

void insert(char *word)
{
   if (list == NULL)
   {
      list = calloc(1, sizeof(NODE));
      if (NULL == list)
      {
         perror("Memory allocation failed");
         return;
      }
      strncpy(list->word, word, 99);
      list->count = 1;
      list->next = NULL;
      head = list;
   }
   else
   {
      list->next = calloc(1, sizeof(NODE));
      if (NULL == list->next)
      {
         perror("Memory allocation failed");
         return;
      }
      strncpy(list->next->word, word, 99);
      list->next->count = 1;
      list->next->next = NULL;
      list = list->next;
   }
}

void createList(FILE* fp)
{
   char word[100] = {0};
   while (EOF != fscanf(fp, "%99s", word))
   {
      if (0 < strlen(word))
      {
         insert(word);
      }
      memset(word, 0x00, 100);
   }
}

int main()
{
   FILE *fp = NULL;
   fp = fopen("text.txt", "r");
   if (NULL == fp)
   {
      //File is not readable
      perror("text.txt file open failed");
      return -1;
   }

   createList(fp);

   list = head;
   while(NULL != list)
   {
      printf("%s\n", list->word);
      list = list->next;
   }

   if (NULL != fp)
   {
      fclose(fp);fp = NULL;
   }

   return 0;
}

And also create function to free all memory which is allocated in insert function.

can you see that problem ? http://stackoverflow.com/questions/23241760/add-node-alphabetical-in-c — user3546593, Apr 23 '14 at 14:13
also i try to insert the nodes alphabetically.here is the main problem — user3546593, Apr 23 '14 at 14:19

Segmantation Fault in C

6 Answers6