6
\$\begingroup\$

Some things I want to achieve with the logging code.

  • Code is for an embedded device. Code must only work for Linux. C++11 can be used.
  • Syslog daemon is running on the device. So I want to wrap syslog.h and let syslog take care of the actual logging.
  • Get better checking at compile time. I added enums for this. The logging code may never crash and should not throw exceptions at runtime.
  • The code is split into modules. I want named loggers for each module so that I can set different severity for different modules and change them at runtime.
  • Define the severity of a logger in a config file. The logger should be defined in the code, so you can't create a new logger with the config file. I added some init stuff for this, but this is really simple and requires more work.
  • The idea is to get severity of a logger in O(1) by using the enum value as index, So not loop over a array or a vector etc.
  • When the severity is not ok, no string formatting should take place. That is why I put the if around the logging statement with a macro. Example: log(complexObject.to_string()) if you check the severity inside the log function you waste cpu on to_string() to then do nothing with it.
  • One thing thing I don't really like is the use of MAX_INDEX_Logger. Not using it required lots of macros which frankly I don't understand. But I might give it a try...
  • Another thing I don't like is the use of the macro ADD_LOGGER, but it is kind of required for now, because sooner rather then later I will put a logger at a wrong index and the data structures wont match the enum.
  • This is C++ code but I don't see any benefit of creating a logger object or something similar? I played with this but it made everything more complex with no real benefits. But I could be wrong.

Please be gentle; this is a first draft of the code XD.

Slog.hpp

#ifndef SLOG_ENUMS_HPP
#define SLOG_ENUMS_HPP

#include <string>
#include <syslog.h>

namespace slog
{
  // Make enums to guarantee typesafety.

  /*
   * Enum to define the available loggers.
   * IF YOU ADD A NEW LOGGER ALSO ADD IT TO THE initLoggerConversionStructures
   * FUNCTION WITH THE ADD_LOGGER MACRO IN SLOG.CPP FILE!!
   */
  enum Logger
  {
    UNKNOWN,
    TEST,
    MAIN,
    LOG,
    MODULE1,
    MODULE2,
    MODULE3,
    MODULE4,
    MODULE5,
    MODULE6,
    MODULE7,
    MODULE8,
    MAX_INDEX_Logger //Add new entries before this one
  };

  // Severity levels are mapped to those in syslog.h
  enum Severity
  {
    EMERG = LOG_EMERG,
    ALERT = LOG_ALERT,
    CRIT = LOG_CRIT,
    ERR = LOG_ERR,
    WARNING = LOG_WARNING,
    NOTICE = LOG_NOTICE,
    INFO = LOG_INFO,
    DEBUG = LOG_DEBUG
  };

  // Facilities are mapped to those in syslog.h
  enum Facility
  {
    KERN = LOG_KERN,
    USER = LOG_USER,
    MAIL = LOG_MAIL,
    DAEMON = LOG_DAEMON,
    AUTH = LOG_AUTH,
    CRON = LOG_CRON,
    AUTHPRIV = LOG_AUTHPRIV,
    LOCAL0 = LOG_LOCAL0,
    LOCAL1 = LOG_LOCAL1,
    LOCAL2 = LOG_LOCAL2,
    LOCAL3 = LOG_LOCAL3,
    LOCAL4 = LOG_LOCAL4,
    LOCAL5 = LOG_LOCAL5,
    LOCAL6 = LOG_LOCAL6,
    LOCAL7 = LOG_LOCAL7,
  };

  /*
   * Init logging framework.
   * You need to call this in the main thread before anything else to configure
   * the logger conversion structures and initialze the severity for each logger.
   */
  void init(const std::string& configFile, Facility facility = Facility::LOCAL0);

  /*
   * Call to openlog from syslog.h with some default arguments.
   * This function can be used to change the facility.
   */
  void openSyslog(Facility facility = Facility::LOCAL0);

  /*
   * Translate string to logger enum.
   * This function is case sensitive.
   * Returns UNKNOWN by default.
   */
  Logger strToLogger(const std::string& logger);

  // Translate logger enum to string.
  std::string loggerToStr(Logger logger);

  /*
   * Translate string to severity enum.
   * This function is case sensitive.
   * Returns EMERG by default.
   */
  Severity strToSeverity(const std::string& severityStr);

  // Translate severity enum to string.
  std::string severityToStr(Severity severity);

  // Return severity for specified logger.
  Severity getSeverity(Logger logger);

  // Set severity for specified logger.
  void setSeverity(Logger logger, Severity severity);

  // Return if the logger should log for given severity.
  bool isLoggerEnabled(Logger logger, Severity severity);

  // Print loggers for debugging purpose.
  void printLoggers();

  /*
   * Generic logging function. Should not be used directly.
   * Is called through Macros.
   */
  template<typename... Args>
  void __log(const char* file, const char* func, uint32_t line, Logger logger, Severity severity, const char* fmt, Args... args)
  {
      std::string msg = loggerToStr(logger).append(": [%s:%s:%i] ").append(fmt);
      ::syslog(severity, msg.c_str(), file, func, line, args...);
  };
} // namespace slog

/*
 * Determine the filename without full path.
 * Because it is constexpr with __FILE__ this is determined at compile time.
 */
static constexpr const char* past_last_slash(const char* str, const char* last_slash)
{
    return
        *str == '\0' ? last_slash :
        *str == '/'  ? past_last_slash(str + 1, str + 1) :
                       past_last_slash(str + 1, last_slash);
}

static constexpr const char* past_last_slash(const char* str)
{
    return past_last_slash(str, str);
}

/*
 * Put it in a nice macro that creates a temp function. this forces the compiler to optimize the
 * funtion away else you might get a function returning a constant.
 */
#define __SHORT_FILE__ ({constexpr const char* sf__ {past_last_slash(__FILE__)}; sf__;})

//Macro to add filename, function and linenumber.
#define SLOG(logger, severity, fmt, ...) \
  slog::__log(__SHORT_FILE__, __FUNCTION__, __LINE__, logger, severity, fmt, ##__VA_ARGS__)

/*
 * These macro's add an if statement.
 * This is very important for conditional formatting.
 * Don't waste cpu cycles on string formatting if the logging level is not correct.
 */
#define SLOG_EMERG(logger, fmt, ...) \
    if(slog::isLoggerEnabled(logger, slog::Severity::EMERG)) \
      SLOG(logger, slog::Severity::EMERG, fmt, ##__VA_ARGS__)

#define SLOG_ALERT(logger, fmt, ...) \
    if(slog::isLoggerEnabled(logger, slog::Severity::ALERT)) \
      SLOG(logger, slog::Severity::ALERT, fmt, ##__VA_ARGS__)

#define SLOG_CRIT(logger, fmt, ...) \
    if(slog::isLoggerEnabled(logger, slog::Severity::CRIT)) \
      SLOG(logger, slog::Severity::CRIT, fmt, ##__VA_ARGS__)

#define SLOG_ERR(logger, fmt, ...) \
    if(slog::isLoggerEnabled(logger, slog::Severity::ERR)) \
      SLOG(logger, slog::Severity::ERR, fmt, ##__VA_ARGS__)

#define SLOG_WARNING(logger, fmt, ...) \
    if(slog::isLoggerEnabled(logger, slog::Severity::WARNING)) \
      SLOG(logger, slog::Severity::WARNING, fmt, ##__VA_ARGS__)

#define SLOG_NOTICE(logger, fmt, ...) \
    if(slog::isLoggerEnabled(logger, slog::Severity::NOTICE)) \
      SLOG(logger, slog::Severity::NOTICE, fmt, ##__VA_ARGS__)

#define SLOG_INFO(logger, fmt, ...) \
    if(slog::isLoggerEnabled(logger, slog::Severity::INFO)) \
      SLOG(logger, slog::Severity::INFO, fmt, ##__VA_ARGS__)

#define SLOG_DEBUG(logger, fmt, ...) \
    if(slog::isLoggerEnabled(logger, slog::Severity::DEBUG)) \
      SLOG(logger, slog::Severity::DEBUG, fmt, ##__VA_ARGS__)

#endif // SLOG_ENUMS_HPP

Slog.cpp

#include <string>
#include <fstream>
#include <unordered_map>
#include <array>
#include <algorithm>
#include <syslog.h>

#include "Slog.hpp"

/*
 * Array to translate from severity enum to string. Indexed on severity enum.
 * Severity levels don't change so this is declared const.
 */
static const std::array<std::string, 8> severityToStrArray_ =
{
  "EMERG",
  "ALERT",
  "CRIT",
  "ERR",
  "WARNING",
  "NOTICE",
  "INFO",
  "DEBUG"
};


/*
 * Map to translate from string to severity enum. Indexed on string.
 * Severity levels don't change so this is declared const.
 */
static const std::unordered_map<std::string, slog::Severity> strToSeverityMap_ =
{
  {"EMERG", slog::Severity::EMERG},
  {"ALERT", slog::Severity::ALERT},
  {"CRIT", slog::Severity::CRIT},
  {"ERR", slog::Severity::ERR},
  {"WARNING", slog::Severity::WARNING},
  {"NOTICE", slog::Severity::NOTICE},
  {"INFO", slog::Severity::INFO},
  {"DEBUG", slog::Severity::DEBUG},
};


/*
 * Array to translate from logger enum to string. Indexed on logger enum.
 * This array is filled by the ADD_LOGGER Macro in the initLoggerConversionStructures()
 * function, to avoid mismatches between enum and it's string representation.
 */
static std::array<std::string, slog::Logger::MAX_INDEX_Logger> loggertoStrArray_;


/*
 * Map to translate from string to logger enum. Indexed on string.
 * This map is filled by the ADD_LOGGER Macro in the initLoggerConversionStructures()
 * function, to avoid mismatches between enum and it's string representation.
 */
static std::unordered_map<std::string, slog::Logger> strToLoggerMap_;


/*
 * Macro to help fill the logger enum conversion structures.
 * Avoid mismatches between enum and it's string representation.
 */
#define ADD_LOGGER(x) \
  loggertoStrArray_[slog::Logger::x] = #x; \
  strToLoggerMap_[#x] = slog::Logger::x;


/*
 * IF YOU DEFINE A NEW LOGGER. YOU MUST ADD THE LOGGER TO THIS FUNCTION!
 * Function to fill the logger enum conversion structures.
 * Must be called as part of the init process.
 */
static void initLoggerConversionStructures()
{
  ADD_LOGGER(UNKNOWN);
  ADD_LOGGER(TEST);
  ADD_LOGGER(MAIN);
  ADD_LOGGER(MODULE1);
  ADD_LOGGER(MODULE2);
  ADD_LOGGER(MODULE3);
  ADD_LOGGER(MODULE4);
  ADD_LOGGER(MODULE5);
  ADD_LOGGER(MODULE6);
  ADD_LOGGER(MODULE7);
  ADD_LOGGER(MODULE8);
  ADD_LOGGER(LOG);
}


/*
 * Array to map a severity to a logger. Indexed on logger enum.
 */
static std::array<slog::Severity, slog::Logger::MAX_INDEX_Logger> loggerSeverityArray_;


void slog::init(const std::string& configFile, slog::Facility facility)
{
  initLoggerConversionStructures();
  openSyslog(facility);

  setSeverity(Logger::LOG, Severity::INFO);

  std::ifstream file{configFile};
  if(file.good())
  {
    SLOG_INFO(Logger::LOG, "Loading config file: %s", configFile.c_str());

    std::string line;
    while(getline(file, line))
    {
      line.erase(std::remove_if(line.begin(), line.end(), [](char x){return std::isspace(x);}), line.end());
      std::size_t i = line.find(":");
      if(i != std::string::npos)
      {
        std::string logger = line.substr(0,i);
        Logger l = slog::strToLogger(logger);
        if(l != Logger::UNKNOWN)
        {
          std::string severity = line.substr(i+1, std::string::npos);
          Severity s = strToSeverity(severity);
          loggerSeverityArray_[l] = s;
        }
      }
      else
      {
        SLOG_ERR(Logger::LOG, "Invalid line in config file: %s", line.c_str());
      }
    }
  }
  else
  {
    SLOG_EMERG(Logger::LOG, "Could not open config file: %s", configFile.c_str());
  }

  if(isLoggerEnabled(Logger::LOG, Severity::DEBUG))
  {
    for(int lInt=0; lInt<Logger::MAX_INDEX_Logger; lInt++)
    {
      Logger l = static_cast<Logger>(lInt);
      SLOG(Logger::LOG, Severity::DEBUG,
        "Logger %s : %s", loggerToStr(l).c_str(), severityToStr(getSeverity(l)).c_str());
    }
  }
}


void slog::openSyslog(slog::Facility facility)
{
  ::openlog(nullptr, LOG_CONS | LOG_NDELAY | LOG_PID, facility);
}


slog::Severity slog::getSeverity(slog::Logger logger)
{
  if (logger != Logger::MAX_INDEX_Logger)
  {
    return loggerSeverityArray_[logger];
  }
  else
  {
    return Severity::EMERG;
  }
}


void slog::setSeverity(slog::Logger logger, slog::Severity severity)
{
  if (logger != Logger::MAX_INDEX_Logger)
  {
    loggerSeverityArray_[logger] = severity;
  }
}


bool slog::isLoggerEnabled(slog::Logger logger, slog::Severity severity){
  if (logger != Logger::MAX_INDEX_Logger)
  {
    return (loggerSeverityArray_[logger] >= severity);
  }
  else
  {
    return false;
  }
}


slog::Severity slog::strToSeverity(const std::string& severity)
{
  try
  {
    return strToSeverityMap_.at(severity);
  }
  catch(std::out_of_range e)
  {
    SLOG_ERR(Logger::LOG, "Severity %s does not exist", severity.c_str());
    return Severity::EMERG;
  }
}


std::string slog::severityToStr(slog::Severity severity)
{
  return severityToStrArray_[severity];
}


slog::Logger slog::strToLogger(const std::string& logger)
{
  try
  {
    return strToLoggerMap_.at(logger);
  }
  catch(std::out_of_range e)
  {
    SLOG_ERR(Logger::LOG, "Logger %s does not exist", logger.c_str());
    return Logger::UNKNOWN;
  }
}


std::string slog::loggerToStr(slog::Logger logger)
{
  if (logger != Logger::MAX_INDEX_Logger)
  {
    return loggertoStrArray_[logger];
  }
  else
  {
    return "UNKNOWN";
  }
}

void slog::printLoggers()
{
    for(int lInt=0; lInt<Logger::MAX_INDEX_Logger; lInt++)
    {
      Logger l = static_cast<Logger>(lInt);
      printf("Logger %s: %s", loggerToStr(l).c_str(), severityToStr(getSeverity(l)).c_str());
    }

}

Some notes about the constexpr recursion

Constexpr is calculated at compile time.

static constexpr const char* past_last_slash(const char* str, const char* last_slash)
{
    return
        *str == '\0' ? last_slash :
        *str == '/'  ? past_last_slash(str + 1, str + 1) :
                       past_last_slash(str + 1, last_slash);
}

static constexpr const char* past_last_slash(const char* str)
{
    return past_last_slash(str, str);
}

#define __SHORT_FILE__ ({constexpr const char* sf__ {past_last_slash(__FILE__)}; sf__;})

main()
{
    printf("%s", __SHORT_FILE__);
}

This gives following assembler

.LC0:
  .string "/tmp/compiler-explorer-compiler118024-63-codig2.fpj1v/example.cpp"
.LC1:
  .string "%s"
main:
  push rbp
  mov rbp, rsp
  sub rsp, 16
  mov QWORD PTR [rbp-8], OFFSET FLAT:.LC0+54
  mov eax, OFFSET FLAT:.LC0+54
  mov rsi, rax
  mov edi, OFFSET FLAT:.LC1
  mov eax, 0
  call printf
  mov eax, 0
  leave
  ret
__static_initialization_and_destruction_0(int, int):
  push rbp
  mov rbp, rsp
  sub rsp, 16
  mov DWORD PTR [rbp-4], edi
  mov DWORD PTR [rbp-8], esi
  cmp DWORD PTR [rbp-4], 1
  jne .L5
  cmp DWORD PTR [rbp-8], 65535
  jne .L5
  mov edi, OFFSET FLAT:std::__ioinit
  call std::ios_base::Init::Init()
  mov edx, OFFSET FLAT:__dso_handle
  mov esi, OFFSET FLAT:std::__ioinit
  mov edi, OFFSET FLAT:std::ios_base::Init::~Init()
  call __cxa_atexit
.L5:
  nop
  leave
  ret
_GLOBAL__sub_I_main:
  push rbp
  mov rbp, rsp
  mov esi, 65535
  mov edi, 1
  call __static_initialization_and_destruction_0(int, int)
  pop rbp
  ret

So at runtime on the device there is no recursion.

You can't use std::string with constexpr; see Is it possible to use std::string in a constexpr?

\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

Recursion in an Embedded Environment
This function static constexpr const char* past_last_slash(const char* str, const char* last_slash) is a recursive implementation. Recursion is a memory hog and in an embedded system you may be very limited as to the memory you have. Instead of using C type strings, why not use std::string? The C++ string type std::string has a find_last member that will perform at least half of this function.

Macros in C++
It appears that you are more comfortable writing C rather than C++. While macros are supported for backwards compatibility with the C programming language, the use of macros is strongly discouraged because they aren't type safe. There are many alternatives to the use of macros. For defining constants there are const and constexpr; for functions that handle many different types there are templates. In C++ there is no good reason to use macros.

Missing Header File The standard library function std::isspace(int ch) requires the header file

#include <cctype>

line.erase(std::remove_if(line.begin(), line.end(), [](unsigned char x) {return std::isspace(x); }), line.end());

Enum Conversions
It's not clear why the Facility enum and the Severity enum are created. The implementation in syslog.h is pretty clear.

If you absolutely have to convert from the macros in <syslog.h> then you might consider using std::map<int, OTHER_TYPE> which is another of the C++ container types. The std::map<type, type> container class can be used to map integers to strings, integers to other integers, and integers to functions. It can also map other things to each other.

The values in <syslog.h> have been designed so that a single byte or word can be used to test whether they are enabled or not. This is a space-saving feature that one needs when doing embedded programming.

static unsigned char enabledMask = 0;

static void slog::setEnable(int severity)
{
    enabledMask |= (0x01 << severity);
}

static void slog::disable(int severity)
{
    unsigned char disableMask = ~(0x01 << severity);
    enabledMask &= disableMask;
}

static bool slog::isEnabled(int severity)
{
    return (enabledMask & (0x01 << severity)) != 0;
}

static const std::map<int, std::string> mapseverityToStr =
{
    { LOG_EMERG, "EMERG" },
    { LOG_ALERT,        "ALERT" },
    { LOG_CRIT, "CRIT" },
    { LOG_ERR, "ERR" },
    { LOG_WARNING, "WARNING" },
    { LOG_NOTICE, "NOTICE" },
    { LOG_INFO, "INFO" },
    { LOG_DEBUG, "DEBUG" }
};

static void slog::printSeverity(int severity)
{
    std::string severityStr;

    auto selectString = mapseverityToStr.find(severity);
    if (selectString != mapseverityToStr.end())
    {
        severityStr = selectString->second;
        printf("%s", severityStr.c_str());
    }
    else
    {
        printf("UNKNOW")
    }

}

Classes
Using a class might have been simpler than creating a namespace. The namespace as implemented is pretty much a class. The class will provide a namespace for all its members. Variables defined in the class won't need to be declared static because they are within the class.

class sLogWrapper
{
private:
    unsigned char enabledMask = 0;
    std::map<int, std::string> mapseverityToStr;

public:
    sLogWrapper(std::string fileName);
    ~sLogWrapper();
    void printSeverity(int severity);
    void setEnable(int severity);
    void disable(int severity);
    bool isEnabled(int severity);

}

sLogWrapper slog(fileName);

if (slog.isEnabled(severity))
{
    ...
}

The best reason not to use classes is due to lack of memory resources. When programming in an embedded system, the software engineer is responsible for tracking memory usage and that may be difficult when using classes.

\$\endgroup\$
9
  • 1
    \$\begingroup\$ 1. Code is cross-compiled. Memory at compile time is not a problem. 2. Enums are used safety. Passing ints you constantly need to check the range, enums solve this very easily. \$\endgroup\$
    – rinn2883
    Jan 9, 2018 at 8:03
  • 1
    \$\begingroup\$ 3. In the example for using a class you provide you have 1 slog object. I Want named loggers for different modules, not just 1 slog object. I have tried before with creating logger objects, but you have to start passing them around or storing them on the heap if you want to change severity at runtime. Like I stated it becomes more complex for no real benefit. My loggers are in an enum, I use this enum value to index in an array to get the severity. \$\endgroup\$
    – rinn2883
    Jan 9, 2018 at 8:12
  • \$\begingroup\$ I use a macro because I need to put code around and in the log function. I am not the one writing all the modules, so I dont want devs to need to write if(isLoggerEnabled(...)), FUNCTION and LINE everywhere. A macro keeps it nice an tidy in the code. As far as I know a macro is the only way to get the if around the logging statement instead of inside. I explained why I do this. Don't get me wrong I am not a macro fanatic, but as far as I know I can't solve these problems with templates and constexpr. \$\endgroup\$
    – rinn2883
    Jan 9, 2018 at 8:18
  • 1
    \$\begingroup\$ blog.galowicz.de/2016/02/20/short_file_macro This is the blog for the SHORT_FILE macro \$\endgroup\$
    – rinn2883
    Jan 9, 2018 at 8:48
  • \$\begingroup\$ @rinn2883 All my concerns about memory usage are at execution time and not at compile time. \$\endgroup\$
    – pacmaninbw
    Jan 9, 2018 at 14:42
0
\$\begingroup\$

I have too small reputation to post comment, but I want to pay attention that the code from topic starter is buggy! You have to make a correction before using it. Try simple example and you will see a problem:

if (1)
   SLOG_ERR(logger...)
else
{
   std::cout << "BUG"!
}

You will see that if the logger not enabled, you will see BUG in the console.

\$\endgroup\$
3
  • \$\begingroup\$ Your answer could be improved with additional supporting information. Please edit to add further details, such as citations or documentation, so that others can confirm that your answer is correct. You can find more information on how to write good answers in the help center. \$\endgroup\$
    – Community Bot
    Sep 19 at 5:39
  • \$\begingroup\$ This does not provide an answer to the question. Once you have sufficient reputation you will be able to comment on any post; instead, provide answers that don't require clarification from the asker. - From Review \$\endgroup\$ Sep 19 at 5:39
  • \$\begingroup\$ @user16217248 Keep in mind that answers on Code Review must contain observations about the code. This answer does make an observation about the code. \$\endgroup\$
    – pacmaninbw
    Sep 19 at 12:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.