Implement function in doubly linked list The following is the entire function for implementing a doubly linked list. Can anyone show me if there is a better way to write these functions, especially the delete_at function please?

#include&lt;stdio.h&gt;
#include&lt;stdlib.h&gt;
typedef struct linked_list
{
    int point;
    struct linked_list *p_node;
    struct linked_list *n_node;
}node;

node *create_head_node(node *, int);
node *insert_head(node *, int);
void insert_tail(node *, int);
node *insert_at(node *, int, int);
int count_node(node *);
void print_list(node *);
node *delete_tail(node *);
node *delete_head(node *);
node *delete_at(node *, int);

node *create_head_node(node *head, int point){
    head-&gt;point=point;
    head-&gt;p_node=NULL;
    head-&gt;n_node=NULL;
    return head;
}
node *insert_head(node *head, int point){
    if (head==NULL){
        printf("no head exist");
        return 0;
    }
    node *temp=(node*)malloc(sizeof(node));
    temp-&gt;point=point;
    temp-&gt;p_node=NULL;
    temp-&gt;n_node=head;
    head-&gt;p_node=temp;
    head=temp;
    return head;
}
void insert_tail(node *head, int point){
    if (head==NULL){
        printf("no head exist");
        return;
    }
    node *p=head;
    node *temp=(node*)malloc(sizeof(node));
    temp-&gt;point=point;
    temp-&gt;n_node=NULL;
    while (p-&gt;n_node!=NULL)
    {
        p=p-&gt;n_node;
    }
    p-&gt;n_node=temp;
    temp-&gt;p_node=p;
}
node *insert_at(node *head, int point, int pos){
    if (head==NULL){
        printf("no head exist");
        return 0;
    }
    int count=count_node(head);
    while (pos&gt;count)
    {
        printf("choose %d positions to add. choose again: ", count); scanf("%d", &amp;pos);
    }
        node *p=head;
        for (int i = 0; i &lt; pos; i++)
        {
            p=p-&gt;n_node;
        }
        node *temp=(node*)malloc(sizeof(node));
    temp-&gt;point=point;
    temp-&gt;n_node=p;
    temp-&gt;p_node=p-&gt;p_node;
    if(p-&gt;p_node!=NULL) p-&gt;p_node-&gt;n_node=temp;
    p-&gt;p_node=temp;
    return head;
}
int count_node(node *head){
    node *p=head;
    int count=0;
    while (p!=NULL)
    {
        count++;
        p=p-&gt;n_node;
    }
    free(p);
    return count;
}
void print_list(node *head){
    if (head==NULL){ printf("nothing to print"); return;}
    node *p=head;
    while (p!=NULL)
    {
        printf("%d ", p-&gt;point);
        p=p-&gt;n_node;
    }
}
node *delete_head(node *head){
    if (head==NULL){
        printf("no head exist\n");
        return 0;
    }
    node *p=head;
    // head-&gt;p_node=NULL;
    if(p-&gt;n_node!=NULL){
        head=p-&gt;n_node;
        free(p);
    }
    else head=NULL;
    return head;
}
node *delete_tail(node *head){
    if (head==NULL){
        printf("no head exist\n");
        return 0;
    }
    node *p=head;
    while (p-&gt;n_node!=NULL)
    {
        p=p-&gt;n_node;
    }
    if (p-&gt;p_node!=NULL){
        p-&gt;p_node-&gt;n_node=NULL;
        free(p);
    }
    else head=NULL;
    return head;
}
node *delete_at(node *head, int pos){
    int count=count_node(head);
    if(head==NULL){
        printf("no head exist");
        return 0;
    }
    while (count&lt;pos)
    {
        printf("choose %d positions to delete. choose again", count); scanf("%d", &amp;pos);
    }

    node *p=head;
    if(pos==0) head=delete_head(head);
    else if(pos==count-1) head=delete_tail(head);
    else{
        for (int i = 0; i &lt; pos; i++)
        {
            p=p-&gt;n_node;
        }
        p-&gt;p_node-&gt;n_node=p-&gt;n_node;
        p-&gt;n_node-&gt;p_node=p-&gt;p_node;
        free(p);
    }
    return head;
}
Design

Why do you need a create_head_node() that takes a node as the first parameter? Seems counter intuitive. Personally I would just remove that function does not seem needed to me.

The other thing is that you don't seem to have a way to initialize an empty list (yes this can simply be the NULL pointer but it does leave it easy to accidentally have an uninitialized list).

You can make the design better by separating Node and the list into different types. That way a lot of the work for tail is removed.

i.e. It is easy to have A List and a Node.

struct Node;
struct List;
typedef struct Node Node;
typedef struct List List;

struct List
{
    Node*   head;
    Node*   tail;
    int     count;
};

struct Node
{
    int     value;
    Node*   next;
    Node*   prev;
};


Code Review

The create_head_node() seem to be more like reset the head node to this value and leak the rest of the list.

node *create_head_node(node *head, int point){
    head-&gt;point=point;
    head-&gt;p_node=NULL;
    head-&gt;n_node=NULL;
    return head;
}
----
node*  list = NULL;
list = insert_head(list, 10);
list = insert_head(list, 20);

/*
 * now lets reset the head to 30 and leak the tail (20)
 */
list = create_head_node(list, 30);




Why does it matter if there is currently no head node?

node *insert_head(node *head, int point){
    if (head==NULL){
        printf("no head exist");
        return 0;
    }


Can't I add to the head of an empty list?
Also why are you returning 0. That is an integer. An empty pointer is represented by NULL.



Again I am not sure we I can add to the tail of an empty list.

void insert_tail(node *head, int point){
    if (head==NULL){
        printf("no head exist");
        return;
    }




node *insert_at(node *head, int point, int pos){


Yes sure it is an error.

    while (pos&gt;count)
    {
        printf("choose %d positions to add. choose again: ", count); scanf("%d", &amp;pos);
    }


But this code can not determine the context that it is being used within. So it should not be generating an error message. You should be returning some form of status for the calling code to check. If the calling code decides an error message is appropriate then it can print the error message.

This is called the separation of concerns. Code either handles resource management (creating an maintaining a linked list) or it handles business logic (deciding if an error is critical or how to inform the user about errors).

Does this section not look exectly like insert_head()?

        node *temp=(node*)malloc(sizeof(node));
    temp-&gt;point=point;
    temp-&gt;n_node=p;
    temp-&gt;p_node=p-&gt;p_node;
    if(p-&gt;p_node!=NULL) p-&gt;p_node-&gt;n_node=temp;
    p-&gt;p_node=temp;
    return head;




int count_node(node *head){
    ....
    free(p);


You are freeing nodes that are still in the list?

A Simpler version;

#include &lt;stdlib.h&gt;
#include &lt;stdbool.h&gt;
#include &lt;stdio.h&gt;

struct Node;
struct List;
typedef struct Node Node;
typedef struct List List;

struct List
{
    Node*   head;
    Node*   tail;
    int     count;
};

struct Node
{
    int     value;
    Node*   next;
    Node*   prev;
};

List* createList();
void  destroyList(List* list);

int   countList(List* list);

bool insertHead(List* list, int value);
bool insertTail(List* list, int value);
bool insertAt(List* list, int value, int pos);

bool deleteHead(List* list);
bool deleteTail(List* list);
bool deleteAt(List* list, int pos);

typedef void (*Action)(int);
void visitNode(List* list, Action action);

/* Don't define a print function.
 * Use the visitor pattern to visit each node.
 * Then printing the list can simply be a function that prints each node as it is visited.
 */


/* Private Function */
Node* allocateNewNode(int value, Node* next, Node* prev)
{
    Node* newNode = (Node*)malloc(sizeof(Node));
    newNode-&gt;value = value;
    newNode-&gt;next  = next;
    newNode-&gt;prev  = NULL;
    return newNode;
}

List* createList()
{
    List* newList = (List*)malloc(sizeof(List));
    newList-&gt;head = NULL;
    newList-&gt;tail = NULL;
    newList-&gt;count = 0;
    return newList;
}
void  destroyList(List* list)
{
    /*
     * remove all elements
     */
    Node* head;
    Node* next;
    for(head = list-&gt;head;head;head = next) {
        next = head-&gt;next;
        free(head);
    }
    /*
     * remove the list
     */
    free(list);
}
int countList(List* list)
{
    return list-&gt;count;
}

bool insertHead(List* list, int value)
{
    Node* newNode = allocateNewNode(value, list-&gt;head, NULL);
    if (list-&gt;head) {
        list-&gt;head-&gt;prev = newNode;
    }
    list-&gt;head = newNode;
    if (list-&gt;tail == NULL) {
        list-&gt;tail = newNode;
    }
    ++newList-&gt;count;
    return true;
}
bool insertTail(List* list, int value)
{
    Node* newNode = allocateNewNode(value, NULL, list-&gt;tail);
    if (list-&gt;tail) {
        list-&gt;tail-&gt;next = newNode;
    }
    list-&gt;tail = newNode;
    if (list-&gt;head == NULL) {
        list-&gt;head = newNode;
    }
    ++newList-&gt;count;
    return true;
}
bool insertAt(List* list, int value, int pos)
{
    if (pos == 0) {
        return insertHead(list, value);
    }
    if (pos &gt;= list-&gt;count) {
        return insertTail(list, value);
    }

    Node* loop = list-&gt;head;
    while(--pos) {
        loop = loop-&gt;next;
    }
    Node* newNode = allocateNewNode(value, loop-&gt;next, loop);
    if (newNode-&gt;next) {
        newNode-&gt;next-&gt;prev = newNode;
    }
    if (newNode-&gt;prev) {
        newNode-&gt;prev-&gt;next = newNode;
    }
    ++newList-&gt;count;
    return true;
}

bool deleteHead(List* list)
{
    Node* old = list-&gt;head;
    if (list-&gt;head) {
        list-&gt;head = list-&gt;head-&gt;next;
        if (list-&gt;head == NULL) {
            list-&gt;tail = NULL;
        }
        --newList-&gt;count;
    }
    free(old);
    return true;
}
bool deleteTail(List* list)
{
    Node* old = list-&gt;tail;
    if (list-&gt;tail) {
        list-&gt;tail = list-&gt;tail-&gt;prev;
        if (list-&gt;tail == NULL) {
            list-&gt;head = NULL;
        }
        --newList-&gt;count;
    }
    free(old);
    return true;
}
void visitNode(List* list, Action action)
{
    Node* loop = list-&gt;head;
    for(;loop;loop = loop-&gt;next) {
        action(loop-&gt;value);
    }
}

void printInt(int value)    {fprintf(stdout, "%d ", value);}
void printList(List* list)  {visitNode(list, printInt);}