A thread-safe simple logger

Question

Since the vprintf is not thread-safe, so I use a std::mutex.

It's a pity that there are a lot similar macros(both name and function) in the code below.

#include <stdio.h>
#include <type_traits>
#include <mutex>
#include <stdarg.h>

typedef enum 
{
    NONE,
    DEBUG,
    WARNING,
    ERROR,
}log_level_em;

class SimpleLog
{
private:
    SimpleLog() = default;
    SimpleLog(const SimpleLog&) = delete;
    SimpleLog& operator=(const SimpleLog&) = delete;
public:
    static SimpleLog& get_instance()
    {
        static SimpleLog logger;
        return logger;
    }

    void set_block_level(log_level_em level)
    {
        block_level = level;
    }

    bool block_log(log_level_em level)
    {
        return static_cast<std::underlying_type<log_level_em>::type>(level) <= \
               static_cast<std::underlying_type<log_level_em>::type>(block_level);
    }

    void log_output(log_level_em level, const char* fmt, ...)
    {
        if(block_log(level))
        {
            return;
        }

        va_list args;
        va_start(args, fmt);
        {
            std::unique_lock<std::mutex> lock{mtx};
            vprintf(fmt, args);
        }
        va_end(args);
    }

    log_level_em block_level{NONE};
    std::mutex mtx;
};

#define LOG_SET_LEVEL(lvl)                          \
do{                                                 \
    SimpleLog::get_instance().set_block_level(lvl); \
} while(false);

// Define the LOG_COMMON macro to accept variadic parameters
#define LOG_COMMON(log_level, fmt, ...)                                        \
do{                                                                            \
    SimpleLog::get_instance().log_output(log_level, "[%s] [%s:%d] " fmt "\n",  \
                             __FUNCTION__, __FILE__, __LINE__, ##__VA_ARGS__); \
} while(false);

// Define the foo macro to accept variadic parameters and pass them to LOG_COMMON
#define CAM_LOG_ERROR(fmt, ...)   LOG_COMMON(ERROR,   "[CAM_E] "  fmt, ##__VA_ARGS__)
#define CAM_LOG_WARNING(fmt, ...) LOG_COMMON(WARNING, "[CAM_W] " fmt, ##__VA_ARGS__)
#define CAM_LOG_DEBUG(fmt, ...)   LOG_COMMON(DEBUG,   "[CAM_D] " fmt, ##__VA_ARGS__)

#define PROC_LOG_ERROR(fmt, ...)   LOG_COMMON(ERROR,   "[PROC_E] " fmt, ##__VA_ARGS__)
#define PROC_LOG_WARNING(fmt, ...) LOG_COMMON(WARNING, "[PROC_W] " fmt, ##__VA_ARGS__)
#define PROC_LOG_DEBUG(fmt, ...)   LOG_COMMON(DEBUG,   "[PROC_D] " fmt, ##__VA_ARGS__)

#define SERVICE_LOG_ERROR(fmt, ...)   LOG_COMMON(ERROR,   "[SERVICE_E] " fmt, ##__VA_ARGS__)
#define SERVICE_LOG_WARNING(fmt, ...) LOG_COMMON(WARNING, "[SERVICE_W] " fmt, ##__VA_ARGS__)
#define SERVICE_LOG_DEBUG(fmt, ...)   LOG_COMMON(DEBUG,   "[SERVICE_D] " fmt, ##__VA_ARGS__)

int main() {
    LOG_SET_LEVEL(DEBUG);

    CAM_LOG_ERROR("the fd(%d) can't be opened!", 5);
    CAM_LOG_WARNING("this is a warning from %s !", "message processing thread");
    PROC_LOG_ERROR("this is a warning from %s !", "message processing thread");
    PROC_LOG_DEBUG("this message should not be seen");
    SERVICE_LOG_WARNING("failed to run function");
    return 0;
}

```

Answer 1

printf() and friends are thread-safe

While originally the C standard did not mention threads or thread-safety at all, most standard library implementations already ensured that printf() was thread-safe. But since C11 it is actually defined in the standard to be thread-safe.

C++'s iostreams are also thread-safe.

The only thing you have to worry about is if you use multiple print statements to print a single line; in that case it could be that calls from multiple threads get interleaved, resulting in the text appearing in an order that is not desirable. Since C++20 there is std::osyncstream to help with that.

Answer 2

Support Logging to any std::ostream

Currently, you only log to stdout from the C standard library. It would be better to tell the logger which file or stream to output to when it is constructed, defaulting to std::cerr or perhaps std::clog. You can instead construct a logger that logs to a file, by passing it a std::ofstream&.

Use C++ Style I/O, or std::format

Currently, the interface is a C-style format string and variadic arguments, but without the checks the compiler does for correctness. This is obsolete and finicky.

If you accept a std::string_view, you can pass in any string-like object, including the return value of std::format. You probably want an additional overload for const char*, to cover all your bases. It also guarantees that each log entry is a single output call, which (as G. Sliepen reminded me), is already thread-safe.

Alternatively, you can overload operator<< to accept any type that is overloaded to output to an ostream&.

Use std::source_location Instead of C-style Macros

A better solution than a C-style macro for passing the logger the source line that calls it is std::source_location. Give the logging function a default final argument const std::source_location srcl = std::source_location::current(). Then, the compiler will default to passing the function the source location of the call site.

Also Replace the Other C-Style Macros

In general, use macros only when you have to. A good solution here would be to replace the macros that do formatting with wrappers that return a std::string object from std::format or std::stringstream, and pass (or maybe move) it to the logger’s output function in a tail-call. The string will automatically be destroyed when the logger is done with it and returns.

Use the Mutex

Having the threads claim a mutex to be able to log to the stream is a great idea, but threads need to actually lock the mutex before doing I/O.

If you are writing to a stream that something other than the log object might write to, it could be worthwhile to pass the constructor a reference to the mutex it should be using. That way, other functions can also lock the same mutex and interoperate with the logger. By default, the logger can still create its own mutex, which only it uses.

Are You Sure About the Private and Deleted Constructors?

There doesn’t appear to be a way to actually create one of these. If it’s meant to be a base class, the private: constructors should be protected:, so daughter classes can implicitly call them, and also should have a protected:, virtual destructor. If this is meant to be instantiated, it needs at least one public: constructor. It probably makes sense for there to be a default constructor as well, since there are sensible defaults.

Consider as well whether you actually want to delete the copy constructor. You say that you want this to be used from multiple threads, and although they might all access it through a std::shared_ptr or reference, you might legitimately want to pass each thread its own copy. That way, the object only needs to serialize the output operation itself, not all access from any of the threads.

Answer 3

Obsolete typedef enum ... Syntax

Only in C, structs/enums/unions inhabited a separate namespace so that you needed to refer to them as struct foo or add a typedef. In C++, that is obsolete.

Looking at the log_level_em enumeration, also note that the constants "leak" into the surrounding namespace, so they end up as globals. That and the fact that they look like PREPROCESSOR_MACROS makes for brittle code. In modern C++ (not sure if it's already in C++11), use enum class instead. At least, use a common prefix to all log levels to distinguish them from other names within the namespace.

Singleton Pattern

Singletons are not much better than plain globals. For that reason, the singleton pattern is typically also considered an antipattern. Prefer modern technologies like dependency inversion (a.k.a. dependency injection).

Variadic Functions

For type safety, I'd much prefer use of templates. You can call the logging function with mismatches between the format string and the arguments supplied to it, which can lead to all kinds of issues.

I'm not sure about C++11, but current C++ also has variadic templates, which kind-of combine the variable number of arguments with the type-safety of templates.

Ineffective Synchronization

You use a lock around the vprintf() call, but you are not the exclusive owner of the stream this writes to. It may prevent multiple logging calls from stepping on each other's feet, but it will not prevent any other code from doing that. In addition, as others noted, that call doesn't need synchronization.

Use Of Preprocessor Function-Likes

LOG_SET_LEVEL really sticks out here as something that doesn't need to be a preprocessor macro at all. The others unfortunately need to be macros, since they need __FILE__ and __LINE__, which can't be helped. However, the three CAM/PROC/SERVICE flavours shouldn't be in this library, they are specific to the code using it.

Controlling Log Level During Compilation

Note: This point is opinionated and/or context-dependent.

I'm missing a way to cleanly disable some or all logging, so it doesn't cause any overhead during runtime. That could be achieved by making the log level a constant, which can be evaluated at compile time. Any of the logging macros could then resolve to nothing. This can be useful in some cases, especially for a development version, as opposed to a production version, which could then also perform additional internal diagonstics. In other cases, you really want a runtime-configurable logging level though, e.g. when you pass --verbose on the commandline.

Use Of Old-Style C Headers

There are cstdio and cstdarg to replate stdio.h and stdarg.h. You then need to use e.g. std::vprintf(), of course.

Answer 4

Setting the block level could create a data race.

Writes to the block level by set_block_level are not synchronized with reads from block_log (and thus indirectly from log_output). If a program sets the log level to a different value while also logging on another thread with no additional synchronization, your program will have undefined behavior.

You can either make all accesses to the block level synchronized (for instance, by making it atomic), or you could change the API to no longer give users the capability to change the log level after the logger instance has been created.