4
\$\begingroup\$

I've improved, with help of the answers and comments, the linked list that I posted yesterday. It's been through a lot of changes, and is much more useful and safe now.

I documented the linked list in the header file, so I won't describe it here (if the doc isn't enough, please feel free to complain).


Disclaimer:

The library requires POSIX extensions, GCC extensions, and LIBBSD extensions, but if someone wants to make it portable, it shouldn't be difficult to get rid of the extensions I used. But to me, they are of great help and make my life much easier.

I use goto because it is cleaner to me (the way I use it), but if someone wants to get rid of it, it should be easy; I don't think in this case I made a spaghetti of the code.

The names of the funcions don't make it obvious that the list is circular because that would make the names longer, and this list can also easily simulate a non-circular linked list (it has a head and a tail, and instead of checking against NULL one can check against head or tail). Nevertheless, the documentation makes it clear that it is circular.


Now, the code:

linked-list.h:

/******************************************************************************
 ******* include guard ********************************************************
 ******************************************************************************/
#pragma once    /* libalx/extra/alx/linked-list.h */


/******************************************************************************
 ******* about ****************************************************************
 ******************************************************************************/
/*
 * Circular doubly-linked list
 *
 * A list pointer can be created with `struct Alx_LinkedList *list;`
 * To use the list, it has to be initialized with `alx_llist_init(&list)`.
 * It can be deinitialized with `alx_llist_deinit(list)`.
 *
 * Data is copied into `malloc`ed memory, and `free`d or `realloc`ed
 * automatically by the functions.
 *
 * Each node stores a pointer to its allocated data, the size of the data,
 * and pointers to the two connecting nodes.
 *
 * If any of the list metadata is manually modified by the user, the list may
 * be corrupted, and the behavior is undefined.  The only thing that the user
 * can safely manually modify are the contents of data, being careful of not
 * overrunning the buffer.
 */


/******************************************************************************
 ******* headers **************************************************************
 ******************************************************************************/
#include <stddef.h>


/******************************************************************************
 ******* macros ***************************************************************
 ******************************************************************************/


/******************************************************************************
 ******* enum *****************************************************************
 ******************************************************************************/


/******************************************************************************
 ******* struct / union *******************************************************
 ******************************************************************************/
/*
 * Doubly-linked list node
 *
 * data:    Pointer to allocated memory containing useful data.
 * size:    Size of the allocated buffer (in bytes).
 * prev:    Pointer to the previous node.
 * next:    Pointer to the next node.
 */
struct  Alx_LLNode {
    void                *data;
    size_t              size;
    struct Alx_LLNode   *prev;
    struct Alx_LLNode   *next;
};

/*
 * Doubly-linked list
 *
 * head:    Pointer to the first node.
 * tail:    Pointer to the last node.
 * nmemb:   Number of nodes in the list.
 */
struct  Alx_LinkedList {
    struct Alx_LLNode   *head;
    struct Alx_LLNode   *tail;
    ptrdiff_t           nmemb;
};


/******************************************************************************
 ******* prototypes ***********************************************************
 ******************************************************************************/
/*
 * Initialize list.
 *
 * return:
 *  0:          OK.
 *  -ENOMEM:    Aborted; failure to allocate the list.
 */
__attribute__((nonnull, warn_unused_result))
int     alx_llist_init      (struct Alx_LinkedList **list);

/*
 * Deinitialize list.
 * Any remaining nodes will automatically be `free`d.
 */
__attribute__((nonnull))
void    alx_llist_deinit    (struct Alx_LinkedList *list);

/*
 * Inserts a node at the begining of the list.
 * Allocates memory for the node and for the data, copies the data passed
 * by the user to the newly allocated space, and updates any necessary metadata.
 *
 * return:
 *  0:          OK.
 *  -ENOMEM:    Aborted; failure to allocate the node.
 */
__attribute__((nonnull, warn_unused_result))
int     alx_llist_prepend   (struct Alx_LinkedList *list,
                             const void *data, size_t size);

/*
 * Inserts a node at the end of the list.
 * Allocates memory for the node and for the data, copies the data passed
 * by the user to the newly allocated space, and updates any necessary metadata.
 *
 * return:
 *  0:          OK.
 *  -ENOMEM:    Aborted; failure to allocate the node.
 */
__attribute__((nonnull, warn_unused_result))
int     alx_llist_append    (struct Alx_LinkedList *list,
                             const void *data, size_t size);

/*
 * Inserts a node just before the `ref` node.
 * Allocates memory for the node and for the data, copies the data passed
 * by the user to the newly allocated space, and updates any necessary metadata.
 *
 * return:
 *  0:          OK.
 *  -ENOMEM:    Aborted; failure to allocate the node.
 */
__attribute__((nonnull, warn_unused_result))
int     alx_llist_insert_before (struct Alx_LinkedList *list,
                                 const void *data, size_t size,
                                 struct Alx_LLNode *ref);

/*
 * Inserts a node just after the `ref` node.
 * Allocates memory for the node and for the data, copies the data passed
 * by the user to the newly allocated space, and updates any necessary metadata.
 *
 * return:
 *  0:          OK.
 *  -ENOMEM:    Aborted; failure to allocate the node.
 */
__attribute__((nonnull, warn_unused_result))
int     alx_llist_insert_after  (struct Alx_LinkedList *list,
                                 const void *data, size_t size,
                                 struct Alx_LLNode *ref);

/*
 * Inserts a node at the desired position in the list.
 * Allocates memory for the node and for the data, copies the data passed
 * by the user to the newly allocated space, and updates any necessary metadata.
 *
 * `pos` can be too large or negative, which results in natural wrapping
 * around the list (a value of -1 would be the tail, and so on).
 *
 * return:
 *  0:          OK.
 *  -ENOMEM:    Aborted; failure to allocate the node.
 */
__attribute__((nonnull, warn_unused_result))
int     alx_llist_insert_at (struct Alx_LinkedList *list,
                             const void *data, size_t size, ptrdiff_t pos);

/*
 * Removes the first node from the list.
 * Deallocates memory from the node and from the data, and updates any
 * necessary metadata.
 *
 * return:
 *  0:          OK.
 *  ENOENT:     Aborted; the list is already empty.
 */
__attribute__((nonnull))
int     alx_llist_remove_head   (struct Alx_LinkedList *list);

/*
 * Removes the last node from the list.
 * Deallocates memory from the node and from the data, and updates any
 * necessary metadata.
 *
 * return:
 *  0:          OK.
 *  ENOENT:     Aborted; the list is already empty.
 */
__attribute__((nonnull))
int     alx_llist_remove_tail   (struct Alx_LinkedList *list);

/*
 * Removes the `node` from the list.
 * Deallocates memory from the node and from the data, and updates any
 * necessary metadata.
 *
 * return:
 *  0:          OK.
 *  -ENOENT:    Aborted; the list is already empty.
 */
__attribute__((nonnull))
int     alx_llist_remove_node   (struct Alx_LinkedList *list,
                                 struct Alx_LLNode *node);

/*
 * Removes all nodes from the list.
 * Deallocates memory from the nodes and from the data, and updates any
 * necessary metadata.
 */
__attribute__((nonnull))
void    alx_llist_remove_all    (struct Alx_LinkedList *list);

/*
 * Finds the `node` in the list.
 *
 * return:
 *  >= 0:       Found at that position relative to the head.
 *  -ENOENT:    Not found.
 */
__attribute__((nonnull, pure, warn_unused_result))
ptrdiff_t alx_llist_find    (const struct Alx_LinkedList *list,
                             const struct Alx_LLNode *node);

/*
 * Gets a pointer to the `node` in the position `pos` relative to the head.
 *
 * return:
 *  0:          OK.
 *  > 0:        OK; wrapped around the end of the list those many times.
 */
__attribute__((nonnull))
int     alx_llist_get_node_at   (const struct Alx_LinkedList *list,
                                 struct Alx_LLNode **node,
                                 ptrdiff_t pos);

/*
 * Gets a pointer to the `node` in the position `pos` relative to `ref`.
 *
 * return:
 *  0:          OK.
 *  > 0:        OK; wrapped around the end of the list those many times.
 */
__attribute__((nonnull))
int     alx_llist_get_relative  (const struct Alx_LinkedList *list,
                                 struct Alx_LLNode **node,
                                 const struct Alx_LLNode *ref,
                                 ptrdiff_t pos);

/*
 * Moves the `node` in the list to the position `pos` in the list.
 * If `pos` is negative, it wraps around (-1 is the tail, and so on).
 * If the position is too large
 * (pos <= (-list->nmemb + 1)  ||  pos >= (list->nmemb - 1)), it doesn't
 * wrap around.  In that case, if `pos` is negative, the node is moved to the
 * head, and if `pos` is positive, the node is moved to the tail.
 */
__attribute__((nonnull))
void    alx_llist_move_node_to  (struct Alx_LinkedList *list,
                                 struct Alx_LLNode *node, ptrdiff_t pos);

/*
 * Moves the `node` in the list to the position `pos` relative to the `node`'s
 * position at the moment of the call.
 * If `pos` is negative, it wraps around (-1 is the previous, and so on).  If
 * `pos` is negative and sufficiently large, it wraps around to the tail.
 * If the position is too large
 * (pos <= (-list->nmemb + 1)  ||  pos >= (list->nmemb - 1)), it doesn't
 * wrap around any more.  In those cases, the node isn't moved at all.
 */
__attribute__((nonnull))
void    alx_llist_move_relative (struct Alx_LinkedList *list,
                                 struct Alx_LLNode *node, ptrdiff_t pos);

/*
 * Edits the node data.
 * Reallocates memory for the the data, copies the data passed by the user to
 * the reallocated space, and updates any necessary metadata.
 *
 * return:
 *  0:          OK.
 *  -ENOMEM:    Aborted; failure to reallocate the data.  Previous
 *              data is left untouched.
 */
__attribute__((nonnull, warn_unused_result))
int     alx_llist_edit_node_data(struct Alx_LLNode *node,
                                 const void *data, size_t size);

/*
 * Sets the `node` in the position `pos` as the new head, and updates any
 * necessary metadata.
 */
__attribute__((nonnull))
void    alx_llist_set_head  (struct Alx_LinkedList *list, ptrdiff_t pos);


/******************************************************************************
 ******* inline ***************************************************************
 ******************************************************************************/


/******************************************************************************
 ******* end of file **********************************************************
 ******************************************************************************/

linked-list.c:

/******************************************************************************
 ******* headers **************************************************************
 ******************************************************************************/
#include "libalx/extra/alx/linked-list.h"

#include <errno.h>
#include <stdlib.h>
#include <string.h>

#include "libalx/base/stdlib/alloc/mallocarrays.h"
#include "libalx/base/stdlib/alloc/mallocs.h"
#include "libalx/base/stdlib/alloc/reallocs.h"


/******************************************************************************
 ******* macros ***************************************************************
 ******************************************************************************/


/******************************************************************************
 ******* enum / struct / union ************************************************
 ******************************************************************************/


/******************************************************************************
 ******* static prototypes ****************************************************
 ******************************************************************************/
__attribute__((nonnull))
static
int     alx_llist_add_first_element (struct Alx_LinkedList *list,
                                     const void *data, size_t size);
__attribute__((nonnull))
static
void    alx_llist_remove_last       (struct Alx_LinkedList *list);


/******************************************************************************
 ******* global functions *****************************************************
 ******************************************************************************/
int alx_llist_init      (struct Alx_LinkedList **list)
{

    if (alx_mallocarrays(list, 1))
        return  -ENOMEM;

    (*list)->head       = NULL;
    (*list)->tail       = NULL;
    (*list)->nmemb      = 0;

    return  0;
}

void    alx_llist_deinit    (struct Alx_LinkedList *list)
{

    alx_llist_remove_all(list);
    free(list);
}

int     alx_llist_prepend   (struct Alx_LinkedList *list,
                             const void *data, size_t size)
{
    struct Alx_LLNode   *node;

    if (!list->nmemb)
        return  alx_llist_add_first_element(list, data, size);

    if (alx_mallocarrays(&node, 1))
        return  -ENOMEM;
    if (alx_mallocs(&node->data, size))
        goto err;
    node->size  = size;

    memcpy(node->data, data, size);
    node->prev  = list->tail;
    node->next  = list->head;

    list->head->prev    = node;
    list->tail->next    = node;

    list->head  = node;
    (list->nmemb)++;

    return  0;
err:
    free(node);
    return  -ENOMEM;
}

int     alx_llist_append    (struct Alx_LinkedList *list,
                             const void *data, size_t size)
{
    struct Alx_LLNode   *node;

    if (!list->nmemb)
        return  alx_llist_add_first_element(list, data, size);

    if (alx_mallocarrays(&node, 1))
        return  -ENOMEM;
    if (alx_mallocs(&node->data, size))
        goto err;
    node->size  = size;

    memcpy(node->data, data, size);
    node->prev  = list->tail;
    node->next  = list->head;

    list->head->prev    = node;
    list->tail->next    = node;

    list->tail  = node;
    (list->nmemb)++;

    return  0;
err:
    free(node);
    return  -ENOMEM;
}

int     alx_llist_insert_before (struct Alx_LinkedList *list,
                                 const void *data, size_t size,
                                 struct Alx_LLNode *ref)
{
    struct Alx_LLNode   *new;

    if (!list->nmemb)
        return  alx_llist_add_first_element(list, data, size);

    if (ref == list->head)
        return  alx_llist_prepend(list, data, size);

    if (alx_mallocarrays(&new, 1))
        return  -ENOMEM;
    if (alx_mallocs(&new->data, size))
        goto err;
    new->size   = size;

    memcpy(new->data, data, size);
    new->prev   = ref->prev;
    new->next   = ref;

    ref->prev->next = new;
    ref->prev   = new;
    (list->nmemb)++;

    return  0;
err:
    free(new);
    return  -ENOMEM;
}

int     alx_llist_insert_after  (struct Alx_LinkedList *list,
                                 const void *data, size_t size,
                                 struct Alx_LLNode *ref)
{
    struct Alx_LLNode   *new;

    if (!list->nmemb)
        return  alx_llist_add_first_element(list, data, size);

    if (ref == list->tail)
        return  alx_llist_append(list, data, size);

    if (alx_mallocarrays(&new, 1))
        return  -ENOMEM;
    if (alx_mallocs(&new->data, size))
        goto err;
    new->size   = size;

    memcpy(new->data, data, size);
    new->prev   = ref;
    new->next   = ref->next;

    ref->next->prev = new;
    ref->next   = new;
    (list->nmemb)++;

    return  0;
err:
    free(new);
    return  -ENOMEM;
}

int     alx_llist_insert_at (struct Alx_LinkedList *list,
                             const void *data, size_t size, ptrdiff_t pos)
{
    struct Alx_LLNode   *tmp;

    if (!list->nmemb)
        return  alx_llist_add_first_element(list, data, size);

    alx_llist_get_node_at(list, &tmp, pos);
    return  alx_llist_insert_before(list, data, size, tmp);
}

int     alx_llist_remove_head   (struct Alx_LinkedList *list)
{
    struct Alx_LLNode   *node;

    switch (list->nmemb) {
    case 0:
        return  ENOENT;
    case 1:
        alx_llist_remove_last(list);
        return  0;
    }

    node    = list->head;
    free(node->data);

    list->head->prev->next  = node->next;
    list->head->next->prev  = node->prev;
    list->head              = node->next;
    free(node);
    (list->nmemb)--;

    return  0;
}

int     alx_llist_remove_tail   (struct Alx_LinkedList *list)
{
    struct Alx_LLNode   *node;

    switch (list->nmemb) {
    case 0:
        return  ENOENT;
    case 1:
        alx_llist_remove_last(list);
        return  0;
    }

    node    = list->tail;
    free(node->data);

    list->tail->prev->next  = node->next;
    list->tail->next->prev  = node->prev;
    list->tail              = node->prev;
    free(node);
    (list->nmemb)--;

    return  0;
}

int     alx_llist_remove_node   (struct Alx_LinkedList *list,
                                 struct Alx_LLNode *node)
{

    switch (list->nmemb) {
    case 0:
        return  -ENOENT;
    case 1:
        alx_llist_remove_last(list);
        return  0;
    }

    if (node == list->head)
        return  alx_llist_remove_head(list);
    if (node == list->tail)
        return  alx_llist_remove_tail(list);

    free(node->data);

    node->prev->next    = node->next;
    node->next->prev    = node->prev;
    free(node);
    (list->nmemb)--;

    return  0;
}

void    alx_llist_remove_all    (struct Alx_LinkedList *list)
{
    ptrdiff_t   n;

    n   = list->nmemb;
    if (!n)
        return;

    for (ptrdiff_t i = 0; i < n; i++)
        alx_llist_remove_tail(list);
}

ptrdiff_t alx_llist_find    (const struct Alx_LinkedList *list,
                             const struct Alx_LLNode *node)
{
    struct Alx_LLNode   *tmp;

    tmp = list->head;
    for (ptrdiff_t i = 0; i < list->nmemb; i++) {
        if (tmp == node)
            return  i;
        tmp = tmp->next;
    }

    return  -ENOENT;
}

int     alx_llist_get_node_at   (const struct Alx_LinkedList *list,
                                 struct Alx_LLNode **node,
                                 ptrdiff_t pos)
{

    return  alx_llist_get_relative(list, node, list->head, pos);
}

int     alx_llist_get_relative  (const struct Alx_LinkedList *list,
                                 struct Alx_LLNode **node,
                                 const struct Alx_LLNode *ref,
                                 ptrdiff_t pos)
{
    int status;

    status  = 0;
    *node   = (struct Alx_LLNode *)ref;
    if (pos >= 0) {
        for (ptrdiff_t i = 0; i < pos; i++) {
            *node   = (*node)->next;
            if (*node == list->head)
                status++;
        }
    } else {
        for (ptrdiff_t i = 0; i < pos; i++) {
            *node   = (*node)->prev;
            if (*node == list->tail)
                status++;
        }
    }

    return  status;
}

void    alx_llist_move_node_to  (struct Alx_LinkedList *list,
                                 struct Alx_LLNode *node, ptrdiff_t pos)
{
    struct Alx_LLNode   *ref;

    if (list->nmemb < 2)
        return;

    node->prev->next    = node->next;
    node->next->prev    = node->prev;
    if (node == list->tail)
        list->tail      = node->prev;
    if (node == list->head)
        list->head      = node->next;

    if (pos <= (-list->nmemb + 1)  ||  pos >= (list->nmemb - 1)) {
        ref = list->head;
    } else {
        if (pos < 0)
            alx_llist_get_node_at(list, &ref, pos + 1);
        else
            alx_llist_get_node_at(list, &ref, pos);
    }

    node->prev  = ref->prev;
    node->next  = ref;

    ref->prev->next = node;
    ref->prev       = node;
    if (ref == list->head) {
        if (pos == -1  ||  pos > 0)
            list->tail  = node;
        else
            list->head  = node;
    }
}

void    alx_llist_move_relative (struct Alx_LinkedList *list,
                                 struct Alx_LLNode *node, ptrdiff_t pos)
{
    struct Alx_LLNode   *ref;

    if (list->nmemb < 2)
        return;
    if (!pos  ||  pos <= (-list->nmemb + 1)  ||  pos >= (list->nmemb - 1))
        return;
    if (pos > 0)
        pos++;

    alx_llist_get_relative(list, &ref, node, pos);

    node->prev->next    = node->next;
    node->next->prev    = node->prev;
    if (node == list->tail)
        list->tail      = node->prev;
    if (node == list->head)
        list->head      = node->next;

    node->prev  = ref->prev;
    node->next  = ref;

    ref->prev->next = node;
    ref->prev   = node;
    if (ref == list->head) {
        if (pos < 0)
            list->head  = node;
        else
            list->tail  = node;
    }
}

int     alx_llist_edit_node_data(struct Alx_LLNode *node,
                                 const void *data, size_t size)
{

    if (alx_reallocs(&node->data, size))
        return  -ENOMEM;
    node->size  = size;

    memmove(node->data, data, size);

    return  0;
}

void    alx_llist_set_head  (struct Alx_LinkedList *list, ptrdiff_t pos)
{
    struct Alx_LLNode   *node;

    if (list->nmemb < 2)
        return;

    alx_llist_get_node_at(list, &node, pos);
    list->head  = node;
    list->tail  = node->prev;
}


/******************************************************************************
 ******* static function definitions ******************************************
 ******************************************************************************/
static
int alx_llist_add_first_element (struct Alx_LinkedList *list,
                     const void *data, size_t size)
{
    struct Alx_LLNode   *node;

    if (alx_mallocarrays(&node, 1))
        return  -ENOMEM;
    if (alx_mallocs(&node->data, size))
        goto err;

    memcpy(node->data, data, size);
    node->prev  = node;
    node->next  = node;

    list->head  = node;
    list->tail  = node;
    list->nmemb = 1;

    return  0;
err:
    free(node);
    return  -ENOMEM;
}

static
void    alx_llist_remove_last       (struct Alx_LinkedList *list)
{
    struct Alx_LLNode   *node;

    node    = list->head;
    free(node->data);

    list->head  = NULL;
    list->tail  = NULL;
    free(node);
    list->nmemb = 0;
}


/******************************************************************************
 ******* end of file **********************************************************
 ******************************************************************************/

Other functions and macros used within the code:

/*
 * [[gnu::nonnull]]
 * int  alx_mallocarrays(type **restrict ptr, ptrdiff_t nmemb);
 */
#define alx_mallocarrays(ptr, nmemb)    (                   \
{                                                           \
    __auto_type ptr_    = (ptr);                            \
                                                            \
    *ptr_   = alx_mallocarray(nmemb, sizeof(**ptr_));       \
                                                            \
    !(*ptr_);                                               \
}                                                           \
)


__attribute__((malloc, warn_unused_result))
inline
void    *alx_mallocarray    (ptrdiff_t nmemb, size_t size)
{

    if (nmemb < 0)
        goto ovf;
    if (!size)
        return  NULL;
    if ((size_t)nmemb > (SIZE_MAX / size))
        goto ovf;

    return  malloc(size * (size_t)nmemb);
ovf:
    errno   = ENOMEM;
    return  NULL;
}


__attribute__((nonnull, warn_unused_result))
inline
int alx_mallocs     (void **restrict ptr, size_t size)
{

    *ptr    = malloc(size);

    return  !(*ptr);
}


__attribute__((nonnull, warn_unused_result))
inline
int alx_reallocs    (void **restrict ptr, size_t size)
{

    *ptr    = realloc(*ptr, size);

    return  !(*ptr);
}

Thanks a lot to those who contributed to these improvements!

I'd like to know any improvements, ideas, and anything you may think about this code (and docs)!

\$\endgroup\$
1
  • \$\begingroup\$ Wow, a lot of code. \$\endgroup\$ – Anatolii Dec 19 '19 at 22:16
2
\$\begingroup\$

Be more productive with an auto formatter

Code certainty is hand formatted. Although it makes for a nice presentation, it is not worth the time versus value - especially when maintenance is concerned. Use a good auto formatted instead.

Consider restrict`

With functions like alx_llist_remove_node(), consider restrict. It lets the compiler know the pointed to data does not overlap and thus allows for more optimization opportunities.

// int alx_llist_remove_node(struct Alx_LinkedList *list, struct Alx_LLNode *node);
int alx_llist_remove_node(struct Alx_LinkedList * restrict list, 
    struct Alx_LLNode * restrictnode);

Consider alx_llist_remove_all(NULL)

C allows free(NULL). It is a useful idiom that encourages good code clean-up. Consider re-write to allow alx_llist_remove_all(NULL).

Negation of error value unclear

I see no advantage of return -ENOMEM; over return ENOMEM;. IMO, it adds confusion. If code wants a new error value, then define it #define E_MY_NEW_ERROR (-ENOMEM) or the like.

Consider an apply function

This function is applied to each element of the queue. It is very useful.

int alx_apply(struct Alx_LinkedList *list, int (*f)(void *state, void *data), 
    void *state);

Tip: If return value is not 0, quite the loop early. IAC, return the last int.

Opaque structures

I would hide the members of struct Alx_LLNode, struct Alx_LinkedList from outside users. No need for them to see/use these members.

Unclear need for cast

No mathematical reason for (size_t).

ptrdiff_t nmemb
...
if ((size_t)nmemb > (SIZE_MAX / size)) goto ovf;

It is reasonable to assume PTRDIFF_MAX <= SIZE_MAX, but there is no need to make that assumption and risk truncation.

Either go cast-less

if (nmemb > (SIZE_MAX / size)) goto ovf;

Or quiet warnings about mis-matched signed/unsigned compare with a type that will not truncate.

if ((uintmax_t)nmemb > (SIZE_MAX / size)) goto ovf;
\$\endgroup\$
6
  • \$\begingroup\$ Very good points. About opaque structures, I usually prefer to give the struct to the user, even if that allows him to shoot in his feet. In this case, I think it can be useful for the user to be able to (at least) read the values of the structs. I considered giving the user a consted version of the struct so that the user could only modify *data, and could read everything else. \$\endgroup\$ – alx Dec 20 '19 at 9:06
  • \$\begingroup\$ "Consider alx_llist_remove_all(NULL)": I think the use of that function is suitable when you plan to continue using the list, and therefore a NULL list doesn't make much sense. However, you are right in that something like free(NULL) would help cleanup. I think it would be good to disallow NULL on that one but allow it on alx_llist_deinit(NULL); \$\endgroup\$ – alx Dec 20 '19 at 9:15
  • \$\begingroup\$ About using restrict: I used restrict at first, but then I thought that it might be possible that someone would want to copy the contents of a node into another, and restrict wouldn't allow that (for example, this: alx_llist_append(list, list->head->data, list->head->size);. I can imagine cases where that would be useful. \$\endgroup\$ – alx Dec 20 '19 at 10:02
  • 1
    \$\begingroup\$ @CacahueteFrito "prefer to give the struct to the user," --> does then freeze the data structure going forward. Should the underlying struct benefit with a change, one is stuck with the original design - else break your user base. This prevents code advancement. I'd rather supply select getter functions. Think of the FILE. We only use FILE *, allowing many different implementations over decades as we only use a functional interface. Of course, your call. \$\endgroup\$ – chux - Reinstate Monica Dec 20 '19 at 13:49
  • 1
    \$\begingroup\$ @CacahueteFrito BTW: good job providing function documentation on the .h file. static/inline functions in the .c file would benefit with some though. \$\endgroup\$ – chux - Reinstate Monica Dec 20 '19 at 13:51
1
\$\begingroup\$

Don't use #pragma once. It is a gcc extension that doesn't work elsewhere. Instead, write something like:

#ifndef LINKED_LIST_H
#define LINKED_LIST_H 1
...all the code here...
#endif

This is portable, effective, and (provided you use different tokens in each header) safe.

\$\endgroup\$
1
  • \$\begingroup\$ #pragma once is not GCC specific. In fact, it is implemented in most compilers today. IMO, header guards are more dangerous than the pragma, because you have to remember to update them for every file (I usually copy&paste them from other files), and that bug has happened to me many times, while I yet have to encounter a problem with the pragma. More interesting info here and here \$\endgroup\$ – alx Dec 19 '19 at 22:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.