Segmentation fault (core dumped) when reading file

Question

I'm doing a simple project to read a input.txt text file and show some basic statistics about it. The file is supposed to be about the entrance on a company building. That's how the file is:

1 ; Visitor ; 10 ; 19 ; 2
2 ; 1 ; Worker ; 8 ; 0
3 ; 2 ; Director ; 12 ; 19
4 ; 5 ; Worker ; 18 ; 22
5 ; Visitor ; 8 ; 0 ; 3

Format is = ID ; Companions(if employee) ; Type ; Entrance Time ; Exit Time ; Services(if Visitor)

I got the program to read the file correctly (i guess), it reads the first worker correctly, but when it gets to the second one it reads the ID and suddenly exits with Segmentation fault (core dumped).

I'd really appreciate if someone with more knowledge could help since i have no idea what's happening, and other questions with the same error didn't help.

Here's the code:

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

typedef enum { false, true } bool;

char * removeSpaces(char *line) {
    int counter = 0,i=0;

    while (line[i]!='\0'){
        if (line[i] == ' '){
            i++;
        } else{
            line[counter] = line[i];
            counter++;
            i++;
        }
    }
    return(line);
    line[counter] = '\0';
}

int main (int argc, char *argv[]){
    FILE * fp;
    char * line = NULL;
    char field[30];
    size_t len = 0;
    ssize_t read;
    bool flag = false;
    int i=0,j=0,k=0,counter=0; //variables to count on loops

    fp = fopen("input.txt", "r");
    if (fp == NULL)
        exit(EXIT_FAILURE);

    while ((read = getline(&line, &len, fp)) != -1) {
        printf("%s", line);
        if(strstr(line, "Worker") != NULL) { //determine the entrance type
            line = removeSpaces(line);

            printf("\nWORKER READ\n");
            i = 0;
            while(line[i] != ';'){ //Read ID
                field[i] = line[i]; //saves the content of the field (all between ';')
                i++;
            }
            field[i] = '\0';
            printf("\nID: ");
            for(i=0;field[i] != '\0';i++){
                printf("%c",field[i]);
            }
            //memset(field,0,strlen(field));
            i = 0; j = 0;
            while(flag != true){ //Read Companions
                if(line[i] == ';'){
                    flag = true; //keeps skipping the string till it finds the right field
                }
                if(flag == true){
                    j = i+1;
                    while(line[j] != ';'){
                        field[k] = line[j]; //saves the content of the field (all between ';')
                        j++; k++;
                    }
                }
                i++;
            }
            field[k] = '\0';
            printf("\nCOMPANIONS: ");
            for(i=0;field[i] != '\0';i++){
                printf("%c",field[i]); //prints what the number of companions read
            }
            //memset(field,0,strlen(field));
            i = 0; j = 0; k = 0; flag = false;
            while(flag != true){ //Read Type
                if(line[i] == ';'){
                    counter++;
                    if(counter == 2){
                        flag = true; //keeps skipping the string till it finds the right field
                    }
                }
                if(flag == true){
                    j = i+1;
                    while(line[j] != ';'){
                        field[k] = line[j]; //saves the content of the field (all between ';')
                        j++; k++;
                    }
                }
                i++;
            }
            field[k] = '\0';
            printf("\nTIPO: ");
            for(i=0;field[i] != '\0';i++){
                printf("%c",field[i]); /prints the type of entrance read
            }
            //memset(field,0,strlen(field));
            i = 0; j = 0; k = 0;  flag = false; counter = 0;
            while(flag != true){ //Read Entrance Time
                if(line[i] == ';'){
                    counter++;
                    if(counter == 3){
                        flag = true; //keeps skipping the string till it finds the right field
                    }
                }
                if(flag == true){
                    j = i+1;
                    while(line[j] != ';'){
                        field[k] = line[j]; //saves it
                        j++; k++;
                    }
                }
                i++;
            }
            field[k] = '\0';
            printf("\nENTRANCE: ");
            for(i=0;field[i] != '\0';i++){
                printf("%c",field[i]);
            }
            i = 0; j = 0; k = 0;  flag = false; counter = 0;
            while(flag != true){ //Read Exit Time
                if(line[i] == ';'){
                    counter++;
                    if(counter == 4){
                        flag = true;
                    }
                }
                if(flag == true){
                    j = i+1;
                    while(line[j] != ';'){
                        field[k] = line[j];
                        j++; k++;
                    }
                }
                i++;
            }
            field[k] = '\0';
            printf("\nSAIDA: ");
            for(i=0;field[i] != '\0';i++){
                printf("%c",field[i]);
            }
            printf("\n\n");

            i = 0; j = 0; k = 0;  flag = false;
            memset(field,0,strlen(field));
        } else if(strstr(line, "Director") != NULL){
            //TODO
        } else if(strstr(line, "Visitor") != NULL){
            //TODO
        }
    }

    return 0;
}

Answer 1

While I recommend sscanf or fscanf over writing complicated parsing code, it's also good for your coding skills to learn how to write parsers. After adjusting your code to use fgets instead of getline so that I could get it to compile, I hit a fault in this loop:

while ( flag != true )
{ //Read Type
    if ( line[i] == ';' ) // <<<< Fault when i = 2488
    {
        counter++;
        if ( counter == 2 )
        {
            flag = true; //keeps skipping the string till it finds the right field
        }
    }
    if ( flag == true )
    {
        j = i + 1;
        while ( line[j] != ';' )
        {
            field[k] = line[j]; //saves the content of the field (all between ';')
            j++; k++;
        }
    }
    i++;
}

You're incrementing i without reference to the length of the actual line, so at some point you are accessing a memory address that you do not own.

You should enable all warnings when you compile your code. Your compiler should be warning you of some of your mistakes. One of the important ones you should be seeing is "Unreachable code..." at line 27 in your removeSpaces function:

return(line);
line[counter] = '\0'; // This never executed.

See http://pubs.opengroup.org/onlinepubs/009696699/functions/fgets.html

Answer 2

sscanf could be used to parse the values from line.

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

int main (int argc, char *argv[]){
    FILE * fp;
    char * line = NULL;
    char id[30];
    char companions[30];
    char type[30];
    char entrance[30];
    char depart[30];
    size_t len = 0;
    ssize_t read;

    fp = fopen("input.txt", "r");
    if (fp == NULL)
        exit(EXIT_FAILURE);

    while ((read = getline(&line, &len, fp)) != -1) {
        printf("%s", line);
        if(strstr(line, "Worker") != NULL) { //determine the entrance type
            printf("\nWORKER READ\n");
            if ( 5 == sscanf ( line, "%29s ; %29s ; %29s ; %29s ; %29s"
            , id, companions, type, entrance, depart)) {
                printf("\nID: %s", id);
                printf("\nCOMPANIONS: %s", companions);
                printf("\nTIPO: %s", type);
                printf("\nENTRANCE: %s", entrance);
                printf("\nSAIDA: %s", depart);
                printf("\n\n");
            }
            else {
                printf ( "problem parsing Worker\n");
            }

        } else if(strstr(line, "Director") != NULL){
            //TODO
        } else if(strstr(line, "Visitor") != NULL){
            //TODO
        }
    }

    return 0;
}