paras
paras

Reputation: 17

Getting Segmentation Fault for struct pointer in C

Here is the code, Getting segmentation fault. I have created two struct data type and assigned them memory via malloc() function but still it shows me segmentation fault.

 #include<stdio.h>
    #include<stdlib.h>
    struct Node {
        int data;
        struct Node * next;
    };

    struct Queue{
        struct Node *front, *rear;
    };

    struct Node* newNode(int n){
        struct Node* node=(struct Node*)malloc(sizeof(struct Node));
        node->next=NULL;
        node->data=n;
        return node;
    }

    void printNode(struct Queue* queue){
        while(queue->front<=queue->rear){
            printf("%d ",queue->front->data);
            queue->front=queue->front->next;
        }
    }
    int main(void) {
        int i;
        struct Queue* queue=(struct Queue*)malloc(sizeof(struct Queue));
        queue->front=NULL;
        queue->rear=NULL;

        for(i=0;i<4;i++){
            if(queue->rear==NULL){
            queue->front->next=queue->rear->next=newNode(i);
            }
            else{
                queue->rear->next=newNode(i);
                queue->rear=queue->rear->next;
            }
        }
        printNode(queue);
        return 0;
    }

Upvotes: 0

Views: 354

Answers (2)

Hariom Singh
Hariom Singh

Reputation: 3632

There are few problems in your queue first is

insertion

Modified Insertion Explanation insertion at the end (There are ways of insertion like front in middle , look at other posts )

If there is no node both front and rear are same else rear end now points to the new node and now the new node become the rear

for(i=0;i<4;i++){
    Node* temp = newNode(i);

    if(queue->front==NULL){
        queue->front = temp;
        queue->rear = temp;

    }
    else{
        queue->rear->next=temp;
        queue->rear=temp;
    }
}

Then the print as suggested by queue->front<=queue->rear is invalid. – BLUEPIXY see comments section

void printNode(struct Queue* queue){

    if(queue->front==NULL)
    {
        printf("\nQueue is empty\n");
        return;
    }
    Node *ptr;
    ptr=queue->front;
    printf("\n\nThe queue :\n\n");
    while(ptr)
    {
        printf("%d->",ptr->data);
        ptr=ptr->next;
    }
    printf("NULL\n");
    return;

}

Execution

#include<stdio.h>
#include<stdlib.h>
struct Node {
    int data;
    struct Node * next;
};

struct Queue{
    struct Node *front, *rear;

};

struct Node* newNode(int n){
    struct Node* node=(struct Node*)malloc(sizeof(struct Node));
    node->next=NULL;
    node->data=n;
    return node;
}

void printNode(struct Queue* queue){

    if(queue->front==NULL)
    {
        printf("\nQueue is empty\n");
        return;
    }
    Node *ptr;
    ptr=queue->front;
    printf("\n\nThe queue :\n\n");
    while(ptr)
    {
        printf("%d->",ptr->data);
        ptr=ptr->next;
    }
    printf("NULL\n");
    return;

}
int main(void) {
    int i;
    struct Queue* queue=(struct Queue*)malloc(sizeof(struct Queue));
    queue->front=NULL;
    queue->rear=NULL;

    for(i=0;i<4;i++){
        Node* temp = newNode(i);

        if(queue->front==NULL){
            queue->front = temp;
            queue->rear = temp;

        }
        else{
            queue->rear->next=temp;
            queue->rear=temp;
        }
    }
    printNode(queue);
    return 0;
}

Output

The queue :

0->1->2->3->NULL
Program ended with exit code: 0

Upvotes: 0

Stephan Lechner
Stephan Lechner

Reputation: 35154

The major issues, which are already mentioned in the comments, are the following ones:

First, you initialize queue->front with NULL, but then access it when writing queue->front->next = .... This is the first point in your code which yields undefined behaviour (likely to cause the segfault). So you should write something like queue->front = queue->rear = newNode(i).

Second, in the loop condition while(queue->front<=queue->rear), you compare memory addresses, which is at least senseless as there is no guarantee that "earlier" pointers have lower memory addresses then the "later" ones. Actually I think that comparing memory addresses, which are not taken out of the same object is undefined behaviour, too. Generally, I would not loop until reaching queue->rear but until reaching the node that hast next == NULL.

Third, your printNode-function alters the queue, produces memory leaks, and two successive calls of printNode would yield different results. Generally, the function should be called printQueue, and it should operate on a local Node-object:

void printQueue(const struct Queue* queue){
    struct Node *node = queue->front;
    while(node){
        printf("%d ",node->data);
        node=node->next;
    }
}

Upvotes: 1

Related Questions