1

I am trying to create a composite logger which will be selected and defined at compile time. The original implementation is using inheritance and VTable, which is causing some overhead, takes too much code and is ineffective at runtime. This is supposed to be simplification, which accepts basically any stream defined in C++ and then reference to syslog() function, checks types and then let's compiler generate the appropriate machine code.

It works at it is supposed to, however I'd like to ask about this warning:

$ g++ -std=c++17 template_types.cpp -o asd
template_types.cpp: In function ‘int main(int, char**)’:
template_types.cpp:44:41: warning: ignoring attributes on template argument ‘void (&)(int, const char*, ...)’ [-Wignored-attributes]
44 |   logger<syslog_log_t, decltype(syslog)&> C(syslog);

If I understand it correctly, logger template should take type of the logger as first template type and the logging channel type as second template type. This allows me to move and reference the channel in the F file variable. It can hold file descriptor in the form of std::ofstream, as well as reference std::cout or any other stream as well as it can work with function syslog. However the compiler g++ (Debian 9.3.0-18) 9.3.0 thinks that there is something wrong with the code. Could you please tell me, why is it thinking so? Thank you.

/* template_types.cpp */
#include <iostream>
#include <fstream>
#include <syslog.h>

/* types of logger, just for semantics */
using syslog_log_t = struct syslog_type{};
using console_log_t = struct console_type{};
using file_log_t = struct file_type{}; 

/* template for composite logger */
template <typename T, typename F>
class logger {
private:
  F file;
public:
  /* perfect forward the channel for logging as rvalue, can move and reference */
  logger(F &&out) : file(std::forward<F>(out)) {};
  /* if the logging channel is file, close it on end */
  ~logger() {
    if constexpr (std::is_same_v<T, file_log_t>) {
      file.close();
    }
  }
  /* log, allows using stream and syslog call */
  void log(const std::string &msg, int log_level = 0) {
    if constexpr (std::is_same_v<T, syslog_log_t>) {
      file(log_level, msg.c_str());
    }
    else {
      file << msg;
    }
  }
};

int main(int argc, char *argv[]) {
  //logger<file_log_t, std::ofstream> A(std::ofstream("text.txt", std::ios_base::out | std::ios_base::app));
  std::ofstream file("text.txt", std::ios_base::out | std::ios_base::app);
  logger<file_log_t, decltype(file)> A(std::move(file));
  A.log("Hello\n");

  logger<console_log_t, decltype(std::cout)&> B(std::cout);
  B.log("COUT\n");

  logger<syslog_log_t, decltype(syslog)&> C(syslog);
  C.log("SYSLOG\n", LOG_DEBUG);

  return 0;
}
Botje
  • 26,269
  • 3
  • 31
  • 41
Jirka Zahradnik
  • 91
  • 1
  • 7
  • Related: [How to use GCC's printf format attribute with C++11 variadic templates?](https://stackoverflow.com/questions/12573968/how-to-use-gccs-printf-format-attribute-with-c11-variadic-templates) but don't get expectations too high, the answer there is that you can't, that is - you can make your code work, as you did, but the attribute for checking the va_list parameter (in _syslog_ in your case) would not work. – Amir Kirsh Oct 09 '20 at 13:45
  • @AmirKirsh Thank you. – Jirka Zahradnik Oct 09 '20 at 13:53

2 Answers2

1

If you check your system header files for the declaration of the syslog function, you will see something akin to the following:

extern void syslog (int __pri, const char *__fmt, ...)
     __attribute__ ((__format__ (__printf__, 2, 3)));

Under normal usage, this attribute serves to match the format string against the argument for you.
Your compiler is telling you it will not be able to perform these checks in your templated version.

A secondary problem is that your current code is basically accepting a format string without performing any validation! You can thus expect lots of fireworks (or at least undefined behavior) if you ever do C.log("some text %s bla bla\n", LOG_DEBUG); and your compiler will not be able to warn you.

You should rewrite the syslog branch of your log method as follows, this is safe to do as the actual format string is "%s" and the log string is a simple argument to it:

file(log_level, "%s", msg.c_str());
Botje
  • 26,269
  • 3
  • 31
  • 41
1

Instead of putting too much in the logger class, you could put the actual log implementation in what is now your tag classes and use them using the Curiously recurring template pattern.

With this your logger can be used by anyone you wants to plug-in a new device to your logging class. Just implement a log function and it's ready.

It could look something like this:

#include <syslog.h>

#include <fstream>
#include <iostream>
#include <sstream>

/* types of logger, now including implementations */

template<int prio>
struct syslog_log_t {
    static void log(const std::string& msg) {
        ::syslog(prio, "%s", msg.c_str());
    }
};

struct console_log_t {
    static void log(const std::string& msg) { std::cout << msg; }
};

struct file_log_t {
    // this can be instantiated just like any ofstream
    template<typename... Args>
    file_log_t(Args&&... f) : file(std::forward<Args>(f)...) {}

    // non-static here since we own an ofstream instance
    void log(const std::string& msg) { file << msg; }

private:
    std::ofstream file; // no dtor necessary - it'll close automatically
};

template<typename T>
class logger : public T {
public:
    template<typename... Args>
    explicit logger(Args&&... args) : T(std::forward<Args>(args)...) {}
};

// a common front to support streaming
template<typename T, typename U>
logger<T>& operator<<(logger<T>& l, U&& u) {
    std::stringstream ss;
    ss << std::forward<U>(u);
    l.log(ss.str());
    return l;
}

int main() {
    logger<file_log_t> A("text.txt", std::ios_base::app);
    A << "Hello\n";

    logger<console_log_t> B;
    B << "COUT\n";

    logger<syslog_log_t<0>> C;
    C << "SYSLOG\n";
}

Bonus: This has the benefit of not giving the warning you got - but I didn't really dig into that.

Ted Lyngmo
  • 93,841
  • 5
  • 60
  • 108