segmentation fault in a linked list while loop?

Question

I'm trying to setup a graph in C. I tried the graph with user input and it works perfectly. However, i am trying to implement a read from file. The last else statement is where the error is coming from because when i commented it out it compiles without any problems. I have included a comment over the block i think that has the problem. Please let me know if there is anything else needed for this question.

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

struct node{
    int data;
    struct node* next;
};

//int counter and mainVertex would be used to determine if graph is connected.

// void graphConnection(){
// 
// 
// 
// 
// 
// 
// }


char* deblank(char* input)
{
    int i,j;
    char *output=input;
    for (i = 0, j = 0; i<strlen(input); i++,j++)
    {
        if (input[i]!=' ')
            output[j]=input[i];
        else
            j--;
    }
    output[j]=0;
    return output;
}


struct node *G[1000];
int counter = 0;
char *mainVertex;

void readingEachLine(){

  FILE * fp;
  char * line = NULL;
  size_t len = 0;
  ssize_t read;

  //Read file and exit if fail
  fp = fopen("test.txt", "r");
  if (fp == NULL)
      exit(EXIT_FAILURE);

  while ((read = getline(&line, &len, fp)) != -1) {
    line = deblank(line);
    int i = 0;
    struct node* cursor = malloc(sizeof(struct node));
    struct node* secondcursor = malloc(sizeof(struct node));
    struct node* tempitem;
    while(line[i] != '\n'){

      //If its the first of the line look into the array and set struct cursor to the corresponding
      //array position

      if (i == 0){
        mainVertex[counter] = line[0];
        int convertor = line[i] - '0';
        cursor = G[convertor];
        counter++;
      }
      //if its not the first, then set a struct with that number as data

      else{
        tempitem = malloc(sizeof(struct node));
        int convertor = line[i] - '0';
        tempitem->data = convertor;
        tempitem->next = NULL;
      }
      //if there is no element connected to the struct in array, connect the tempitem

      if (cursor->next == NULL){
        cursor->next = tempitem;
      }
      //If there are already connected elements, loop until the end of the linked list
      //and append the tempitem
      //ERROR: I GET SEGMENTATION FAULT FROM HERE. TRIED AFTER COMMENTING IT OUT

      else{
        secondcursor = cursor;
        while(secondcursor->next != NULL){
          secondcursor = secondcursor->next;
        }
        secondcursor->next = tempitem;
      }
      i++;
    }
    printf("\n");
  }
}

int main(void){
  for (int i = 1; i < 1000; i++)
  {
      G[i]= malloc(sizeof(struct node));
      G[i]->data = i;
      G[i]->next = NULL;
  }
  readingEachLine();
}

EDIT: This is how the text file looks like:

OT: regarding: if (fp == NULL) exit(EXIT_FAILURE); This results in the program exiting without giving the user any indication of what happened. Suggest: if (fp == NULL) { perror( "fopen failed" ); exit(EXIT_FAILURE); } as the call to perror() will output to stderr both your error message and the text reason the system thinks the error occurred., — user3629249
– user3629249, Commented Nov 18, 2018 at 13:27
OT: when calling any of the heap allocation functions: malloc calloc realloc 1) always check (!=NULL) the returned value to assure the operation was success — user3629249
– user3629249, Commented Nov 18, 2018 at 13:29
Is it possible to post these in an answer? Very good advise and would like the explanation — NeverSleepAlwaysCode
– NeverSleepAlwaysCode, Commented Nov 18, 2018 at 13:31
code should be written to be as robust and is reasonable. code should be written to be very friendly to the user. These 'OT' comments are NOT an answer to your question, and are not mandatory to the execution of the code, so they are given the prefix 'OT' (offtopic) — user3629249
– user3629249, Commented Nov 18, 2018 at 13:35
regarding: int convertor = line[i] - '0'; The code should be checking that the first character is a digit (0....9) before trying to convert the first character to a int — user3629249
– user3629249, Commented Nov 18, 2018 at 13:36

M Oehm · Accepted Answer · 2018-11-18 16:29:57Z

Your code has several misconceoptions:

Apparently, you can have a maximum of 1,000 nodes. You have an array G of 1,000 head pointers to linked lists. Don't allocate memory for all 1,000 nodes at the beginning. At the beginning, all lists are empty and an empty linked list is one that has no node and whose head is NULL.
In your example, cursor is used to iterate oer already existing pointers, so don't allocate memory for it. If you have code like this:
```
struct node *p = malloc(...);

// next use of p:
p = other_node;
```
you shouldn't allocate. You would overwrite p and lose the handle to the allocated memory. Not all pointers have to be initialised with malloc; allocate only if you create a node.
Your idea to strip all spaces from a line and then parse single digits will fail if you ever have more then 9 nodes. (But you cater for 1,000 node.) Don't try to parse the numbers yourself. There are library functions for that, for example strtol.
It is not clear what mainVertex is supposed to be. You use it only once, when you assign to it. You treat it like an array, but it is a global pointer, initialised to NULL. When you dereference it, you get undefined behaviour, which is where your segmentation fault probably comes from.

Here's a program that does what you want to do. (It always inserts nodes at the head for simplicity and it should have more allocation checks.)

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

enum {
    maxNodes = 1000
};

struct node{
    int data;
    struct node* next;
};

struct node *G[maxNodes];
size_t nnode = 0;

int read_graph(const char *fn)
{
    FILE * fp;
    char * line = NULL;
    size_t len = 0;

    fp = fopen(fn, "r");
    if (fp == NULL) return -1;

    while (getline(&line, &len, fp) != -1) {
        char *p;
        char *end;
        int id;
        int n;

        id = strtol(line, &end, 10);
        if (end == line) continue;

        if (id < 1 || id > maxNodes) break;

        if (id > nnode) nnode = id;
        id--;

        p = end;
        n = strtol(p, &end, 10);

        while (p != end) {
            struct node *nnew = malloc(sizeof(*nnew));

            nnew->data = n - 1;
            nnew->next = G[id];
            G[id] = nnew;

            p = end;
            n = strtol(p, &end, 10);
        }
    }

    fclose(fp);
    free(line);

    return 0;
}

int main(void)
{
    if (read_graph("test.txt") < 0) {
        fprintf(stderr, "Couldn't gread raph.\n");
        exit(1);
    }

    for (int i = 0; i < nnode; i++) {
        struct node *p = G[i];

        if (p) {
            printf("%d:", i + 1);

            for (; p; p = p->next) {
                printf(" %d", p->data + 1);
            }

            puts("");
        }
    }

    for (int i = 0; i < nnode; i++) {
        struct node *p = G[i];

        while (p) {
            struct node *old = p;

            p = p->next;
            free(old);
        }
    }

    return 0;
}

Collectives™ on Stack Overflow

segmentation fault in a linked list while loop?

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related