HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

The benefits of sentry in a linked list

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thesentrybenefitslistlinked

Problem

I'm trying to put together a little tutorial to demonstrate the benefits of using a sentry node when creating a linked list. Target audience is novices that already tried to make a linked list. Any kind of feedback, regarding best practices, readability etc appreciated.

```
// 2014 donated to Public Domain

#include
#include
#include
#include

// linked list using sentry

// this could be anything
typedef struct
{
int val; // here is where you change what this is a list of
} Data;

// this will be our Node, the items in the linked list
typedef struct NodeTag
{
struct NodeTag* next;
struct NodeTag* prev;
Data data;
} Node;

// this will be out sentry node
// more later on why this is a good idea
typedef struct SentryTag
{
struct NodeTag* next;
struct NodeTag* prev;
} Sentry;

// this is the 'container'
typedef struct
{
Node* sentry; // node that sentry 'poses' as a node, saves most casting
int size;
} List;

// this creates a new empty list
void ListInit( List* lst )
{
lst->sentry = (Node*) malloc( sizeof(Sentry) ); // make a new sentry node
lst->sentry->next = lst->sentry;
lst->sentry->prev = lst->sentry; // that points to itself
lst->size = 0;
}

// pointer to first item of list, or end if empty
Node ListBegin( List lst )
{
return lst->sentry->next; // no special-cases for empty list needed when using sentry
}

// pointer to end (one past last)
Node ListEnd( List lst )
{
return lst->sentry; // no special-cases for empty list needed when using sentry
}

// now we have what we need for iterating through the list
// this is how you do it:
//
// Node* iter = ListBegin(lst);
// while( iter != ListEnd(lst) ) {
// DoSomething( iter->data );
// iter = iter->next;
// }

// this inserts new data before 'here'
Node ListInsert( List lst, Node here, Data data )
{
// create new item
Node item = (Node) malloc( sizeof(Node) );
// copy in data
memcpy( &item->data, data, sizeof(Data) );

Solution

A few aspects regarding coding style and layout:

-
In C you don't have to cast from void to T, so don't do it, it is just redundant code. Don't cast the result value of malloc():

lst->sentry = malloc( sizeof(Sentry) );


I should note however, that if you plan on making C code compilable as C++, then the casts are absolutely necessary. If this is the case, disregard this point.

-
When using the sizeof operator, it is better to apply it to the variable in question, rather than the type. Taking as an example the allocation of Node in ListInsert(), suppose you change the type of node to some other thing (a NodeEx or something), but keep the old Node struct around. If you happen to forget to update the use of Node in malloc( sizeof(Node) ) and type sizes differ, you are in for a bad time. If you use the variable instead, this category of maintenance problems just goes away. So this can facilitate and simplify maintenance:

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


-
You never check the return value of malloc()! Programs can run out of memory. You should always check the return value of malloc, calloc and friends to at least print a message log if allocation fails. As it is now, if the program were to run out of memory, it would crash with very little to no information about the error.

-
When we are talking about sizes and lengths, size_t is the type of choice, rather than int. Consider making List::size a size_t.

-
At the moment, none of the pointers your functions receive are being validated. Since this is a tutorial, it should be a good opportunity to introduce the use of assert for function parameter validation.

-
This lst->size += 1; and lst->size -= 1; are fine, but the ++ and -- operators are the most idiomatic way of incrementing and decrementing by one in C, so the readers of your tutorial should start seeing it from early because that's how 99% of the C code they'll read is going to be written.

-
Tiny inconsistency with spacing here lst->sentry=0; inside ListDelete(). Make sure to put spaces between arithmetical operators and assignments.

Code Snippets

lst->sentry = malloc( sizeof(Sentry) );
Node* item = malloc( sizeof(*item) );

Context

StackExchange Code Review Q#73304, answer score: 7

Revisions (0)

No revisions yet.