Fixing code location log level filtering

[aquesnel]

As part of the logging refactoring in #1708 I broke the code location based log level filtering. This pull request fixes the code location log level filtering.

[matt335672]

Some minor comments

[matt335672]

I'm a little concerned with this mega-conditional. It's quite hard to parse mentally (at least it is for me), and there's some repetition in it. Maybe split it up a bit? Here's a suggestion:-

{
    bool_t result;
    /* Is log initialized? */
    if (g_staticLogConfig == NULL)
    {
        result = 0;
    }
    else if (override_destination_level)
    {
        /* Override in place - if the message should be logged, is any
         * destination enabled? */
        if (log_level > override_log_level)
        {
            result = 0;
        }
        else
        {
            result =
                g_staticLogConfig->fd >= 0 ||
                g_staticLogConfig->enable_syslog ||
                g_staticLogConfig->enable_console;
        }
    }
    else
    {
        /* Is there at least one log destination which will accept the
         * message based on the log level? */
        result =
            (g_staticLogConfig->fd >= 0 &&
             log_level <= g_staticLogConfig->log_level) ||
            (g_staticLogConfig->enable_syslog &&
             log_level <= g_staticLogConfig->syslog_level) ||
            (g_staticLogConfig->enable_console &&
             log_level <= g_staticLogConfig->console_level);
    }

    return result;
}

[aquesnel]

cleaned up to be easier to understand.

[aquesnel]

@matt335672 I've implemented the changes you suggested.

Please let me know if more changes are needed before merging.

[metalefty]

Is this should be in v0.9.15?

[aquesnel]

@metalefty Yes I think this pull request should be in v0.9.15 because the log filtering by source location in devel is broken without this pull request.

[matt335672]

I'm happy with either multiple returns in a function, or if / else if with a single return at the end. Here you've mixed styles somewhat. It'll work fine, but I think there's a chance that in the future a static analysis will moan about an unreachable 'else' branch following a return.

I'd suggest either removing the unnecessary else statements, or using a single return - whatever seems more natural to you.

[aquesnel]

@matt335672 I've implemented this feedback using multiple returns by only have a single level of nesting for the if/else. Please let me know if I correctly interpreted your feedback, or if you'd prefer that I use the single return at the end style.

[matt335672]

Sorry - I've probably not been very clear.

My point was that in this kind of construction:-

if (<something>)
{
    return 1;
}
else if (<something else>)
...

the else in bold is superfluous and not required. At some stage I'd expect a static analysis tool to say something about it. Does that make more sense?

The nesting style you use, or whether you use a single return or multiple returns is completely up to you. I've got my own preferences but they're not relevant at all.

[aquesnel]

CR feedback implemented, branch squashed and rebased to devel.

This should be ready to merge once @matt335672 reviews the changes.