0

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;
        }
    }
}

  • You invoke *Undefined Behavior* in `getNumberOfVerticesAndEdges()` when you use `x` and `y` after failing to ***check the return*** of `fscanf()` and do not check the stream state until the end of the function. You have written [**Why is while ( !feof (file) ) always wrong?**](https://stackoverflow.com/questions/5431941/why-is-while-feoffile-always-wrong) in a slightly different way. You can also remove the `'\n'` from `fscanf(fp, "%d %d^\n", &x, &y);` as it isn't doing what you think it is and the `"%d"` conversion specifier ignores leading whitespace anyway. – David C. Rankin Sep 28 '20 at 03:37
  • Without the input file, nobody can test the code. – Nate Eldredge Sep 28 '20 at 03:42

1 Answers1

1

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)

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85
  • Thanks, that makes a lot of sense and I fixed my function. Although this helped, I believe the seg fault originates from the main(). I commented out getNumberOfVerticesAndEdges() and hard coded the values for testing, and the seg fault persists. Do you see any issues in my main(). Also if it helps, the input file is simply: "%d %d\n%d %d\n..." so basically to ints separated by a space and then a new line. Also, I know the seg fault is occurring in the push calls, because if I comment those out, and I am able to scan all ints and print them out – Adam Menker Sep 28 '20 at 13:16
  • Let me take a look. I'll have time in a couple of hours and will go through the rest of the code. If you have sample data you can post, that would help ensure I can reproduce the exact issue you are seeing. – David C. Rankin Sep 28 '20 at 20:40