Doubly linked list code in C

Let me know if it seems right to you and if anything can be optimized.

```
/*
* Doubly Linked List
*
* Each node of the list contain two references (or links) – one to the previous node and other to the next node.
* The previous link of the first node and the next link of the last node points to NULL.
* In comparison to singly-linked list, doubly-linked list requires handling of more pointers
* but less information is required as one can use the previous links to observe the preceding element.
*/

#include
#include

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

void insert(Node *current, int data);
void delete(Node *current, int data);
void print(Node *current);
int find(Node *current, int data);

void insert(Node *current, int data) {

// current is pointing to first element
// we iterate until we find the end
while(current->next != NULL) {
current = current->next;
}
// create a new Node and insert the item
current->next = (Node *)malloc(sizeof(Node));
(current->next)->prev = current;
current = current->next;
current->data = data;
current->next = NULL;
}

void delete(Node *current, int data){

// Iterate until we find a pointer next to the one we need to delete
while (current->next != NULL && (current->next)->data != data) {
current = current->next;
}

// Item is not found
if(current->next == NULL) {
printf("\nElement %d is not present in the list\n", data);
return;
}

// The element is found in the node next to the one that current points to
// We removed the node which is next to the pointer (which is also temp)
Node *tmp = current->next;
// In special case that we are deleting last node
if(tmp->next == NULL) {
current->next = NULL;
} else {
current->next = tmp->next;
(current->next)->prev = tmp->prev;
}
tmp->prev = current;

// We got rid of the node, n

Some mostly stylistic improvements you could apply:

-
There is no need to cast the return value of malloc(), unless you plan on compiling this code as C++. In C, void* converts implicitly to any other pointer type. So the cast is just boilerplate in this case.

-
Prefer using sizeof(variable) instead of sizeof(Type). Example:

Node *head = malloc(sizeof(*head));

This makes maintenance easier, since now if you introduce a new Node type, you only have to change one place, with no chance of updating one side of the expression and forgetting the other. Note that in this example, the dereference of head is valid because sizeof is a compile-time operation, so there is no actual pointer access taking place here, just type inference.

-
Consider giving more robust names to your functions. find(), insert(), etc, are very common names, which can apply to many things, not just lists. In C, you cannot overload function names, so in any major project, these names would be very likely to cause collisions with other global identifiers. Name prefixes are a common approach in C to solve this problem. Consider prefixing the function names with list_ or any other prefix you might prefer. I.e.:

void list_insert(Node *current, int data);
void list_delete(Node *current, int data);

-
Instead of returning an integer 0|1 to represent a boolean value, you can use the bool type, declared in the ` header, which is more idiomatic.

-
delete() function is a void function, so it doesn't need an explicit return statement at the end.

-
Looks like you are using printf() to log errors:

if(current->next == NULL) {
    printf("\nElement %d is not present in the list\n", data);
    return;
}

For error output, it is more correct to use fprintf and write to the stderr standard stream.

-
The list printing function print() could be made more flexible by receiving the output stream as a parameter. So you don't limit the caller to always printing to stdout, it can also be used to dump the list to a file (notice the use of fprintf()).

void print(Node *current, FILE *stream) {
    while(current != NULL) {
        fprintf(stream, "%d ", current->data);
        current = current->next;
    }   
}

-
When you want to allocate and immediately zero-initialize data, such as in here:

Node *head = (Node *)malloc(sizeof(Node));
head->next = NULL;
head->prev = NULL;

The standard function calloc()` can be used to make code more concise:

Node *head = calloc(1, sizeof(*head));

StackExchange Code Review Q#76931, answer score: 7