HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Basic Linux daemon

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
linuxdaemonbasic

Problem

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 
#include 
#include 

#include 

#include 
#include 
#include 

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  0) {
        return EXIT_SUCCESS;
    }

    umask(0);

    /*
     * Create a new signature ID for the child
     */
    sid = setsid();
    if (sid debug() << "TODO:";
        sleep(60);
    }

    return EXIT_SUCCESS;
}


commandLineOpts.hpp

#ifndef COMMAND_LINE_OPTS_H
#define COMMAND_LINE_OPTS_H

#include 

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
#include

#include

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:

Solution

-
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;
}

Code Snippets

if (pid < 0) {
    std::perror("fork");
    return EX_OSERR;
}
bool contains(std::string const& argName);
bool CommandLineOpts::contains(std::string const& argName) {
    return commandLineMap.count(argName);
}
std::string path(void) const;
std::string LoggerConfig::path(void) const {
    return get(LoggerConfig::LogPath) + "/" + get(LoggerConfig::LogName);
}

Context

StackExchange Code Review Q#105017, answer score: 6

Revisions (0)

No revisions yet.