How to ideally fix the log function

[John Wang]

On Fri, Jul 26, 2019 at 10:08 AM Andrew Jeffery <andrew at aj.id.au> wrote:
>
>
>
> On Thu, 25 Jul 2019, at 20:52, vishwa wrote:
> > Hi John,
> >
> > Just some background; The goal is not to have func() pass in CODE_LINE
> > and CODE_FUNC since it would be a huge change all over in the code.
>
> There's no way this is really going to work. It turns out sd_journal_print()
> is actually a macro that adds the CODE_* properties via __LINE__ etc when
> SD_JOURNAL_SUPPRESS_LOCATION is not defined[1], so we either get
> the misleading information or force the caller to provide it.
>
> A hack approach that almost immediately falls short is that you could try
> macro over log() and adjust the actual log function prototype to take in the
> location parameters so we can call sd_journal_send_with_location()
> directly. However macro-ing over log won't handle namespaces correctly,
> and log() seems to parameterise the level with a template so we can't
> define the hypothetical log macro as a macro function (which we need to
> do in order to adjust the parameter list).
>
> Having said all that I'm not really a C++ person, but it appears the API has
> trapped itself in a C++ corner. Maybe people more experienced and creative
> than I can come up with something, but I think the best thing we can do
> is three-fold:
>
> 1. Build libsystemd with `CFLAGS=-DSD_JOURNAL_SUPPRESS_LOCATION`
> 2. Add a new log macro that allows us to capture the info properly
Maybe something like below:
```
struct traceLog
{
    traceLog(const source_location location = source_location::current()) :
        location(location)
    {
    }
    template <level L, typename Msg, typename... Entry>
    void log(Msg msg, Entry... e)
    {
        phosphor::logging::log<L>(msg, entry("CODE_LINE=%d", location.line()),
                                  entry("CODE_FILE=%s", location.file_name()),
                                  e...);
    }

  private:
    source_location location;
};

int main()
{
    traceLog{}.log<level::INFO>("xxxxx", entry("entry=%s", "xxxx"));
}
```

> 3. Change callsites to the new macro function over time.
>
> With 1 we remove the misleading information from the journal, and 2 and 3
> get us to the right place, eventually at the expense of CPP macros.
>
> Andrew
>
> [1] https://github.com/systemd/systemd/blob/master/src/systemd/sd-journal.h#L53