I created stack using linked list and in inputting character to make string. But it's not working

Question

In while loop i am unable to enter character in val. Also I am making stack using doubly linked list but my stack is not forming, maybe node is not linked properly. Something is wrong in function create_node(). Then I am displaying stack. Please help me to resolve issuses.

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

typedef struct Node Node;
struct Node{
    Node *prev;
    int ch;
    Node *next;
};

typedef enum SIGNAL SIGNAL;
enum SIGNAL {MEMORY_CREATION_FAILED, SUCCESS, FAILED, UNDERFLOW, OVERFLOW};

// Prototype declaration.
SIGNAL create_Node(Node **top, char c);
void display(Node *q);

// Driver Code
int main(){
    Node *top = NULL;
    char c, val=9;
    while (1){
        printf("\n\nEnter character to insert in string :");
        scanf("%c",&c);
        create_Node(&top, c);
        printf("\nPress ';' to exit or any key to continue : ");
        scanf("%c",&val);
        if(val == ';'){
            printf("\nexiting...");
            display(top);
            return 0;
        }
    }
    return 0;
}

// Function to create Node and enter character.
SIGNAL create_Node(Node **top, char c){
    Node *tem = (Node*)malloc(sizeof(Node));
        return MEMORY_CREATION_FAILED;
    tem->ch = c;
    tem->next = tem->prev = NULL;
    if(*top != NULL){
        (*top)->next = tem;
        tem->prev = *top;
    }
    *top = tem;
    return SUCCESS;
}

// function to display string.
void display(Node *q){
    if(q == NULL){
        printf("\nString is empty nothing to display");
        return;
    }
    printf("String is :\n\n");
    for(; q != NULL ; q = q->prev)
        printf("%d  ",q->ch);
}

Answer 1

At first, please note that in

SIGNAL create_Node(Node **top, char c){
    Node *tem = (Node*)malloc(sizeof(Node));
        return MEMORY_CREATION_FAILED;
    tem->ch = c;
    tem->next = tem->prev = NULL;
    if(*top != NULL){
        (*top)->next = tem;
        tem->prev = *top;
    }
    *top = tem;
    return SUCCESS;
}

you are returning MEMORY_CREATION_FAILED. Just this.

Enable all compiler warnings. There must be some warning about unreachable code...

There is even an indentation left, and I believe and if got deleted by mistake.

about the code

You have a struct for a node, not for a double linked list. Sure, you can write it this way, but is more difficult and harder to reuse and maintain.

A list is a container of nodes. Nodes are containers of some sort of data. Each data record in general has a key, like an id or code. Linked list is an abstraction of a collection of something. You should write the code this way.

Using this abstraction in your code

Your data record is an int but is named ch and used to build a string?

Anyway, about the SIGNAL typedef thing, you should just return the pointer: it is simpler and avoid thing like Node** in the arguments.

Compare with this:

typedef int Data;

typedef struct a_node
{
    Data*   data;

    struct a_node *prev;
    struct a_node *next;

} Node;

typedef struct
{
    Node* head;
    Node* tail;

    int size;

}   List;

List *create_list();
int   insert_list(Data *, List *);
int   show_list(List *);

The list contains Node. Each node contains a pointer to some Data. Generic. You can reuse forever.

And it is easier to read: insert_list() gets a pointer to Data and a pointer to List and does his thing.

create_list() returns a pointer. To a new list. Or NULL.

For it to be really generic there are some other steps, but just as an example see

create a List

List *create_list()
{
    List* list = (List*) malloc(sizeof(List));
    if (list == NULL) return NULL;
    list->head = NULL;
    list->tail = NULL;
    list->size = 0;
    return list;
}

It is simpler to just return a pointer. Another function should follow to delete the list and return NULL to invalidade the pointer. And having a size inside List will prove itself handy in the future.

insert_list()

int insert_list(Data *data, List *L)
{
    // insert at the head, return new size
    Node *node    = (Node *)malloc(sizeof(Node));
    node->next    = L->head;  // new starting node
    node->prev    = NULL;
    node->data    = (Data *)malloc(sizeof(Data));
    *(node->data) = *data;  // plain copy
    // adjusts the pointers
    if (L->size == 0)
    {
        L->head = node;
        L->tail = node;
    }
    else
    {
        L->head->prev = node; // now prev head follows this
        L->head       = node; // and we have new head
    }
    L->size += 1;  // + 1
    return L->size;
}

Note that a new Data is allocated and created inside the function, so List does not depend on external data.

show_list()

int     show_list(List* L)
{
    if (L == NULL)
    {
        printf("There is no list: nothing to display");
        return -1;
    }
    printf("List has %d elements ", L->size);
    if ( L->size > 0 )
        printf("(1st is '%c', last is '%c'): [ \"",
            *(L->head->data), *(L->tail->data)
            );
    Node *p = L->head;
    for (int i = 0; i < L->size; i += 1)
    { 
        printf("%c", *(p->data));
        p = p->next;
    }
    printf(" \"]\n\n");
    return L->size;
}

Note that having size inside the list makes life easier: we can use size to iterate over. And to have head and tail inside the list makes for an easy way to navigate the list.

a simple test

int main(void)
{
    const char value[] = "wolfrevO kcatS";
    List *one = create_list();
    if (one == NULL) return -1;

    char *p = (char*)value;
    while ( *p != 0 )
    { 
        insert_list((Data*)p, one);
        p += 1;
    }
    show_list(one);
    return 0;
}

Here we make use of the 3 functions. And it is arguably easier to read: the list is created. "Stack Overflow" is inserted in reverse and we show the list

the output

List has 14 elements (1st is 'S', last is 'w'): [ "Stack Overflow "]

Answer 2

For starters these declarations

typedef enum SIGNAL SIGNAL;
enum SIGNAL {MEMORY_CREATION_FAILED, SUCCESS, FAILED, UNDERFLOW, OVERFLOW};

are incorrect. You may not use an incomplete enumeration type in typedef declaration. You need to exchange the declarations

enum SIGNAL {MEMORY_CREATION_FAILED, SUCCESS, FAILED, UNDERFLOW, OVERFLOW};
typedef enum SIGNAL SIGNAL;

These calls of scanf

scanf("%c",&c);

and

scanf("%c",&val);

can read white space characters. You should write

scanf( " %c", &c );
       ^^^^

and

scanf( " %c", &val );
       ^^^^

The function create_Node exits at once after the first declaration

SIGNAL create_Node(Node **top, char c){
    Node *tem = (Node*)malloc(sizeof(Node));
        return MEMORY_CREATION_FAILED;
        // ... 

So nothing is added to the list.

Also it is unclear why the data member ch of the structure Node has the type int while you are asking the user to enter characters of a string. Why not to ask the user to enter a string?