function swap 2 nodes in doubly linked list Helo everyone, I need to review on function swapNode in doubly linked list. It works but I want to make it clearer and better. Are there any errors like memory leak in my code?

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

struct node;
struct list;

typedef struct node node;
typedef struct list list;

struct node
{
    int point;
    char name[30];
    node *next;
    node *prev;
};

struct list
{
    node *head;
    node *tail;
    int count;
};

node *createNewNode(int point, char name[30], node *prev, node *next);
list *createList();
void insertHead(list *listNode, int point, char name[30]);
bool compareName(char a[30], char b[30]);
void swapNode(list *listNode, char nameA[30], char nameB[30]);


int main()
{
    list *listNode = createList();

    insertHead(listNode, 10, "abc def");
    insertHead(listNode, 9, "qwe rty");
    insertHead(listNode, 8, "ui op");
    insertHead(listNode, 30, "fgh jkl");
    insertHead(listNode, 1234, "akaka");

    swapNode(listNode, "fgh jkl", "akaka");

    node *temp = listNode-&gt;head;
    while (temp != NULL)
    {
        printf("%-20s%d\n", temp-&gt;name, temp-&gt;point);
        temp = temp-&gt;next;
    }

    return 0;
}

node *createNewNode(int point, char name[30], node *prev, node *next)
{
    node *newNode = (node *)malloc(sizeof(node));
    newNode-&gt;point = point;
    strcpy(newNode-&gt;name, name);
    newNode-&gt;next = next;
    newNode-&gt;prev = prev;
    return newNode;
}

list *createList()
{
    list *listNode = (list *)malloc(sizeof(list));
    listNode-&gt;count = 0;
    listNode-&gt;head = NULL;
    listNode-&gt;tail = NULL;
    return listNode;
}

void insertHead(list *listNode, int point, char name[30])
{
    node *newNode = allocateNewNode(point, name, NULL, listNode-&gt;head);
    if (listNode-&gt;head)
        listNode-&gt;head-&gt;prev = newNode;
    listNode-&gt;head = newNode;
    if (listNode-&gt;tail == NULL)
        listNode-&gt;tail = newNode;
    ++listNode-&gt;count;
}

bool compareName(char a[30], char b[30])
{
    for (int i = 0; i &lt; 31; i++)
    {
        if (a[i] != b[i])
            return false;
        if (a[i] == '\0')
            break;
    }

    return true;
}

void swapNode(list *listNode, char nameA[30], char nameB[30])
{
    node *A = NULL, *B = NULL;
    node *temp = listNode-&gt;head;

    for (int i = 0; i &lt; listNode-&gt;count; i++)
    {
        if (compareName(temp-&gt;name, nameA))
            A = temp;
        else if (compareName(temp-&gt;name, nameB))
            B = temp;
        temp = temp-&gt;next;
        if (A &amp;&amp; B)
            break;
    }
    if (!A || !B)
        return false;
    else if (A == B)
        return false;

    node p=*A;
    *A=*B;
    *B=p;

    B-&gt;next = A-&gt;next;
    B-&gt;prev = A-&gt;prev;

    A-&gt;next = p.next;
    A-&gt;prev = p.prev;
}
Don't Return Values from void Functions

    if (!A || !B)
        return false;
    else if (A == B)
        return false;


The function void swapNode(List *listNode, char nameA[30], char nameB[30]) is declared void, which means it doesn't return a value, yet it attempts to return false in two places. Some compilers actually report this as an error. Rather than return false; it should just be return;. 

The two if statements above could be rewritten as one if statement

    if ((!A || !B) || (A == B))
    {
        return;
    }


Missing Error Checking

The C programming memory allocation functions malloc(size_t size), calloc(size_t count, size_t size) and realloc( void *ptr, size_t new_size) may fail. If they do fail then they return NULL. Any time one of these functions are called, the result should be tested to see if it is NULL. Referencing fields through a NULL pointer yields unknown behavior and is generally a bug.

Node *safeMalloc(size_t size)
{
    Node* newNode = malloc(size);
    if (newNode == NULL)
    {
        fprintf(stderr, "Memory allocation failed in safeMalloc\n");
        exit(EXIT_FAILURE);
    }

    return  newNode;
}

Node *createNewNode(int point, char name[30], Node *prev, Node *next)
{
    Node *newNode = safeMalloc(sizeof(*newNode));
    newNode-&gt;point = point;
    strcpy(newNode-&gt;name, name);
    newNode-&gt;next = next;
    newNode-&gt;prev = prev;

    return newNode;
}


Declarations of Node Structs

It might have been easier to write the struct declarations as 

typedef struct node
{
    int point;
    char name[30];
    struct node *next;
    struct node *prev;
} Node;

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


Complexity

The function void swapNode(List *listNode, char nameA[30], char nameB[30]) can be simplified by breaking it into 2 functions, one that does the comparisons and then calls a swaping function as necessary:

void doSwap(Node *A, Node*B)
{
    Node p=*A;
    *A=*B;
    *B=p;

    B-&gt;next = A-&gt;next;
    B-&gt;prev = A-&gt;prev;

    A-&gt;next = p.next;
    A-&gt;prev = p.prev;
}

void swapNode(List *listNode, char nameA[30], char nameB[30])
{
    Node *A = NULL, *B = NULL;
    Node *temp = listNode-&gt;head;

    for (int i = 0; i &lt; listNode-&gt;count; i++)
    {
        if (compareName(temp-&gt;name, nameA))
        {
            A = temp;
        }
        else if (compareName(temp-&gt;name, nameB))
        {
            B = temp;
        }
        temp = temp-&gt;next;
        if (A &amp;&amp; B)
        {
            break;
        }
    }

    if ((A &amp;&amp; B) &amp;&amp; (A != B))
    {
        doSwap(A,B);
    }
}


There is also a programming principle called the Single Responsibility Principle that applies here. The Single Responsibility Principle states:


  that every module, class, or function should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by that module, class or function.


A Good Habit for Programming in C and C++

For readability and maintainability a good good habit (best practice) to get into is to always put the actions in if statements and loops into braces ({ and }) as shown in the previous example. One of the major causes of bugs is to add a single line to the contents of an iff statement and to forget to add the necessary the necessary braces. This type of problem is very hard to track down when it doesn't result in a compiler error.

Leaks


  Are there any errors like memory leak in my code?  


If the code was part of a larger project there would be memory leaks, the function free(void *ToBeFreed) is never called. It might be better if some linked list operations such as deleteNode() were added to the code.