Undo and redo functionality for a GTK+ program

I'm working on a small GTK+ program and I'm now implementing undo and redo capabilities. This is the code that I'm using to handle the different states.

It's just a linked list storing the allocated char returned by the text buffer. It monitors the size of the structure and the char s so it will always be less than the maximum size set.

What should I change?

Header:

```
#ifndef TEXT_STATE_H
#define TEXT_STATE_H

#include
#include

#define TS_SUCCESS 0
#define TS_ERROR 1
#define TS_GREATER_THAN_BUFFER 2
#define TS_FAILED_TO_ALLOCATE_NODE 4

/* Here's how it works:
1 - a buffer is set
2 - every new state is pushed to the next position, if there are other "next"
states, they are removed, so if the user clicks undo then writes, he can't redo the
previous "next" states
3 - if the new state is bigger than the total allocated space, an error is
returned
4 - if there's not enough space for the new state, the oldest is removed
repeatedly until there's space.
5 - if it fails to allocate the node structure, nothing is done and an error
is returned

Size means total size, structure + text. So there's no chance of using a lot
more memory when the text states are really small.
*/

/ Store the state /
typedef struct Text_Node Text_Node;

typedef struct {
char *text;
size_t size; //including '\0'
} State;

/ The states are stored in a linked-list /
typedef struct {
Text_Node *first;
Text_Node *current;
Text_Node *last;
size_t in_use;
size_t buffer_size;
} Text_State;

//Set up the structure, buffer size
static inline void ts_init(Text_State *ts, size_t buffer_size)
{
ts->first = NULL;
ts->current = NULL;
ts->last = NULL;
ts->in_use = 0;
ts->buffer_size = buffer_size;
}

//Free all
void ts_free(Text_State *ts);

//Free all, keep buffer size, set structure so it can be used again (e.g. when
//a new document is created)
void ts_clear(Text_State *ts);

//Push new state, delete all old "next" stat

Just a few notes:

-
You have some #define statements at the beginning of your header.

#define TS_SUCCESS 0
   #define TS_ERROR 1
   #define TS_GREATER_THAN_BUFFER 2
   #define TS_FAILED_TO_ALLOCATE_NODE 4

You could use an enum instead.

typedef enum 
{
    SUCCESS = 0;
    ERROR = 1;
    GREATER_THAN_BUFFER = 2;
    FAILED_TO_ALLOCATE_NODE = 4;
} TS;

If FAILED_TO_ALLOCATE_NODE could be set to equal 3, this could even be reduced to a one-liner.

typedef enum {SUCCESS, ERROR, GREATER_THAN_BUFFER, FAILED_TO_ALLOCATE_NODE} TS;

-
I'm usually not a fan of including `. The only reason you use it is for return values.

static inline bool buffer_is_too_small(Text_State *ts, Text_Node *node)
   {
       return space_required(node) > ts->buffer_size;
   }

I prefer to just return int values themselves. This way, you can return a value to indicate an error (probably a negative number), a value to indicate success (probably 0), and/or a value to guide the user if there is an error (for example, buffer_is_too_small could return how much larger the buffer needs to be).

-
Your NULL checks can be simplified.

if(node->previous != NULL)
     node->previous->next = NULL;

The != NULL can be excluded while testing node->previous.

if(node) node->previous->next = NULL;

It is also quite simple to check if a value is NULL` in a simpler form.

if(!ts->current)  // Same as: if(ts->current == NULL)
{
    ...
}

-
I'm not the biggest fan of your very large comments.

/////////
//  Public functions
///////

But that is to your discretion.

StackExchange Code Review Q#41350, answer score: 6