6
\$\begingroup\$

I've been programming in Go and I enjoy how easy it is to create descriptive errors and propagate them up the call stack. I wanted that sort of ease and consistency in C so I created a small error handling library to propagate errors and generate a sort of error code stack trace.

I've been using C for 6 months so I'm not sure if I have designed this program well. I would appreciate feedback on the API and if it is well-structured. I am unsure if I am adhering well to software engineering best practices. I would also appreciate some feedback on the implementation itself, particularly for the 1. SetNewError_() function and 2. the callbacks (hooks) for the memory allocator and logging mechanisms in the InitError() function.

This is essentially an array-based stack where the user can push error nodes which contain the metadata for a specific error. I tried to reduce usage of dynamic allocation as much as possible, so that the error handler itself isn't prone to failure. So no linked lists and no malloc for strings.

I made a github repository here which contains everything, including the unit tests and examples. I might also appreciate feedback on if the repository is set up correctly, as I am fairly new to source control. https://github.com/birendpatel/XT

Thank you!

Library API: error.h

/*
* NAME: Copyright (c) 2020, Biren Patel
* DESC: API for a lightweight error handling library
* LISC: MIT License
*/

#ifndef ERROR_H
#define ERROR_H

/*******************************************************************************
* DESC: modifiable macros
* NOTE: each definition must be >= 1
* @ ERROR_FNAME_SIZE  : maximum chars alloted for file name
* @ ERROR_FXNAME_SIZE : maximum chars alloted for function name
* @ ERROR_DESC_SIZE   : maximum chars alloted for error description
*******************************************************************************/
#define ERROR_FNAME_SIZE    16
#define ERROR_FXNAME_SIZE   16
#define ERROR_DESC_SIZE     24

/*******************************************************************************
* DESC: error handling and API set up
*******************************************************************************/
#include <stdlib.h>
#include <stdio.h>
#include <stdbool.h>
#include <stdarg.h>

#ifndef __clang__
    #ifndef __GNUC__
        #error error.h recommends Clang or GCC for function attributes \
               disable this directive to continue compilation
    #endif
#endif

#ifdef __STDC_VERSION__
    #if __STDC_VERSION__ >= 199901L
        #define ERROR_H_VA_ARGS_OK
    #endif
#endif

#if ERROR_FNAME_SIZE < 1
    #error error.h fname size is too small
#endif

#if ERROR_FXNAME_SIZE < 1
    #error error.h fxname size is too small
#endif

#if ERROR_DESC_SIZE < 1
    #error error.h desc size is too small
#endif

/*******************************************************************************
* NAME: struct error_node_t
* DESC: metadata container for a single error
* @ code   : error code
* @ line   : file line
* @ fname  : file name
* @ fxname : function name
* @ desc   : error description
*******************************************************************************/
struct error_node_t
{
    int code;
    int line;
    char fname[ERROR_FNAME_SIZE];
    char fxname[ERROR_FXNAME_SIZE];
    char desc[ERROR_DESC_SIZE];
};

/*******************************************************************************
* NAME: struct error_t
* DESC: the error stack and end-user handle
* @ cb_log : logging callback
* @ tmp    : placeholder (struct padding allows for it)
* @ lost   : tracks overflow statistics
* @ cap    : total capacity of stack
* @ top    : index of next available element
* @ stack  : LIFO unless overflow is imminent
*******************************************************************************/
typedef struct error_t
{
    void (*cb_log)(int code);
    int tmp;
    int lost;
    int cap;
    int top;
    struct error_node_t stack[];
} *error_t;

/*******************************************************************************
* NAME: InitError
* DESC: initialize a variable with optional callbacks
* OUTP: null if stdlib calloc failed
* NOTE: stdlib calloc used when cb_calloc is NULL. release memory with free.
*
* @ n         : maximum capacity of errors that can be held in error_t
*
* @ cb_calloc : callback for custom calloc which should have the same signature
*               as stdlib calloc and also return NULL on failure. Besides using
*               this for custom memory allocators, with a little extra effort
*               we can overload this pointer to load error_t onto the .DATA
*               segment instead of the heap.
*               
* @ cb_log    : callback for logging errors resulting from API failure. Yes, the
*               error handler itself can fail to handle errors! Use the enum
*               below to track the return codes. Errors are returned by the API
*               functions themselves. If cb_log is non-NULL then errors are both
*               returned and passed to cb_log(). One reason to do this is so you
*               can use the API lazily and have the cb_log passively check all
*               errors and abort/fail/etc if anything goes wrong. The CBLOG
*               enum suffix gives the function acronym.
*******************************************************************************/
struct error_t *InitError
(
    int n,
    void *(*cb_calloc)(size_t num, size_t size),
    void (*cb_log)(int code)
);

//use this to determine error_t byte size when using complex callbacks.
#define ERROR_BYTES(n)                                                         \
(sizeof(struct error_t) + sizeof(struct error_node_t) * (size_t) n)

//these are all possible codes that can be passed to cb_log.
//ERROR_CBLOG_SUCCESS is never passed to cb_log
//ERROR_SNE_VSNPRINTF_FAIL has a fallback mechanism so it's a partial failure.
enum
{
    ERROR_CBLOG_SUCCESS             = 0  ,
    ERROR_CBLOG_STDLIB_CALLOC_FAIL  = 1  ,
    ERROR_CBLOG_CUSTOM_CALLOC_FAIL  = 2  ,
    ERROR_CBLOG_ENIE_NULL_INPUT     = 3  ,
    ERROR_CBLOG_RMAE_NULL_INPUT     = 4  ,
    ERROR_CBLOG_RMRE_NULL_INPUT     = 5  ,
    ERROR_CBLOG_RSAE_NULL_INPUT     = 6  ,
    ERROR_CBLOG_RSRE_NULL_INPUT     = 7  ,
    ERROR_CBLOG_RE_NULL_INPUT       = 8  ,
    ERROR_CBLOG_AE_NULL_INPUT       = 9  ,
    ERROR_CBLOG_EE_NULL_INPUT       = 10 ,
    ERROR_CBLOG_NEE_NULL_INPUT      = 11 ,
    ERROR_CBLOG_SNE_NULL_INPUT      = 12 ,
    ERROR_CBLOG_SNE_VSNPRINTF_FAIL  = 13 ,
    ERROR_CBLOG_RAE_NULL_INPUT      = 14 ,
    ERROR_CBLOG_RAE_FPRINTF_FAIL    = 15 ,
};

/*******************************************************************************
* NAME: SetNewError_, SetNewError, SetNewDetailedError
* DESC: push an error node onto the handle's stack
* NOTE: nodes must use a zero code to indicate success
* NOTE: to prevent uncontrolled format string exploit do not expose function
* @ code   : error code
* @ line   : line number at which SetNewError is called
* @ fname  : file name, not exceeding ERROR_FNAME_SIZE
* @ fxname : function name, not exceeding ERROR_FXNAME_SIZE
* @ desc   : error description, not exceeding ERROR_DESC_SIZE
* @ ...    : optional format parameters for desc
*******************************************************************************/
#if defined(__clang__) || defined(__GNUC__)
    __attribute__((__format__ (__printf__, 6, 7)))
#endif
bool SetNewError_
(
    struct error_t *error,
    int code,
    int line,
    const char *fname,
    const char *fxname,
    const char *desc,
    ...
);

#define SetNewError(error, code, desc)                                         \
SetNewError_(error, code, __LINE__, __FILE__, __func__, desc)


#ifdef ERROR_H_VA_ARGS_OK
    #define SetNewDetailedError(error, code, desc, ...)                        \
    SetNewError_(error, code, __LINE__, __FILE__, __func__, desc, __VA_ARGS__)
#endif

/*******************************************************************************
* NAME: SetNewErrorGo, SetNewDetailedErrorGo
* DESC: wrappers for the commom goto resource cleanup idiom used in C
*******************************************************************************/
#define SetNewErrorGo(error, code, desc, label)                                \
do                                                                             \
{                                                                              \
    SetNewError(error, code, desc);                                            \
    goto label;                                                                \
} while (0)

#ifdef ERROR_H_VA_ARGS_OK    
    #define SetNewDetailedErrorGo(error, code, desc, label, ...)               \
    do                                                                         \
    {                                                                          \
        SetNewDetailedError(error, code, desc, __VA_ARGS__);                   \
        goto label;                                                            \
    } while (0)
#endif

//-*-
//the tough part of the API is done. Below we just have some very simple stack
//manipulation, error checking, and reporting functions. Most are overkill. You
//will probably spend 99% of your time in RecentError() and ReportAllError().

/*******************************************************************************
* NAME: EmptyNodesInError
* DESC: calculate the remaining number of available nodes
* OUTP: negative if input pointer is null
*******************************************************************************/
int EmptyNodesInError(struct error_t *error);

/*******************************************************************************
* NAME: RemoveAllError
* DESC: remove all error nodes from the stack
* OUTP: false if input pointer is null
*******************************************************************************/
bool RemoveAllError(struct error_t *error);

/*******************************************************************************
* NAME: RemoveRecentError
* DESC: remove the error node at the top the stack
* OUTP: false if input pointer is null
*******************************************************************************/
bool RemoveRecentError(struct error_t *error);

/*******************************************************************************
* NAME: ResetAllError
* DESC: completely reset and zero out the error handle to a fresh state
* NOTE: RemoveAllError does synthetic removals and is therefore much faster
*******************************************************************************/
bool ResetAllError(struct error_t *error);

/*******************************************************************************
* NAME: ResetRecentError
* DESC: reset and zero out the error node at the top of the stack
* NOTE: RemoveLastError does synthetic removals and is therefore much faster
*******************************************************************************/
bool ResetRecentError(struct error_t *error);

/*******************************************************************************
* NAME: RecentError
* DESC: Fetch the error code at the top of the stack
* OUTP: negative number on null input and 0 on empty stack
*******************************************************************************/
int RecentError(const struct error_t *error);

/*******************************************************************************
* NAME: AnyError
* DESC: Check if any error code in the stack is nonzero
* OUTP: If any error code is nonzero, return true. zero and empty returns false
*******************************************************************************/
bool AnyError(const struct error_t *error);

/*******************************************************************************
* NAME: ExistsError
* DESC: Check if any error code in the stack matches the argument
* OUTP: true if match, false if no match or empty stack.
*******************************************************************************/
bool ExistsError(const struct error_t *error, const int code);

/*******************************************************************************
* NAME: NotExistsError
* DESC: Check if no error code in the stack matches the argument
* OUTP: true if no match or empty stack, false if match
*******************************************************************************/
bool NotExistsError(const struct error_t *error, const int code);

/*******************************************************************************
* NAME: ReportAllError
* DESC: create a detailed trace of the error stack
* @ stream : write target
*******************************************************************************/
bool ReportAllError(struct error_t *error, FILE *stream);

#endif

Implementation: error.c

/*
* NAME: Copyright (c) 2020, Biren Patel
* DESC: Implementation for a lightweight error handling library
* LISC: MIT License
*/

#include "error.h"
#include <string.h>
#include <assert.h>

/*******************************************************************************
prototypes
*/

static void SafeCopy(char *dest, const char *src, int n);


/*******************************************************************************
file macros
*/

#define LOG_ERROR_IF_FOUND(x)                                                  \
do                                                                             \
{                                                                              \
    if (error->cb_log != NULL) error->cb_log((x));                             \
} while (0)                                                                    \

/******************************************************************************/

struct error_t *InitError
(
    int n,
    void *(*cb_calloc)(size_t num, size_t size),
    void (*cb_log)(int code)
)
{
    if (n <= 0) return NULL;
    
    //override stdlib with callback allocator
    //raw call to cb_log on error since struct isn't configured yet
    struct error_t *error = NULL;
    
    if (cb_calloc == NULL)
    {
        error = calloc(ERROR_BYTES(n), 1);
        if (error == NULL)
        {
            if (cb_log != NULL) cb_log(ERROR_CBLOG_STDLIB_CALLOC_FAIL);
            return NULL;
        }
    }
    else
    {
        error = cb_calloc(ERROR_BYTES(n), 1);
        if (error == NULL) 
        {
            if (cb_log != NULL) cb_log(ERROR_CBLOG_CUSTOM_CALLOC_FAIL);
            return NULL;
        }
    }
    
    //hook the logging mechanism if available
    if (cb_log != NULL) error->cb_log = cb_log;
    else error->cb_log = NULL;
    
    error->cap = n;
    
    return error;
}


/******************************************************************************/

int EmptyNodesInError(struct error_t *error)
{
    if (error == NULL) 
    {
        LOG_ERROR_IF_FOUND(ERROR_CBLOG_ENIE_NULL_INPUT);
        return -1;
    }
    
    return error->cap - error->top;
}

/*******************************************************************************
removal functions are just synthetic pops for speed, unlike the reset functions
*/

bool RemoveAllError(struct error_t *error)
{
    if (error == NULL) 
    {
        LOG_ERROR_IF_FOUND(ERROR_CBLOG_RMAE_NULL_INPUT);
        return false;
    }
    
    error->top = 0;
    
    return true;
}

/******************************************************************************/

bool RemoveRecentError(struct error_t *error)
{
    if (error == NULL) 
    {
        LOG_ERROR_IF_FOUND(ERROR_CBLOG_RMRE_NULL_INPUT);
        return false;
    }
    
    if (error->top != 0)
    {
        error->top--;
    }
    
    return true;
}

/*******************************************************************************
completely zero out the struct instead of just decrementing the top pointer.
This is helpful when we're inspecting memory maps and raw hex and whatever else
where the synthetic pop would just muddy the output with gibberish.
*/

bool ResetAllError(struct error_t * error)
{
    if (error == NULL) 
    {
        LOG_ERROR_IF_FOUND(ERROR_CBLOG_RSAE_NULL_INPUT);
        return false;
    }
    
    //capacity and logging hook need to survive the reset
    int n = error->cap;
    void (*cb_log)(int code) = error->cb_log;
    
    memset(error, 0, ERROR_BYTES(n));
    
    error->cb_log = cb_log;
    error->cap = n;
    
    return true;
}

/******************************************************************************/

bool ResetRecentError(struct error_t * error)
{
    if (error == NULL) 
    {
        LOG_ERROR_IF_FOUND(ERROR_CBLOG_RSRE_NULL_INPUT);
        return false;
    }
    
    if (error->top != 0)
    {
        memset(&error->stack[error->top], 0, sizeof(struct error_node_t));
        error->top--;
    }
    
    return true;
}


/******************************************************************************/

int RecentError(const struct error_t *error)
{
    if (error == NULL) 
    {
        LOG_ERROR_IF_FOUND(ERROR_CBLOG_RE_NULL_INPUT);
        return -1;
    }
    
    if (error->top == 0) return 0;
    
    return error->stack[error->top - 1].code;
}


/*******************************************************************************
sweep the entire stack for a nonzero code.
*/

bool AnyError(const struct error_t *error)
{
    if (error == NULL) 
    {
        LOG_ERROR_IF_FOUND(ERROR_CBLOG_AE_NULL_INPUT);
        return true;
    }
    
    if (error->top == 0) return false;
    
    int i = error->top;
    
    while (i--)
    {
        if (error->stack[i].code != 0) return true;
    }
    
    return false;    
}

/*******************************************************************************
This is handy for generating SQL-like idioms. Sweep the stack like AnyError but
for a specific code.
*/

bool ExistsError(const struct error_t *error, const int code)
{
    if (error == NULL) 
    {
        LOG_ERROR_IF_FOUND(ERROR_CBLOG_EE_NULL_INPUT);
        return false;
    }
    
    if (error->top == 0) return false;
    
    int i = error->top;
    
    while (i--)
    {
        if (error->stack[i].code == code) return true;
    }
    
    return false;
}

/******************************************************************************/

bool NotExistsError(const struct error_t *error, const int code)
{
    if (error == NULL) 
    {
        LOG_ERROR_IF_FOUND(ERROR_CBLOG_NEE_NULL_INPUT);
        return false;
    }
    if (error->top == 0) return true;
    
    int i = error->top;
    
    while (i--)
    {
        if (error->stack[i].code == code) return false;
    }
    
    return true;
}

/*******************************************************************************
Place an error on the stack, if the vsnprintf function fails we fall back on a
straight copy without the va_args. the preprocessor pragmas temporarily disable
any -Wformat-security warnings which will complain about non string literals
being passed to vsnprintf. Since this is an error handling library, an end user
shouldn't have access to control error descriptions so a string exploit is not
an issue.
*/

#ifdef __clang__
    #pragma clang diagnostic push
    #pragma clang diagnostic ignored "-Wformat-security"
#endif

#ifdef __GNUC__
    #pragma GCC diagnostic push
    #pragma GCC diagnostic ignored "-Wformat-security"
#endif

bool SetNewError_
(
    struct error_t *error,
    int code,
    int line,
    const char *fname,
    const char *fxname,
    const char *desc,
    ...
)
{
    if (error == NULL) 
    {
        LOG_ERROR_IF_FOUND(ERROR_CBLOG_SNE_NULL_INPUT);
        return false;
    }
    
    va_list args;
    va_start(args, desc);
    
    //prevent stack overflow by removing the oldest node
    if (error->top == error->cap)
    {
        if (error->cap != 1)
        {
            size_t bytes = sizeof(struct error_node_t) * (size_t) (error->top - 1);
            memmove(error->stack, error->stack + 1, bytes);
        }
        
        error->top--;
        error->lost++;
    }
    
    error->stack[error->top].code = code;
    error->stack[error->top].line = line;
    
    //copy fname and fxname by hand so that no possible error from snprint
    SafeCopy(error->stack[error->top].fname, fname, ERROR_FNAME_SIZE);
    SafeCopy(error->stack[error->top].fxname, fxname, ERROR_FXNAME_SIZE);
    
    //if vsnprintf fails then fall back to a safe copy on the raw format string
    if (desc == NULL) 
    {
        error->stack[error->top].desc[0] = '\0';
    }
    else
    {
        int status = vsnprintf(error->stack[error->top].desc, ERROR_DESC_SIZE, desc, args);
        
        if (status < 0)
        {
            LOG_ERROR_IF_FOUND(ERROR_CBLOG_SNE_VSNPRINTF_FAIL);
            SafeCopy(error->stack[error->top].desc, desc, ERROR_DESC_SIZE);
        }
    }
    
    error->top++;
    va_end(args);
    
    assert(error->top <= error->cap && "top of stack exceeds capacity");
    
    return true;
}

#ifdef __clang__
    #pragma clang diagnostic pop
#endif

#ifdef __GNUC__
    #pragma GCC diagnostic pop
#endif

/*******************************************************************************
Helper function for SetNewError_, safe copy to src as a fallback mechanism in
the extremely rare case that vsnprintf fails. This leaves the format params
in the description, but having some description is better than nothing.
*/

static void SafeCopy(char *dest, const char *src, int n)
{
    assert(dest != NULL && "destination member on error stack is empty");
    assert(n != 0 && "destination member has impossible length of zero");
    
    size_t i = 0;
    size_t limit = (size_t) n;
        
    if (src == NULL) goto null_terminate;
    
    size_t end = strlen(src);
    
    while (i < limit - 1)
    {
        if (i == end) goto null_terminate;
        else dest[i] = src[i];
        
        i++;
    }
    
    null_terminate:
        dest[i] = '\0';
        return;
}

/******************************************************************************/

bool ReportAllError(struct error_t *error, FILE *stream)
{
    if (error == NULL) 
    {
        LOG_ERROR_IF_FOUND(ERROR_CBLOG_RAE_NULL_INPUT);
        return false;
    }
    
    int i = error->top;
    int status = 0;
    
    status = fprintf(stream, "\n-*-\nerror code stack trace:\n");
    
    if (status < 0) 
    {
        LOG_ERROR_IF_FOUND(ERROR_CBLOG_RAE_FPRINTF_FAIL);
        return false;
    }
    
    if (i == 0)
    {
        status = fprintf(stream, "No error nodes found in stack\n");
        
        if (status < 0) 
        {
            LOG_ERROR_IF_FOUND(ERROR_CBLOG_RAE_FPRINTF_FAIL);
            return false;
        }
    }
    
    while (i--)
    {
        const int a = error->stack[i].code;
        const char *b = error->stack[i].desc;
        const char *c = error->stack[i].fname;
        const char *d = error->stack[i].fxname;
        const int e = error->stack[i].line;
        
        const char *call_fmt = "%d. [error %d \"%s\" (%s:%s:%d)]\n";
        status = fprintf(stream, call_fmt, i, a, b, c, d, e);
        
        if (status < 0) 
        {
            LOG_ERROR_IF_FOUND(ERROR_CBLOG_RAE_FPRINTF_FAIL);
            return false;
        }
    }
    
    char *sum_fmt = "\nsummary:\n%d errors collected in stack\n";
    status = fprintf(stream, sum_fmt, error->top);
    
    if (status < 0) 
    {
        LOG_ERROR_IF_FOUND(ERROR_CBLOG_RAE_FPRINTF_FAIL);
        return false;
    }
    
    const char *pop_fmt = "%d errors disregarded at bottom of stack\n-*-\n";
    status = fprintf(stream, pop_fmt, error->lost);
    
    if (status < 0) 
    {
        LOG_ERROR_IF_FOUND(ERROR_CBLOG_RAE_FPRINTF_FAIL);
        return false;
    }
    
    return true;
}

```
\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

That certainly looks a lot better than the code I was writing with six months' experience!

    #error error.h recommends Clang or GCC for function attributes \
           disable this directive to continue compilation

If that's just "recommends", then shouldn't this be a warning, rather than an error? Or change "recommends" to "requires" for more honesty!

struct error_node_t

Names ending in _t are reserved for use by the Standard Library, so avoid them.

Here's a weakness of this library:

//these are all possible codes that can be passed to cb_log.

Sadly, this means the library is not extensible (library users can't add their own error codes, but need to have them present in the library). Solving that is a hard problem, so perhaps wise to defer that for a while.

#if defined(__clang__) || defined(__GNUC__)
    __attribute__((__format__ (__printf__, 6, 7)))
#endif

This is really good: work with the compiler to help catch usage errors.

#define SetNewErrorGo(error, code, desc, label)                                \
do                                                                             \
{                                                                              \
    SetNewError(error, code, desc);                                            \
    goto label;                                                                \
} while (0)

More good work - the do ... while(0) idiom is well-recognised practice for multi-statement macros.

if (error == NULL) 
{

It's simpler and idiomatic to use ! operator on this pointer: if (!error), unless your local coding standard requires an comparison.

#pragma GCC diagnostic ignored "-Wformat-security"

I experimented (with a few compilers) and never triggered the diagnostic this suppresses. So perhaps it can be removed?

static void SafeCopy(char *dest, const char *src, int n)

Good idea, though it's strange to accept a signed int and cast it to size_t (especially without checking whether it's negative). Much better to accept size_t directly (especially given the common pattern char buf[SIZE]; SafeCopy(buf, src, sizeof buf);). Also, we don't need to copy character-by-character:

static void SafeCopy(char *dest, const char *src, size_t n)
{
    assert(dest);
    assert(n);

    if (!src) { src = ""; }

    size_t len = strlen(src);
    if (len >= n) {
        len = n - 1;
    }

    memcpy(dest, src, len);
    dest[len] = '\0';
}

Although not part of this review, I did peek at the makefile in the git repository. If we used the standard CC and CFLAGS variables, we would be able to use the built-in rules to compile, and merely specify the dependencies in the makefile. And users could override those variables in the expected way.

\$\endgroup\$
3
  • \$\begingroup\$ Thank you for this in-depth response! I have a couple of questions. For if (error) I am struggling to understand why this is equivalent if (error == NULL). I thought that NULL pointers always evaluate to false. So that the only acceptable alternative is if (!error). Am I missing something? Second, when you say this library is not extensible due to the cb_log enum codes, could you expand on that briefly? I believe what you're implying is that if I add more codes to the enum in the future, this makes it hard for users to go back and edit their programs which depend on the specific codes. \$\endgroup\$
    – Ollie
    Commented Nov 20, 2020 at 17:29
  • \$\begingroup\$ You're right - I meant !error there. Sorry for that mistake. What I meant by "not extensible" is different to how you understood me: users aren't able to add their own return codes - they only get what's provided by the library. I hope that makes sense. \$\endgroup\$ Commented Nov 20, 2020 at 21:03
  • \$\begingroup\$ That comment on extensibility makes sense, thanks. \$\endgroup\$
    – Ollie
    Commented Nov 21, 2020 at 0:07
2
\$\begingroup\$
  • DRY. Initialization can be streamlined:

      if (cb_calloc == NULL) {
          cb_calloc = calloc;
      }
      error = cb_calloc(ERROR_BYTES(n), 1);
      ....
    

    Along the same line, testing for cb_log == NULL is unnecessary. error->cb_log becomes equal to cb_log no matter what.

  • Avoid special cases. Provide a fallback instead. Consider

      if (cb_log == NULL) {
          cb_log = default_cb_log;
      }
      error->cb_log = cb_log;
    

    This way the not-so-pretty LOG_ERROR_IF_FOUND goes away. The default callback could be as simple as static void default_callback(....) {}.

  • Avoid goto. SafeCopy doesn't need it. Consider

      if (s == 0) {
          s = "";
      }
      end = strlen(s);
      size_t to_copy = min(end, limit - 1);
      strncpy(dest, s, limit);
    
  • Use correct data structure. Since you prevent stack overflow by removing the oldest node, a circular buffer seems more reasonable (no need to memmove anything) than a stack.

  • It would be nice to see some usage examples.

\$\endgroup\$
1
  • \$\begingroup\$ This is a wonderful answer, thank you. Your comment on using a ring buffer instead led me to google around and I've realized that Open GL actually has a somewhat similar mechanism to queue up errors, which the user can then dequeue and take action. The default callback is awesome! I wish I could have accepted your answer too, as both were equally valuable. \$\endgroup\$
    – Ollie
    Commented Nov 21, 2020 at 0:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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