skip to Main Content

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("Hellon");

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

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

  return 0;
}

2

Answers


  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 blan", 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());
    
    Login or Signup to reply.
  2. 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 << "Hellon";
    
        logger<console_log_t> B;
        B << "COUTn";
    
        logger<syslog_log_t<0>> C;
        C << "SYSLOGn";
    }
    

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

    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search