segmentation fault when rewinding my file pointer

236 views Asked by At

When my file pointer hits it's second rewind, it causes a seg fault. I have no clue why. I'll include the main that is problematic at the top and all the code below it.

int main(void){
    // creating the file pointer
    FILE *fptr = fopen("input-machine-problem-1.txt", "r");
    if (fptr == NULL) {
        printf("Error! opening file");
        return 0;
    }

    int edges;
    int vertices;
    int* edgesPtr = &edges;
    int* verticesPtr = &vertices;
    getNumberOfVerticesAndEdges(fptr, verticesPtr, edgesPtr);

    LinkedList arrayOfLinkedLists[vertices];

    int x, y;
    for(int i = 0; i < vertices; i++){ 
        if(fptr == NULL){
            return 0;
        }
        rewind(fptr);
        for(int j = 0; j < edges; j++){
            
            printf("%d: ", j);
            fscanf (fptr, "%d %d", &x, &y);    
            printf ("%d %d ", x, y);
            if(i == x){
                push(arrayOfLinkedLists[i], y);
            } else if(i == y){
                push(arrayOfLinkedLists[i], x);
            }
            printf("**\n");
        }
    }

    // printAdjacencyLists(arrayOfLinkedLists, vertices);
    fclose (fptr); 
}

The whole file in case you want to copy and paste:

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

typedef struct node{
    int vertexNumber;
    struct node *next;
} Node;

typedef struct linkedList{
    Node* head;
    int size;
} LinkedList;

void getNumberOfVerticesAndEdges(FILE* fp, int* vertices, int* edges);
void push(LinkedList linkedList, int vertex);
Node* createNode(int vertexNumber);
void printAdjacencyLists(LinkedList* arrayOfLinkedLists, int vertices);

int main(void){
    // creating the file pointer
    FILE *fptr = fopen("input-machine-problem-1.txt", "r");
    if (fptr == NULL) {
        printf("Error! opening file");
        return 0;
    }

    int edges;
    int vertices;
    int* edgesPtr = &edges;
    int* verticesPtr = &vertices;
    getNumberOfVerticesAndEdges(fptr, verticesPtr, edgesPtr);

    LinkedList arrayOfLinkedLists[vertices];

    int x, y;
    for(int i = 0; i < vertices; i++){ 
        if(fptr == NULL){
            return 0;
        }
        rewind(fptr);
        for(int j = 0; j < edges; j++){
            
            printf("%d: ", j);
            fscanf (fptr, "%d %d", &x, &y);    
            printf ("%d %d ", x, y);
            if(i == x){
                push(arrayOfLinkedLists[i], y);
            } else if(i == y){
                push(arrayOfLinkedLists[i], x);
            }
            printf("**\n");
        }
    }

    // printAdjacencyLists(arrayOfLinkedLists, vertices);
    fclose (fptr); 
}

void push(LinkedList linkedList, int vertex){
    Node* newNode = createNode(vertex);

    Node* cur = linkedList.head;
    Node* prev = cur;
    if(cur == NULL){
        linkedList.head = newNode;
        return;
    }
    while(newNode->vertexNumber > cur->vertexNumber){
        prev = cur;
        cur = cur->next;
    }
    newNode->next = cur;
    prev->next = newNode;


}

Node* createNode(int vertexNumber){
    Node* newNode = malloc(sizeof(Node));
    if(!newNode){
        return NULL;
    }
    newNode->vertexNumber = vertexNumber;
    newNode->next = NULL;
    return newNode;
}

void getNumberOfVerticesAndEdges(FILE* fp, int* vertices, int* edges){
    if (fp == NULL) {
        printf("Error! opening file");
        return;
    } 

    *vertices = 0;
    *edges = 0;
    while(1){
        int x, y;

        fscanf(fp, "%d %d^\n", &x, &y);

        if(x > *vertices){
            *vertices = x;
        } if (y > *vertices){
            *vertices = y;
        }

        *edges = (*edges) + 1;

        if(feof(fp)) { 
            return;
        }
    }
}

void printAdjacencyLists(LinkedList* arrayOfLinkedLists, int vertices){
    for(int i = 0; i < vertices; i++){
        printf("\n%d:  ", i);
        if(arrayOfLinkedLists[i].head == NULL){
            return;
        }
        Node* cur = arrayOfLinkedLists[i].head;
        while(cur != NULL){
            printf("%d --> ", cur->vertexNumber);
            cur = cur->next;
        }
    }
}

1

There are 1 answers

2
David C. Rankin On

You need to control your read loop with the return of fscanf(), e.g.

void getNumberOfVerticesAndEdges(FILE* fp, int* vertices, int* edges)
{
    int x, y;
    
    if (fp == NULL) {
        printf("Error! opening file");
        return;
    } 

    *vertices = 0;
    *edges = 0;
    while (fscanf(fp, "%d %d", &x, &y) == 2) {
        if(x > *vertices){
            *vertices = x;
        } if (y > *vertices){
            *vertices = y;
        }

        *edges = (*edges) + 1;
    }
}

(note: there is no need for ^\n in your format string)

This ensures that you only use the values in x and y when they hold a valid value. As you have written it, either x or y or both can fail the conversion (with a matching or input failure) and you still use those values in comparison and depending on the result, assign the values to *verticies without any guarantee that x or y are valid.

Further, as you have it written, *edges = (*edges) + 1; is executed after fscanf() fails and before you check the stream state leading to edges being off-by-one too many.

You can't use any input function correctly unless you check the return. Let me know if you have further questions. Your code may have other issues, but once the read failed -- that is your likely first major problem.

Next SegFault

Your next SegFault is in push() here:

while(newNode->vertexNumber > cur->vertexNumber){
    prev = cur;
    cur = cur->next;
}

You don't check for end-of-list by checking if cur == NULL before dereferencing cur->vertexNumber. That will do it every time. You can fix it with:

while (cur && newNode->vertexNumber > cur->vertexNumber) {

Where Does The '*' Go?

Throughout your code you are attaching the indirection reference to the type instead of the variable. The can be misleading. Why?

int* a, b, c;

Above you are certainly not declaring three pointers to int. Instead, you declare a as a pointer to int and b and c as simple integers.

int *a, b, c;

Makes that clear.

Obviously, the compiler doesn't care, it ignores whitespace between '*' and the variable name, so what you have is not wrong -- this is more a human factors/readability issue.

Other Issues

If you are not using size, remove it from:

typedef struct linkedList{
    Node* head;
    int size;
} LinkedList;

Further, since edges and vertices cannot be negative, nor can size or x or y, make them size_t instead of int. You match the type to the variable use, and on x86_64 you get an additional 4-bytes of range.

Taking Another Approach To Your Code

Ideally, you want to only read your data file once. You can do so if you dynamically allocate pointers instead of using a VLA for LinkedList arrayOfLinkedLists[vertices]; I'll leave the implementation of that to you. Addressing the issues above and cleaning up your push() function a bit, you could do something like the following:

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

typedef struct node{
    size_t vertexNumber;
    struct node *next;
} Node;

typedef struct linkedList{
    Node *head;
    size_t size;
} LinkedList;

Node *createNode (size_t vertexNumber)
{
    Node *newNode = malloc (sizeof *newNode);
    
    if (!newNode) {
        perror ("malloc-newNode");
        return NULL;
    }
    
    newNode->vertexNumber = vertexNumber;
    newNode->next = NULL;
    
    return newNode;
}

int getNumberOfVerticesAndEdges (FILE *fp, size_t *vertices)
{
    size_t edges = 0, x, y;
    
    *vertices = 0;
    
    while (fscanf(fp, "%zu %zu", &x, &y) == 2) {
        if (x > *vertices)
            *vertices = x;
        if (y > *vertices)
            *vertices = y;

        edges += 1;
    }
    
    return edges;
}

/* add nodes in order of vertexNumber to each list */
Node *push (LinkedList *list, size_t vertex)
{
    Node *newNode = createNode(vertex);         /* create new node/initialize */
    
    if (!newNode)                               /* validate node */
        return NULL;
    
    list->size += 1;                            /* node allocated, increment count */
    
    if (!list->head) {                          /* if 1st node, node is head/tail */
        list->head = newNode;
        return list->head;
    }
    
    Node **pp = &list->head,                    /* iterate with address of pointer */
          *p  = list->head;                     /* and pointer to node */
    
    while (p) { /* loop over each node */
        if (vertex < p->vertexNumber) {         /* if new vertext less than current */
            *pp = newNode;                      /* replace current with new */
            newNode->next = p;                  /* set new->next to current */
            return newNode;
        }
        pp = &p->next;                          /* advance to next node */
        p = p->next;
    }
    
    return *pp = newNode;                       /* insert at end */
}

void printAdjacencyLists (LinkedList *arrayOfLinkedLists, size_t vertices)
{
    for (size_t i = 0; i < vertices; i++) {
        printf ("\n%zu:  ", i);
        
        if (arrayOfLinkedLists[i].head == NULL)
            return;
        
        Node *cur = arrayOfLinkedLists[i].head;
        
        while (cur){
            printf("%zu --> ", cur->vertexNumber);
            cur = cur->next;
        }
    }
    putchar ('\n');     /* tidy up with newline */
}

void freelists (LinkedList *a, size_t v)
{
    for (size_t i = 0; i < v; i++) {
        Node *node = a[i].head;
        while (node) {
            Node *victim = node;
            node = node->next;
            free (victim);
        }
    }
}

int main (int argc, char **argv){
    
    size_t edges = 0,
        vertices = 0,
        x, y;
    /* open filename provides as 1st argument, use default if none provided */
    FILE *fptr = fopen (argc > 1 ? argv[1] : "input-machine-problem-1.txt", "r");
    
    if (!fptr) {
        perror ("fopen-fptr");
        return 1;
    }

    if (!(edges = getNumberOfVerticesAndEdges (fptr, &vertices))) {
        fputs ("error: failed to read edges.\n", stderr);
        return 1;
    }
    
    /* initialize array of lists all zero/NULL */
    LinkedList arrayOfLinkedLists[vertices];
    memset (arrayOfLinkedLists, 0, sizeof arrayOfLinkedLists);

    for (size_t i = 0; i < vertices; i++) { 
        if (!fptr) {
            return 1;
        }
        rewind(fptr);
        for (size_t j = 0; j < edges; j++) {
            
            printf("%zu: ", j);
            if (fscanf (fptr, "%zu %zu", &x, &y) != 2) {
                fprintf (stderr, "error reading vertex: %zu, edge: %zu\n", i, j);
                return 1;
            }   
            printf ("%zu %zu ", x, y);
            
            if (i == x) {
                if (!push (&arrayOfLinkedLists[i], y))
                    return 1;
            }
            else if (i == y) {
                if (!push (&arrayOfLinkedLists[i], x))
                    return 1;
            }
            printf("**\n");
        }
    }
    fclose (fptr);

    printAdjacencyLists (arrayOfLinkedLists, vertices);
    
    freelists (arrayOfLinkedLists, vertices);
}

Also included above is a freelists() function to free() all the memory you allocate for your lists. Always ensure you are tracking and freeing the memory you allocate. That way when allocating in other than main() you are not creating memory-leaks.

Example Made Up Input/Output

Exercising the code with example vertex would result in the following output:

$ ./bin/getverticies dat/edgelist.txt
0: 0 1 **
1: 2 3 **
2: 3 2 **
3: 1 0 **
0: 0 1 **
1: 2 3 **
2: 3 2 **
3: 1 0 **
0: 0 1 **
1: 2 3 **
2: 3 2 **
3: 1 0 **

0:  1 --> 1 -->
1:  0 --> 0 -->
2:  3 --> 3 -->

(note: the filename can now be passed as the first argument to the program)