patterncppMinor
Basic Linux daemon
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
commandLineOpts.hpp
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:
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;
};
#endifcommandLineOpts.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
In addition, your output statement can be improved. Since
-
When passing a non-native type (such as
-
Also regarding
-
Unlike in C, it's unnecessary to specify
-
In
-
In
-
Since you're using C++11, you can use range-based
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.