1

I am trying to create a program which creates a data structure of people with their names and ids. When I it compiles no problem but when it runs I get segmentation fault(core dumped). The file is on the same folder as the .c file.Also, inside the file the data will be separated by tab. The list_create() function creates the data structure in the form of a list.

I tried instead of making one single line of code with many functions, multiple lines with few functions,use the tmp as a list instead of a node,free variables in a different order but it changed nothing.

#include <stdio.h>
#include <stdlib.h>
#include <assert.h>
#define MAXSTRING 50
typedef struct{
    char name[MAXSTRING];
    int id;
} student;
typedef struct _node* node;
typedef struct _list* list;

struct _node {
    student data;
    node next;
};

struct _list {
    node head;
    int size;
};


int list_empty(list l){
        assert(l);
        return l->head==NULL;
}
node list_deletefirst(list l){
        assert(l && l->head);
        node ret;
        l->head=l->head->next;
        l->size--;
        ret->next=NULL;
        return ret;
}

void list_freenode(node n){
        assert(n);
        free(n);
}


void load(char*filename,list l){
    FILE *fd;
    node tmp=l->head;
    if((fd=fopen(filename,"r"))==NULL){
        printf("Error trying to open the file");
                abort();
    }
    else{
                while(!feof(fd)){
                fscanf(fd,"%s\t%d\n",tmp->data.name,&tmp->data.id);

                tmp=tmp->next;
                l->size++;
                }

        }

    tmp->next=NULL;
    fclose(fd);
}

void save(char *filename,list l){
    int i;
    node tmp=l->head;
    FILE *fd;   
    rewind(fd);
    if((fd=fopen(filename,"w"))==NULL){
        printf("File could not be opened");
        abort();
    }
    for(i=0;i<l->size;i++){
        fprintf(fd,"%s\t%.4d\n",tmp->data.name,tmp->data.id);
        tmp=tmp->next;
    }
    rewind(fd);
    fclose(fd);
}

int main(int argc,char *argv[]){
    list l=list_create();
    load(argv[1],l);


    save(argv[1],l);

    while (!list_empty(l)){
                list_freenode(list_deletefirst(l));
        }
    free(l);

    return 0;
}

I expect to get a list with names and ids.

Azzarian
  • 153
  • 2
  • 13
  • 4
    `typedef struct _node* node;` is the root of all your bugs. Never hide pointers behind a typedef. – Lundin Jun 05 '19 at 11:22
  • 2
    Core dumbed? What is that? – Sourav Ghosh Jun 05 '19 at 11:24
  • 2
    What does `list_create` do? Also, maybe try your list with one or two items in, that are hard coded rather than from a file to check the list works fine. Then try a file. – doctorlove Jun 05 '19 at 11:25
  • 3
    And read this: [**Why is “while (!feof(file))” always wrong?**](https://stackoverflow.com/questions/5431941/why-is-while-feoffile-always-wrong) – Andrew Henle Jun 05 '19 at 11:31
  • Even if we ignore the `!feof(file)` problem, the loop in `load()` is assuming the `list` passed is allocated with multiple entries (enough to hold the contents of the file). If the function `list_create()` doesn't allocate such entries, the behaviour of the loop is undefined. A linked list doesn't magically grow by having a pointer traverse its nodes. – Peter Jun 05 '19 at 12:13
  • regarding; `list l=list_create();` The posted code is missing this function! How do you expect us to help you debug the problem when you have not posted a [mcve] – user3629249 Jun 06 '19 at 02:50
  • OT: for ease of readability and understanding: 1) please consistently indent the code. Indent after every opening brace '{'. Unindent before every closing brace '}'. Suggest each indent level be 4 spaces 2) variable (and parameter) names should indicate `content` or `usage` (or better, both). names like `l` are meaningless, even in the current context – user3629249 Jun 06 '19 at 02:51
  • OT: regarding: `load(argv[1],l);` Never access beyond `argv[0]` without first checking `argc` to assure the command line parameter was actually entered by the user – user3629249 Jun 06 '19 at 02:54
  • OT: regarding: `printf("Error trying to open the file");` Error messages should be output to `stderr`, not `stdout` Since the error is from a C library function, should also output (to `stderr`) the text reason the system thinks the error occurred. Suggest using: `perror( "fopen failed" );` – user3629249 Jun 06 '19 at 02:58

1 Answers1

1
free(l->head->next);
l->head=l->head->next;

You are freeing l->head->next and assigning the freed value (garbage) to l->head in the next line.

In consequence you read garbage (segfault) in the second iteration when trying to access to l->head->next.

David Ranieri
  • 39,972
  • 7
  • 52
  • 94
  • Unfortunately it did not fix the segmentation fault but it was a mistake that could have caused problem later. – Azzarian Jun 06 '19 at 13:33