Basic Linux daemon

Question

I'm looking at writing a simple proxy. At the moment I've got a logger, command line argument parse and simple daemon (it does nothing beyond logging).

Next I'll be looking at starting the network-related work, but before that I wanted to get a review to see if there's anything wrong with my basic approach at the moment.

I've spent most of my time recently working in Java/bash/PHP so want to know if there's anything about my approach that "smells" wrong.

proxy.cpp

#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>

#include <boost/filesystem.hpp>

#include <commandLineOpts.hpp>
#include <loggerConfig.hpp>
#include <spdlog/spdlog.h>

namespace fs = boost::filesystem;

int main(int argc, char *argv[]) {
    CommandLineOpts commandLineOpts;
    commandLineOpts.parseOpts(argc, argv);

    if (commandLineOpts.contains(CommandLineOpts::Help)) {
        commandLineOpts.showHelp();
        return EXIT_SUCCESS;
    }

    std::string configFile = commandLineOpts.getConfig();
    fs::path absConfigPath = fs::absolute(fs::path(configFile));
    if (!exists(absConfigPath)) {
        std::cerr << "Configuration file " << absConfigPath << " does not exist." << std::endl << std::endl;
        commandLineOpts.showHelp();
        return EXIT_FAILURE;
    }

    pid_t pid, sid;    
    pid = fork();

    if (pid < 0) {
        std::cerr << "Failed to fork, error code [" << pid << "]. Exitting";
        return EXIT_FAILURE;
    } else if(pid > 0) {
        return EXIT_SUCCESS;
    }

    umask(0);

    /*
     * Create a new signature ID for the child
     */
    sid = setsid();
    if (sid < 0) {
        std::cerr << "Failed to setsid, error code [" << sid << "]. Exiting";
        return EXIT_FAILURE;
    }

    if ((chdir("/")) < 0) {
        std::cerr << "Failed to change directory to /. Exiting";
        return EXIT_FAILURE;
    }

    LoggerConfig logConfig(absConfigPath.string());
    logConfig.setup();
    auto logger = spdlog::get("async_file_logger");

    close(STDIN_FILENO);
    close(STDOUT_FILENO);
    close(STDERR_FILENO);

    while (true) {
        logger->debug() << "TODO:";
        sleep(60);
    }

    return EXIT_SUCCESS;
}

commandLineOpts.hpp

#ifndef COMMAND_LINE_OPTS_H
#define COMMAND_LINE_OPTS_H

#include <boost/program_options.hpp>

class CommandLineOpts {
    private:
        boost::program_options::variables_map commandLineMap;
        std::string appName;

    public:
        void parseOpts(int argc, char **argv);

        bool contains(std::string argName);
        void showHelp();
        std::string getConfig();

        static std::string Help;
        static std::string Config;
};

#endif

commandLineOpts.cpp

#include <boost/program_options.hpp>
#include <stdexcept>

#include <commandLineOpts.hpp>

namespace bpo = boost::program_options;

std::string CommandLineOpts::Help = "help";
std::string CommandLineOpts::Config = "config";

void CommandLineOpts::parseOpts(int argc, char **argv) {
    appName = argv[0];

    try {
        bpo::options_description desc("Allowed options");
        desc.add_options()
            ("help,h", "Display help")
            ("config,c", bpo::value<std::string>(), "Set config file");

        bpo::store(bpo::parse_command_line(argc, argv, desc), commandLineMap);
        bpo::notify(commandLineMap);
    } catch(std::exception& e) {
        throw new std::runtime_error(e.what());
    } catch(...) {
        throw new std::runtime_error("Failed to parse command line arguments, exception of unknown type!");
    }
}

bool CommandLineOpts::contains(std::string argName) {
    return commandLineMap.count(argName);
}

void CommandLineOpts::showHelp() {
    std::cout << "Usage " << appName << " [OPTION]" << std::endl
        << "-h, --help\t\tDisplay this usage" << std::endl
        << "-c, --config\t\tSpecify the configuration file to use" << std::endl;
}

std::string CommandLineOpts::getConfig() {
    if (contains(CommandLineOpts::Config)) {
        return commandLineMap[CommandLineOpts::Config].as<std::string>();
    } else {
        return "app.cfg";
    }
}

loggerConfig.hpp

#ifndef LOGGER_CONFIG_H
#define LOGGER_CONFIG_H

#include <map>
#include <string>

class LoggerConfig {

    private:
        std::string configFile;
        std::map<std::string, std::string> configMap;

        void loadConfig();
        void setDefaults();

        std::string get(std::string);

        std::string path(void);
    public:
        //! Constructor -- string path to the config file
        LoggerConfig(std::string cfg);

        void dump();

        void setup();

        static std::string LogPath;
        static std::string LogName;
        static std::string LogLevel;
        static std::string LogConfigName;
};

#endif

loggerConfig.cpp

#include <stdexcept>
#include <libconfig.h++>

#include <loggerConfig.hpp>
#include <spdlog/spdlog.h>

using namespace libconfig;

std::string LoggerConfig::LogConfigName = "log";
std::string LoggerConfig::LogPath = "LogPath";
std::string LoggerConfig::LogName = "LogName";
std::string LoggerConfig::LogLevel = "LogLevel";

LoggerConfig::LoggerConfig(std::string config) : configFile(config) {
    setDefaults();
    loadConfig();
}

void LoggerConfig::setDefaults() {
    configMap[LoggerConfig::LogPath] = "./log";
    configMap[LoggerConfig::LogName] = "proxy";
    configMap[LoggerConfig::LogLevel] = "info";
}

void LoggerConfig::loadConfig() {
    Config cfg;

    try {
        cfg.readFile(configFile.c_str());
    } catch (const FileIOException &fioex) {
        return;
    } catch (const ParseException &pex) {
        return;
    }

    const Setting &root = cfg.getRoot();
    try {
        const Setting &log = root[LoggerConfig::LogConfigName];
        std::string temp;

        try {
            log.lookupValue(LoggerConfig::LogPath.c_str(), temp);
            configMap[LoggerConfig::LogPath] = temp;
        } catch (const SettingNotFoundException &nfex) { }


        try {
            log.lookupValue(LoggerConfig::LogName.c_str(), temp);
            configMap[LoggerConfig::LogName] = temp;
        } catch (const SettingNotFoundException &nfex) { }


        try {
            log.lookupValue(LoggerConfig::LogLevel.c_str(), temp);
            configMap[LoggerConfig::LogLevel] = temp;
        } catch (const SettingNotFoundException &nfex) { }

    } catch (const SettingNotFoundException &nfex) {
        std::cerr << "Log config not found" << std::endl;
    }
}

std::string LoggerConfig::get(std::string cfg) {
    auto pos = configMap.find(cfg);

    if (pos == configMap.end()) {
        throw std::runtime_error("Logger config element [" + cfg + "] does not exist");
    } else {
        return pos->second;
    }
}

std::string LoggerConfig::path(void) {
    return get(LoggerConfig::LogPath) + "/" + get(LoggerConfig::LogName);
}

void LoggerConfig::setup() {

    try {        
        spdlog::set_async_mode(1048576);
        auto async_file = spdlog::daily_logger_mt("async_file_logger", path(), 23, 59, true);

        std::string logLevel = get(LoggerConfig::LogLevel);
        if (logLevel == "debug") {
            async_file->set_level(spdlog::level::debug);
        } else if (logLevel == "notice") {
            async_file->set_level(spdlog::level::notice);        
        } else if (logLevel == "warn") {
            async_file->set_level(spdlog::level::warn);        
        } else if (logLevel == "error") {        
            async_file->set_level(spdlog::level::err);
        } else {
            async_file->set_level(spdlog::level::info);        
        }
    } catch (const spdlog::spdlog_ex &ex) {
        throw std::runtime_error(ex.what());
    }
}

void LoggerConfig::dump() {
    std::cout << "LoggerConfig::dump " << std::endl;
    for (auto pos = configMap.cbegin() ; pos != configMap.cend() ; pos++) {
        std::cout << pos->first << "=" << pos->second << std::endl;
    }
}

Answer 1

• It may be useful to use more specific error codes, such as EX_OSERR for a failed fork() call.

  In addition, your output statement can be improved. Since fork() updates errno on failure, replace your error output with just a call to std::perror() for a more accurate one.

  if (pid < 0) {
      std::perror("fork");
      return EX_OSERR;
  }
  

• When passing a non-native type (such as std::string) to a function, it's preferred to pass by const& to avoid an extra copy:

  bool contains(std::string const& argName);
  

  bool CommandLineOpts::contains(std::string const& argName) {
      return commandLineMap.count(argName);
  }
  

• Also regarding const: many of your member functions don't modify data members, yet they don't have const. This helps to prevent accidental modification of data members (causing a compiler error) while revealing more of the function's intent to others.

  std::string path(void) const;
  

  std::string LoggerConfig::path(void) const {
      return get(LoggerConfig::LogPath) + "/" + get(LoggerConfig::LogName);
  }
  

• Unlike in C, it's unnecessary to specify void for functions that take no arguments. The compiler will already be aware of this.

• In parseOpts(), it doesn't look like you should try to catch either an std::exception or ... and then throw either one as an std::runtime_error. The specific exception you may be looking for is std::logic_error. But this would depend on exactly which exception(s) can be thrown in this function.

• In showHelp(), you don't need to do all that flushing with std::endl, which affects performance. Instead, output a "\n" for just a newline.

• Since you're using C++11, you can use range-based for loops. For example, this can be applied to the loop in dump():

  for (auto& it : configMap) {
      std::cout << it.first << "=" << it.second << std::endl;
  }