Logging library for C

Over the past couple of weeks I have been working on a general purpose logging library for C in my free time. At the moment I have stabilized the API and most of the features I wanted to implement have been implemented. I am looking for general suggestions (note I haven't done much API documentation yet) on code quality and correctness (especially the synchronization techniques, IO methods, and message building functions).

You can find the full repository here.

And below I have included the sample usage that appears in the repo:

#include 
#include 
#include "scribe.h"

int main(void)
{
    if (SCRB_Success != scrb_init()) {
        return EXIT_FAILURE;
    }

    // We open a new file called `example.log` in write mode without synchronization that only
    // listens to info level log messages.
    struct scrb_stream * const logstream = scrb_open_stream("example.log", "w", false, LVL_INFO);

    // We define a log format to tell scribe how to format our messages.
    // This format prints out the file, method, and line locations followed by the message itself.
    struct scrb_format const * const fmt = scrb_create_format("[%F:%L:%M] %m", NULL);

    // Now we're ready to go.
    scrb_logln(logstream, fmt, LVL_INFO, "Hello from the example program!");

    // We can do formatted output as well.
    scrb_flogln(logstream, fmt, LVL_INFO, "Hello for the %dnd time!", 2);

    // If we want to write directly to the file without formatting or location/time info
    // (or paying attention to log levels) we can use the `scrb_putstrln` method to write directly to the file.
    scrb_putstrln(logstream, "Now we're done.");

    scrb_format_release(&fmt);
    scrb_close_stream(&logstream);
    return EXIT_SUCCESS;
}

I have written a synchronized version similar to the above wherein 4 separate threads all log to the same file, which you may find the the examples folder.

Here is the scribe.h header file (as per request):

```
#ifndef SCRIBE_H
#define SCRIBE_H

#i

It'll take some time to describe everything, and I need to pretend to do some work at some point too. However...

In scribe.h:

-
struct scrb_stream outstream = (struct scrb_stream) { ... }; you are assigning a compound literal instead of just initializing the structure. Lose the (struct scrb_stream) cast notation in these lines.

-
Another way of writing:

if (severity & st->severity) {
    va_list ap;
    va_start (ap, msgfmt);
    int const rval = scrb_flog__internal(mi, st, fmt, severity, msgfmt, false, ap);
    va_end (ap);
    return rval;
} else {
    return SCRB_Success;
}

is:

int rval = SCRB_SUCCESS;
if (severity & st->severity) {
    va_list ap;
    va_start(ap, msgfmt);
    rval = scrb_flog__internal(mi, st, fmt, severity, msgfmt, false, ap);
    va_end(ap);
}
return rval;

-
You have a collection of functions such as:

static inline
void (scrb_close_stream)(struct scrb_stream ** streamptr)
{
    scrb_close_stream__internal(streamptr);
}
#define scrb_close_stream(streamptr) (scrb_close_stream)((struct scrb_stream **)(streamptr))

I see multiple issues, some serious and some less serious.

• Serious: Why do you want to throw away type safety with the macro? The cast in the macro expansion means that you can pass an integer to the function, or a FILE , or a struct scrb_stream (rather than the address of a struct scrb_stream *) and the cast suppresses the error messages that the compiler would otherwise generate. Drop the cast; don't suppress the limited protections that the compiler can provide.

• Serious: You are embedding double underscores into the names. You care about C++ since you've included #ifdef __cplusplus, but C++ reserves double-underscore anywhere in any name for use by the implementation. Don't do it!

• With the cast removed, the macro serves no purpose here.

• What is the perceived benefit of the static inline functions that call onto the *_internal name? In this example, I see no benefit.

-
Some of the macro and inline function combinations worry me:

static inline
int (scrb_flog)(struct scrb_meta_info const mi, 
                struct scrb_stream * const st,
                struct scrb_format const * const fmt, 
                uint16_t const severity,
                char const * const msgfmt, ...)
{
    if (severity & st->severity) {
        va_list ap;
        va_start (ap, msgfmt);
        int const rval = scrb_flog__internal(mi, st, fmt, severity, msgfmt, false, ap);
        va_end (ap);
        return rval;
    } else {
        return SCRB_Success;
    }
}
#define scrb_flog(st, fmt, sv, msg, ...) (scrb_flog)(get_meta_info((st)->name), (st), (fmt), (sv), (msg), ##__VA_ARGS__)

Apart from the issues previously raised, here we have a macro scrb_flog() that calls a function scrb_flog() with an extra argument. This will lead to confusion somewhere along the line. It also really isn't necessary; you can avoid the overhead of the get_meta_info() function call in the 'not used' case by writing this as:

static inline
int (scrb_flog)(struct scrb_stream * const st,
                struct scrb_format const * const fmt, 
                uint16_t const severity,
                char const * const msgfmt, ...)
{
    int rval = SCRB_SUCCESS;
    if (severity & st->severity) {
        va_list ap;
        va_start(ap, msgfmt);
        rval = scrb_flog_internal(get_meta_info(st->name), st, fmt, severity, msgfmt, false, ap);
        va_end(ap);

    }
    return rval;
}
#define scrb_flog(st, fmt, sv, msg, ...) (scrb_flog)((st), (fmt), (sv), (msg), ##__VA_ARGS__)

Clearly, the goal here is to avoid moving the if test into the called function. I've not seen measurements to justify that; my suspicion is that the extra code built into the calling functions may be worse than the function call.

If the meta information is determined from the stream, I wonder why the internal function can't determine the meta information from the stream. Without studying the code in more detail than I'm going to, it is not clear why the scrb_meta_info * argument is passed to scrb_flog_internal() rather than letting the function determine the meta info from the stream.

-
You currently have the header doing:

static struct scrb_stream * const scrb_stdout = &stream_out_default;
static struct scrb_stream * const scrb_stdin  = &stream_in_default;
static struct scrb_stream * const scrb_stderr = &stream_err_default;

Defining static variables in a header is dubious. The non-systematic names on the RHS are a red flag; they should be scrb_stream_out_default etc. More significantly, what is the scrb_init_default() function doing, and when might those pointers be changed, and if they're never changed, why don't you just have

extern struct scrb_stream *scrb_stdout;

I'm not sure what flexibility you think you're buying with this extra level of indirection repeated in each object file where the source file uses the

StackExchange Code Review Q#73885, answer score: 2