Re-implement fifo code

Following informal option testing, a more performant fifo
implementation has been chosen which makes fewer, larger
allocations, but which does not have bad edge-case performance

Clearing the contents of a fifo is a common operation which generally
involves freeing memory. Support has been added to the fifo interface
for doing this.

common/fifo.c

@@ -1,7 +1,7 @@
 /**
  * xrdp: A Remote Desktop Protocol server.
  *
- * Copyright (C) Laxmikant Rashinkar 2004-2014
+ * Copyright (C) Matt Burt 2023
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -14,174 +14,250 @@
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  * See the License for the specific language governing permissions and
  * limitations under the License.
- *
- * FIFO implementation to store pointer to data struct
  */
 
+/**
+ * @file    common/fifo.c
+ * @brief   Fifo for storing generic pointers
+ *
+ * Defines an unbounded FIFO-queue for void * pointers
+ *
+ * The stored pointers are called 'items' below.
+ *
+ * Items are stored in groups called 'chunks'. Chunks are linked together
+ * in a chain:-
+ *
+ * +-------------+    +--------+    +--------+    +--------+
+ * | first_chunk |--->|  next  |--->|  next  |--->|  NULL  |<-+
+ * | last_chunk  |-+  +--------+    +--------+    +--------+  |
+ * | . . .       | |  | item.0 |    | item.0 |    | item.0 |  |
+ * +-------------+ |  |   ...  |    |   ...  |    |   ...  |  |
+ *                 |  | item.n |    | item.n |    | item.n |  |
+ *                 |  +--------+    +--------+    +--------+  |
+ *                 |                                          |
+ *                 +------------------------------------------+
+ *
+ * This allows items to be added to the FIFO by allocating blocks
+ * as each one fills up.
+ *
+ * The code to read from the FIFO de-allocates blocks as each one is
+ * consumed.
+ *
+ * There is always at least one chunk in the FIFO.
+ */
+
+
 #if defined(HAVE_CONFIG_H)
 #include <config_ac.h>
 #endif
 
+#include <stdlib.h>
+
 #include "fifo.h"
-#include "os_calls.h"
 
-/**
+#define ITEMS_PER_CHUNK 31
- * Create new fifo data struct
- *
- * @return pointer to new FIFO or NULL if system out of memory
- *****************************************************************************/
 
-FIFO *
+struct chunk
-fifo_create(void)
 {
-    return (FIFO *) g_malloc(sizeof(FIFO), 1);
+    struct chunk *next;
+    void *items[ITEMS_PER_CHUNK];
+};
+
+struct fifo
+{
+    struct chunk *first_chunk;
+    struct chunk *last_chunk;
+    /** Next address to write in 'last_chunk' */
+    unsigned short writer;
+    /** Next address to read in 'first_chunk' */
+    unsigned short reader;
+    /** Item destructor function, or NULL */
+    fifo_item_destructor item_destructor;
+};
+
+/*****************************************************************************/
+
+struct fifo *
+fifo_create(fifo_item_destructor item_destructor)
+{
+    struct fifo *result = NULL;
+    struct chunk *cptr = (struct chunk *)malloc(sizeof(struct chunk));
+    if (cptr != NULL)
+    {
+        /* 'next' pointer in last block is always NULL */
+        cptr->next = NULL;
+        result = (struct fifo *)malloc(sizeof(struct fifo));
+        if (result == NULL)
+        {
+            free(cptr);
+        }
+        else
+        {
+            result->first_chunk = cptr;
+            result->last_chunk = cptr;
+            result->writer = 0;
+            result->reader = 0;
+            result->item_destructor = item_destructor;
+        }
+    }
+    return result;
 }
 
+/*****************************************************************************/
 /**
- * Delete specified FIFO
+ * Internal function to call the destructor function on all items in the fifo
- *****************************************************************************/
+ *
+ * @param self fifo. Can't be NULL
+ * @param closure Additional argument to destructor function
+ */
+static void
+call_item_destructor(struct fifo *self, void *closure)
+{
+    if (self->item_destructor != NULL)
+    {
+        struct chunk *cptr = self->first_chunk;
+        unsigned int i = self->reader;
 
+        // Process all the chunks up to the last one
+        while (cptr != self->last_chunk)
+        {
+            (*self->item_destructor)(cptr->items[i++], closure);
+            if (i == ITEMS_PER_CHUNK)
+            {
+                cptr = cptr->next;
+                i = 0;
+            }
+        }
+
+        // Process all the items in the last chunk
+        while (i < self->writer)
+        {
+            (*self->item_destructor)(cptr->items[i++], closure);
+        }
+    }
+}
+
+
+/*****************************************************************************/
 void
-fifo_delete(FIFO *self)
+fifo_delete(struct fifo *self, void *closure)
 {
-    USER_DATA *udp;
+    if (self != NULL)
-
-    if (!self)
     {
-        return;
+        call_item_destructor(self, closure);
-    }
 
-    if (!self->head)
+        // Now free all the chunks
-    {
+        struct chunk *cptr = self->first_chunk;
-        /* FIFO is empty */
+        while (cptr != NULL)
-        g_free(self);
-        return;
-    }
-
-    if (self->head == self->tail)
-    {
-        /* only one item in FIFO */
-        if (self->auto_free)
         {
-            g_free(self->head->item);
+            struct chunk *next = cptr->next;
+            free(cptr);
+            cptr = next;
         }
 
-        g_free(self->head);
+        free(self);
-        g_free(self);
-        return;
     }
-
-    /* more then one item in FIFO */
-    while (self->head)
-    {
-        udp = self->head;
-
-        if (self->auto_free)
-        {
-            g_free(udp->item);
-        }
-
-        self->head = udp->next;
-        g_free(udp);
-    }
-
-    g_free(self);
 }
 
-/**
- * Add an item to the specified FIFO
- *
- * @param self FIFO to operate on
- * @param item item to add to specified FIFO
- *
- * @return 0 on success, -1 on error
- *****************************************************************************/
 
+/*****************************************************************************/
+void
+fifo_clear(struct fifo *self, void *closure)
+{
+    if (self != NULL)
+    {
+        call_item_destructor(self, closure);
+
+        // Now free all the chunks except the last one
+        struct chunk *cptr = self->first_chunk;
+        while (cptr->next != NULL)
+        {
+            struct chunk *next = cptr->next;
+            free(cptr);
+            cptr = next;
+        }
+
+        // Re-initialise fifo fields
+        self->first_chunk = cptr;
+        self->last_chunk = cptr;
+        self->reader = 0;
+        self->writer = 0;
+    }
+}
+
+/*****************************************************************************/
 int
-fifo_add_item(FIFO *self, void *item)
+fifo_add_item(struct fifo *self, void *item)
 {
-    USER_DATA *udp;
+    int rv = 0;
-
+    if (self != NULL && item != NULL)
-    if (!self || !item)
     {
-        return -1;
+        if (self->writer == ITEMS_PER_CHUNK)
+        {
+            // Add another chunk to the chain
+            struct chunk *cptr;
+            cptr = (struct chunk *)malloc(sizeof(struct chunk));
+            if (cptr == NULL)
+            {
+                return 0;
+            }
+            cptr->next = NULL;
+            self->last_chunk->next = cptr;
+            self->last_chunk = cptr;
+            self->writer = 0;
+        }
+
+        self->last_chunk->items[self->writer++] = item;
+        rv = 1;
     }
-
+    return rv;
-    if ((udp = (USER_DATA *) g_malloc(sizeof(USER_DATA), 0)) == 0)
-    {
-        return -1;
-    }
-
-    udp->item = item;
-    udp->next = 0;
-
-    /* if fifo is empty, add to head */
-    if (!self->head)
-    {
-        self->head = udp;
-        self->tail = udp;
-        return 0;
-    }
-
-    /* add to tail */
-    self->tail->next = udp;
-    self->tail = udp;
-
-    return 0;
 }
 
-/**
+/*****************************************************************************/
- * Return an item from top of FIFO
- *
- * @param self FIFO to operate on
- *
- * @return top item from FIFO or NULL if FIFO is empty
- *****************************************************************************/
-
 void *
-fifo_remove_item(FIFO *self)
+fifo_remove_item(struct fifo *self)
 {
-    void      *item;
+    void *item = NULL;
-    USER_DATA *udp;
+    if (self != NULL)
-
-    if (!self || !self->head)
     {
-        return 0;
+        // More than one chunk in the fifo?
-    }
+        if (self->first_chunk != self->last_chunk)
+        {
+            /* We're not reading the last chunk. There
+             * must be something in the fifo */
+            item = self->first_chunk->items[self->reader++];
 
-    if (self->head == self->tail)
+            /* At the end of this chunk? */
-    {
+            if (self->reader == ITEMS_PER_CHUNK)
-        /* only one item in FIFO */
+            {
-        item = self->head->item;
+                struct chunk *old_chunk = self->first_chunk;
-        g_free(self->head);
+                self->first_chunk = old_chunk->next;
-        self->head = 0;
+                free(old_chunk);
-        self->tail = 0;
+                self->reader = 0;
-        return item;
+            }
+        }
+        else if (self->reader < self->writer)
+        {
+            /* We're reading the last chunk */
+            item = self->first_chunk->items[self->reader++];
+            if (self->reader == self->writer)
+            {
+                // fifo is now empty. We can reset the pointers
+                // to prevent unnecessary allocations in the future.
+                self->reader = 0;
+                self->writer = 0;
+            }
+        }
     }
-
-    /* more then one item in FIFO */
-    udp = self->head;
-    item = self->head->item;
-    self->head = self->head->next;
-    g_free(udp);
     return item;
 }
 
-/**
+/*****************************************************************************/
- * Return FIFO status
- *
- * @param self FIFO to operate on
- *
- * @return true if FIFO is empty, false otherwise
- *****************************************************************************/
 
 int
-fifo_is_empty(FIFO *self)
+fifo_is_empty(struct fifo *self)
 {
-    if (!self)
+    return (self == NULL ||
-    {
+            (self->first_chunk == self->last_chunk &&
-        return 1;
+             self->reader == self->writer));
-    }
-
-    return (self->head == 0);
 }

common/fifo.h

@@ -14,34 +14,85 @@
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  * See the License for the specific language governing permissions and
  * limitations under the License.
+ */
+
+/**
+ * @file    common/fifo.h
+ * @brief   Fifo for storing generic pointers
  *
- * FIFO implementation to store pointer to data struct
+ * Declares an unbounded FIFO-queue for void * pointers
  */
 
 #ifndef _FIFO_H
 #define _FIFO_H
 
-#include "arch.h"
+struct fifo;
 
-typedef struct user_data USER_DATA;
+/**
+ * Function used by fifo_clear()/fifo_delete() to destroy items
+ *
+ * @param item Item being deleted
+ * @param closure Additional argument to function
+ *
+ * Use this function to free any allocated storage (e.g. if the items
+ * are dynamically allocated)
+ */
+typedef void (*fifo_item_destructor)(void *item, void *closure);
 
-struct user_data
+/**
-{
+ * Create new fifo
-    USER_DATA *next;
+ *
-    void      *item;
+ * @param item_destructor Destructor for fifo items, or NULL for none
-};
+ * @return fifo, or NULL if no memory
+ */
+struct fifo *
+fifo_create(fifo_item_destructor item_destructor);
 
-typedef struct fifo
+/**
-{
+ * Delete an existing fifo
-    USER_DATA *head;
+ *
-    USER_DATA *tail;
+ * Any existing entries on the fifo are passed in order to the
-    int        auto_free;
+ * item destructor specified when the fifo was created.
-} FIFO;
+ *
+ * @param self fifo to delete (may be NULL)
+ * @param closure Additional parameter for fifo item destructor
+ */
+void
+fifo_delete(struct fifo *self, void *closure);
 
-FIFO *fifo_create(void);
+/**
-void   fifo_delete(FIFO *self);
+ * Clear(empty) an existing fifo
-int    fifo_add_item(FIFO *self, void *item);
+ *
-void *fifo_remove_item(FIFO *self);
+ * Any existing entries on the fifo are passed in order to the
-int    fifo_is_empty(FIFO *self);
+ * item destructor specified when the fifo was created.
+ *
+ * @param self fifo to clear (may be NULL)
+ * @param closure Additional parameter for fifo item destructor
+ */
+void
+fifo_clear(struct fifo *self, void *closure);
+
+/** Add an item to a fifo
+ * @param self fifo
+ * @param item Item to add
+ * @return 1 if successful, 0 for no memory, or tried to add NULL
+ */
+int
+fifo_add_item(struct fifo *self, void *item);
+
+/** Remove an item from a fifo
+ * @param self fifo
+ * @return item if successful, NULL for no items in FIFO
+ */
+void *
+fifo_remove_item(struct fifo *self);
+
+/** Is fifo empty?
+ *
+ * @param self fifo
+ * @return 1 if fifo is empty, 0 if not
+ */
+int
+fifo_is_empty(struct fifo *self);
 
 #endif