0

I wrote a recursive function to reverse a linked list as follows:

struct node{
int val;
struct node *next;
};
//Global pointer to structure
struct node *start=NULL,*head=NULL;


//*Function to input node*

void create(int data){

struct node *temp;
temp=(struct node *)malloc(sizeof(struct node));
if(start == NULL){
    temp->val=data;
    temp->next=NULL;
    start=temp;
    head=temp;
}
else{
    temp->val=data;
    temp->next=NULL;
    head->next=temp;
    head=temp;
    }
   }

  *Function to reverse the linked list*
  void* rev(struct node *prev,struct node *cur){
        if(cur!=NULL){
        printf("Works");
        rev(cur,cur->next);
        cur->next=prev;
    }
    else{
        start=prev;
    }

 }

And the related code in main is:

  main(){
  struct node *temp;
  temp=start;
  /*Code to insert values*/
   rev(NULL,temp);
  }

Now the code takes input and prints it perfectly, but after I call rev() function the same traversal function prints nothing. I did run the code on debugger line by line n it gave me the following output:

rev (prev=0x0, cur=0x0)

Also since cur is somehow NULL, the if part of rev() never gets executed and only the else executes once. When I take input in my create() function I do update start to the first element of the linked list and even in main a print statement proves it is so. But then why the function rev() always receives input parameters as NULL?

Please comment if any extra information is required.

cdlane
  • 40,441
  • 5
  • 32
  • 81
Vivek Shankar
  • 770
  • 1
  • 15
  • 37

1 Answers1

0

Specific problems with your code: your main() function lacks sufficient code to test the reversal functionality (e.g. it doesn't create any nodes!); your create() routine really needs a head and tail pointer to work correctly, not the current head and start; your reversal function maintaines the head/start pointer but doesn't handle a tail pointer; you've redundant code in your if and else clauses that can be pulled out of the conditional; you declared rev() a void * instead of simply a void.

I've reworked your code below with addressing the above changes along with some style issues:

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

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

// Global pointers to structure
struct node *head = NULL, *tail = NULL;

// Function to add node

void create(int data) {

    struct node *temporary = malloc(sizeof(struct node));

    temporary->value = data;
    temporary->next = NULL;

    if (head == NULL) {
        head = temporary;
    } else {
        tail->next = temporary;
    }

    tail = temporary;
}

// Function to reverse the linked list

void reverse(struct node *previous, struct node *current) {
    if (current != NULL) {
        reverse(current, current->next);
        current->next = previous;
    } else {
        head = previous;
    }

    if (previous != NULL) {
        tail = previous;
    }
 }

void display(struct node *temporary) {
    while (temporary != NULL) {
        printf("%d ", temporary->value);
        temporary = temporary->next;
    }
    printf("\n");
}

// And the related code in main is:

int main() {

    /* Code to insert values */
    for (int i = 1; i <= 10; i++) {
        create(i);
    }

    display(head);

    reverse(NULL, head);

    display(head);

    create(0);

    display(head);

    return 0;
}

OUTPUT

> ./a.out
1 2 3 4 5 6 7 8 9 10 
10 9 8 7 6 5 4 3 2 1 
10 9 8 7 6 5 4 3 2 1 0 
> 

You should add a routine to free the nodes in the linked list.

cdlane
  • 40,441
  • 5
  • 32
  • 81
  • Oh wow! thanks for your effort. I'm not sure yet what, if something, was wrong with the `rev()` function. I'm sorry my `create()` is indeed messy. Thanks again :) – Vivek Shankar Aug 17 '16 at 08:10