user3155354
user3155354

Reputation: 61

Pass reference causing Segmentation fault

I am trying creating a circular link list. When I try to add an element by passing the head pointer as a reference, it throws a segmentation fault. I noticed that the value of the head pointer is changing as soon as I call the Add element function. Code snippet:

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

int c = 1;
typedef struct Node node;
//node *head = NULL, *temp, *temp2, *z;
node *InitQueue(node *);
node *AddQueue(node *, int);
void DelQueue();
void display(node *);

void main()
{
    int ch;
    node *head = NULL;

    do {
        printf("1.Creation Q\n");
        printf("2.Insert element to Q\n");
        printf("3.Delete element\n");
        printf("4.Display Q\n");
        printf("5.Exit\n");
        printf("Enter your choice:\n");

        scanf("%d", &ch);
        switch (ch) {
        case 1:
            head = InitQueue(&head);
            printf("%d %p\n", head->data, head->next);
            break;

        case 2:
            printf("%d %p\n", head->data, head);
            int item;
            printf("Enter item\n");
            scanf("%d", &item);
            head = AddQueue(&head, item);
            break;

        case 3:
            DelQueue();
            break;

        case 4:
            display(&head);
            break;

        case 5:
            exit(0);
        }

    } while (ch != 5);

}

node *InitQueue(node * head)
{
    node *temp;
    temp = (node *) malloc(sizeof(node));
    printf("%p \n", temp);
    temp->next = temp;
    head = temp;
    printf("%p \n", head->next);
    return head;
}

node *AddQueue(node * head, int item)
{
    //InitQueue(&head);
    printf("%d %p\n", head->data, head);
    node *temp, *temp2;
    temp = head;
    temp2 = (node *) malloc(sizeof(node));
    //printf("Enter the data: \n");
    //scanf("%d", &temp2->data);
    temp2->data = item;

    while (temp->next != head) {
        temp = temp->next;
    }
    temp->next = temp2;
    temp->next = head;
    head = temp2;
    return head;
}

Upvotes: 0

Views: 2023

Answers (2)

user3629249
user3629249

Reputation: 16540

regarding this function, as an example of how to do the operation:

node *InitQueue(node * head)
{
    node *temp;
    temp = (node *) malloc(sizeof(node));
    printf("%p \n", temp);
    temp->next = temp;
    head = temp;
    printf("%p \n", head->next);
    return head;
}

There are a few problems with using that to initialize a linked list with the first entry.

Suggest:

//node *InitQueue(node * head) // passing 'head' here is useless
node *InitQueue( int data ) // use 'data' to fill in field on first node
{
    node *temp = NULL;

    // always check the returned value from malloc 
    // to assure the operation was successful
    // if not successful, handle the error
    if( NULL == (temp = malloc(sizeof(node)) ) )
    { // then malloc failed
        perror( "malloc for initial node failed" );
        exit( EXIT_FAILURE );
    }

    // implied else, malloc successful

    //printf("%p \n", temp);  // debug statement

    temp->data = data;
    temp->next = NULL;    // always set the 'link' to NULL so end of list can be found

    //head = temp;    // head is a copy on the stack, so has no effect on callers 'head'

    //printf("%p \n", head->next); // debug statement

    //return head;  // this is not set to point to the malloc'd node
    return temp;
}

Upvotes: 0

Enzo Ferber
Enzo Ferber

Reputation: 3104

Your problem is that you're passing the actual memory address of the variable head to your InitQueue and AddQueue functions (so that you can modify it inside the functions), but they're declared as:

node *InitQueue(node *);
node *AddQueue(node *, int);

Your functions are expecting node * and you passed node **.

When you do this:

head = InitQueue(&head);
...
AddQueue(&head, item);
    1. Types are wrong. Your compiler complained about it and you ignored.
    1. InitQueue only worked because you're returning something.
    1. AddQueue doesn't work because it's expecting a pointer to node, not a pointer to pointer to node.

You should do:

void InitQueue(node **head)
{
    (*head) = malloc(sizeof(node));
    if (!(*head)) { /* error check */ }
    (*head)->next = (*head);
}

Now you call it the same way you were calling, but no need to return anything.

InitQueue(&head);

You can fix AddQueue by the same means.


Might Interest

Upvotes: 4

Related Questions