Copying elements of a linked list to another linked list in reverse order in C

Question

I'm new to programming in C and taking a course. I'm having trouble with one of the tasks I'm practicing. I'm supposed to Write a program that creates a linked list of 10 characters, then creates a copy of the list in reverse order. I have written (mostly copied) a code, but it only reverses the contents of my linked list, doesn't copy them to a new linked list in reverse order. It's also not working with letters even though I'm using char data type. works fine with numbers.

Here's my code:

#include <stdio.h>
#include <malloc.h>

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

static void reverse(struct Node **head_ref)
{
    struct Node *previous = NULL;
    struct Node *current = *head_ref;
    struct Node *next;
    while (current != NULL)
    {
        next = current->next;
        current->next = previous;
        previous = current;
        current = next;
    }
    *head_ref = previous;
}

void push(struct Node **head_ref, char new_data)
{
    struct Node *new_node =
        (struct Node *)malloc(sizeof(struct Node));
    new_node->data = new_data;
    new_node->next = (*head_ref);
    (*head_ref) = new_node;
}

void printList(struct Node *head)
{
    struct Node *temp = head;
    while (temp != NULL)
    {
        printf("%d ", temp->data);
        temp = temp->next;
    }
}

int main()
{
    struct Node *head = NULL;
    char element = NULL;
    printf("Enter 10 characters:\n");
    for (int i = 0; i <= 9; i++)
    {
        scanf_s("%d", &element);
        push(&head, element);
    }
 
    printf("Given linked list\n");
    printList(head);
    reverse(&head);
    printf("\nReversed Linked list \n");
    printList(head);
    getchar();
}

Answer 1

This for loop

for (int i = 0; i <= 9; i++)
{
    scanf_s("%d", &element);
    push(&head, element);
}

invokes undefined behavior because there is used an incorrect conversion specifier %d with an object of the type char,

You need to write

for (int i = 0; i <= 9; i++)
{
    scanf_s( " %c", &element, 1 );
    push(&head, element);
}

Pay attention to the blank before the conversion specifier %c in the format string. This allows to skip white space characters in the input stream.

As for the function then it can be declared and defined the following simple way using the function push that you already defined

struct Node * reverse_copy( const struct Node *head )
{
    struct Node *new_head = NULL;

    for ( ; head != NULL; head = head->next )
    {
        push( &new_head, head->data );
    }

    return new_head;
}

And in main you can write something like

struct Node *second_head = reverse_copy( head );

Take into account that the function push would be more safer if it would process the situation when memory allocation for a node failed.

Answer 2

To create a copy in reverse order, create a new list with the same values as the original list but prepend the new nodes using the push function.

Here is a modified version:

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

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

void prepend(struct Node **head_ref, char new_data) {
    struct Node *new_node = (struct Node *)malloc(sizeof(struct Node));
    new_node->data = new_data;
    new_node->next = (*head_ref);
    (*head_ref) = new_node;
}

void append(struct Node **head_ref, char new_data) {
    struct Node *new_node = (struct Node *)malloc(sizeof(struct Node));
    struct Node *node = *head_ref;
    new_node->data = new_data;
    new_node->next = NULL;
    if (!node) {
        *head_ref = new_node;
    } else {
        while (node->next)
            node = node->next;
        node->next = new_node;
    }
}

void printList(const struct Node *head) {
    const struct Node *temp = head;
    while (temp != NULL) {
        printf("%c ", temp->data);
        temp = temp->next;
    }
    printf("\n");
}

struct Node *copy_reverse(struct Node *list) {
    struct Node *new_list = NULL;
    while (list) {
        prepend(&new_list, list->data);
        list = list->next;
    }
    return new_list;
}

void freeList(struct Node *list) {
    while (list) {
        struct Node *node = list;
        list = list->next;
        free(node);
    }
}

int main() {
    struct Node *head = NULL;
    char element;
    printf("Enter 10 characters:\n");
    for (int i = 0; i < 10; i++) {
        scanf_s("%c", &element);
        push(&head, element);
    }
 
    printf("Given linked list\n");
    printList(head);

    struct Node *copy = copy_reverse(head);
    printf("\nReversed Linked list \n");
    printList(copy);
    freeList(head);
    freeList(copy);
    getchar();
}

Answer 3

You're almost there. All it needs is one tweak. In reverse, you need to create a new copy of the current node and use that instead. Also, since you'll be ending up with a second list and not altering the original, you should return the new list from reverse.

static struct Node* reverse(const struct Node* head_ref)
{
    struct Node* previous = NULL;
    const struct Node* current = head_ref;
    struct Node* copy;

    while (current != NULL) {
        copy = malloc(sizeof(*copy));
        if (copy == NULL) {
            // handle error
        }
        copy->data = current->data;
        copy->next = previous;
        previous = copy;
        current = current->next;
    }

    return previous;
}

You can also make the loop prettier by converting it to a for loop.

for (current = head_ref; current != NULL; current = current->next) {

Finally, when you print out the list, you're using %d in the printf format string. %d will print the char as an integer. To print out the actual character, use %c instead.