Make source_loc more resilient to invalid data

[serhii-muchychka]

I use spdlog as a logging system for the C++/QML application. The C++ part uses built-in macros for logging: SPDLOG_*. For QML part I redirect standard Qt logger via qInstallMessageHandler function

qInstallMessageHandler([](QtMsgType type, const QMessageLogContext &context, const QString &msg) {
        auto spdLogLevelType = convertQtToSpdLogLevel(type);
        spdlog::source_loc loc{ context.file, context.line, context.function};
        spdlog::log(loc, spdLogLevelType, msg.toStdString());
    });

But different invalid values come from different parts of QML code:

1. context.line can be equal -1
2. context.function can be equal nullptr, while context.file is non-empty string

So as a result, I added workaround to my code:

const char *notNullCString(const char *string)
{
    return string ? string : "";
}
...
spdlog::source_loc loc{ notNullCString(context.file), std::max(context.line, 0), notNullCString(context.function) };

After that everything works fine, no more crashes. But would it be possible to add these checks to the SPDLOG to simplify the integration of other projects?

My suggestions:

1. Treat all source_loc with a zero or negative line as empty (now this is only for zero)
2. Add nullptr checks for filename and funcname so there are no crashes

[tt4g]

It appears to be the same problem #2867

[serhii-muchychka]

Although both of these problems apply to Qt, I would not call them the same. In #2867 is a suggestion to make source_loc more friendly to asynchronous logger by deep copying, here I propose to cover some edge cases (like null strings). I'm not using an async logger

[tt4g]

This is my personal opinion, source_loc is a structure for passing __FILE__ and __LINE__ macros.
So source_loc::empty() checks for the impossible value __LINE__ == 0.
In other words, when __LINE__ ! = 0, it is assumed that there can be no situation where __FILE__ is an invalid pointer.

If QMessageLogContext is incompatible with these macros, it is the user's job to manipulate it so that it can be used in the same way as these macros.

Nevertheless, changing the condition of source_loc::empty() to return line > 0; does not break the ABI, so you may try to submit a PR.

spdlog/include/spdlog/common.h

Line 322 in a2b4262

SPDLOG_CONSTEXPR bool empty() const SPDLOG_NOEXCEPT { return line == 0; }