From a5d203a9bec429cf852e7b99b9f3fe03c2937e2b Mon Sep 17 00:00:00 2001 From: Martin Konrad Date: Tue, 27 Oct 2020 10:28:58 -0400 Subject: [PATCH 1/5] Refactor generation of procServ messages Replace fixed size char string buffers by std::string to rule out buffer overflows/truncating strings. By doing so we get rid of some compiler warnings. --- clientFactory.cc | 46 ++++++++++++++------------------- procServ.cc | 65 ++++++++++++++++++++++------------------------- procServ.h | 11 +++----- processFactory.cc | 8 +++--- 4 files changed, 59 insertions(+), 71 deletions(-) diff --git a/clientFactory.cc b/clientFactory.cc index 9a926ca..24950e7 100644 --- a/clientFactory.cc +++ b/clientFactory.cc @@ -18,6 +18,8 @@ #include "processClass.h" #include "libtelnet.h" +using namespace std::literals::string_literals; + // Wrapper to ignore return values template inline void ignore_result(T /* unused result */) {} @@ -95,32 +97,26 @@ clientItem::clientItem(int socketIn, bool readonly) : char procServStart_buf[32]; // Time when this procServ started - as string struct tm IOCStart_tm; // Time when the current IOC was started char IOCStart_buf[32]; // Time when the current IOC was started - as string -#define BUFLEN 512 - char buf1[BUFLEN], buf2[BUFLEN]; - char greeting1[] = "@@@ Welcome to procServ (" PROCSERV_VERSION_STRING ")" NL; -#define GREETLEN 256 - char greeting2[GREETLEN] = ""; + const char greeting1[] = "@@@ Welcome to procServ (" PROCSERV_VERSION_STRING ")" NL; + std::string greeting2 {}; struct timeval send_timeout; send_timeout.tv_sec = 10; send_timeout.tv_usec = 0; PRINTF("New clientItem %p\n", this); if ( killChar ) { - snprintf(greeting2, GREETLEN, "@@@ Use %s%c to kill the child, ", CTL_SC(killChar)); + greeting2 += "@@@ Use " + ctl_str(killChar) + " to kill the child, "; } else { - snprintf(greeting2, GREETLEN, "@@@ Kill command disabled, "); + greeting2 += "@@@ Kill command disabled, "; } - snprintf(buf1, BUFLEN, "auto restart mode is %s, ", restartModeString()); + greeting2 += "auto restart mode is "s + restartModeString() + ", "; if ( toggleRestartChar ) { - snprintf(buf2, BUFLEN, "use %s%c to toggle auto restart" NL, CTL_SC(toggleRestartChar)); + greeting2 += "use " + ctl_str(toggleRestartChar) + " to toggle auto restart" NL; } else { - snprintf(buf2, BUFLEN, "auto restart toggle disabled" NL); + greeting2 += "auto restart toggle disabled" NL; } - strncat(greeting2, buf1, GREETLEN-strlen(greeting2)-1); - strncat(greeting2, buf2, GREETLEN-strlen(greeting2)-1); if (logoutChar) { - snprintf(buf2, BUFLEN, "@@@ Use %s%c to logout from procServ server" NL, CTL_SC(logoutChar)); - strncat(greeting2, buf2, GREETLEN-strlen(greeting2)-1); + greeting2 += "@@@ Use " + ctl_str(logoutChar) + " to logout from procServ server" NL; } localtime_r( &procServStart, &procServStart_tm ); @@ -131,17 +127,13 @@ clientItem::clientItem(int socketIn, bool readonly) : strftime( IOCStart_buf, sizeof(IOCStart_buf)-1, timeFormat, &IOCStart_tm ); - snprintf(buf1, BUFLEN, "@@@ procServ server started at: %s" NL, - procServStart_buf); + std::string buf1 = "@@@ procServ server started at: "s + procServStart_buf + NL; if ( processClass::exists() ) { - snprintf(buf2, BUFLEN, "@@@ Child \"%s\" started at: %s" NL, - childName, IOCStart_buf ); - strncat(buf1, buf2, BUFLEN-strlen(buf1)-1); + buf1 += "@@@ Child \""s + childName + "\" started at: " + IOCStart_buf + NL; } - snprintf(buf2, BUFLEN, "@@@ %d user(s) and %d logger(s) connected (plus you)" NL, - _users, _loggers); + std::string buf2 = "@@@ " + std::to_string(_users) + " user(s) and " + std::to_string(_loggers) + " logger(s) connected (plus you)" NL; setsockopt( socketIn, SOL_SOCKET, SO_KEEPALIVE, &optval, sizeof(optval) ); setsockopt( socketIn, SOL_SOCKET, SO_SNDTIMEO, &send_timeout, sizeof(send_timeout) ); @@ -151,16 +143,16 @@ clientItem::clientItem(int socketIn, bool readonly) : } else { // Regular (user) client _users++; ignore_result( write(_fd, greeting1, strlen(greeting1)) ); - ignore_result( write(_fd, greeting2, strlen(greeting2)) ); + ignore_result( write(_fd, greeting2.c_str(), greeting2.length() + 1) ); } - ignore_result( write(_fd, infoMessage1, strlen(infoMessage1)) ); - ignore_result( write( _fd, infoMessage2, strlen(infoMessage2)) ); - ignore_result( write( _fd, buf1, strlen(buf1)) ); + ignore_result( write(_fd, infoMessage1.c_str(), infoMessage1.length() + 1) ); + ignore_result( write( _fd, infoMessage2.c_str(), infoMessage2.length() + 1) ); + ignore_result( write( _fd, buf1.c_str(), buf1.length() + 1) ); if ( ! _readonly ) - ignore_result( write(_fd, buf2, strlen(buf2)) ); + ignore_result( write(_fd, buf2.c_str(), buf2.length() + 1) ); if ( ! processClass::exists() ) - ignore_result( write(_fd, infoMessage3, strlen(infoMessage3)) ); + ignore_result( write(_fd, infoMessage3.c_str(), infoMessage3.length() + 1) ); _telnet = telnet_init(my_telopts, telnet_eh, 0, this); diff --git a/procServ.cc b/procServ.cc index a693541..77f3a2b 100644 --- a/procServ.cc +++ b/procServ.cc @@ -36,6 +36,8 @@ #include "procServ.h" +using namespace std::literals::string_literals; + // Wrapper to ignore return values template inline void ignore_result(T /* unused result */) {} @@ -82,13 +84,9 @@ char defaulttimeFormat[] = "%c"; // default bool stampLog = false; // Prefix log lines with time stamp const char *stampFormat; // Log time stamp format string -const size_t INFO1LEN = 512; -const size_t INFO2LEN = 128; -const size_t INFO3LEN = 128; - -char infoMessage1[INFO1LEN]; // Sign on message: server PID, child pwd and command line -char infoMessage2[INFO2LEN]; // Sign on message: child PID -char infoMessage3[INFO3LEN]; // Sign on message: available server commands +std::string infoMessage1; // Sign on message: server PID, child pwd and command line +std::string infoMessage2; // Sign on message: child PID +std::string infoMessage3; // Sign on message: available server commands char *logFile = NULL; // File name for log int logFileFD=-1; // FD for log file @@ -118,6 +116,19 @@ static volatile sig_atomic_t sigPipeSet; static volatile sig_atomic_t sigTermSet; static volatile sig_atomic_t sigHupSet; +bool is_cntrl_char(const char c) { + return c > 0 && c < 32; +} + +std::string ctl_str(const char c) { + if (is_cntrl_char(c)) { + const char human_readable_representation = c + 64; + return std::string("^") + human_readable_representation; + } else { + return std::string(c, 1); + } +} + void writePidFile() { int pid = getpid(); @@ -212,8 +223,6 @@ int main(int argc,char * argv[]) std::vector ctlSpecs; char *command; bool bailout = false; - const size_t BUFLEN = 512; - char buff[BUFLEN]; std::string infofile; time(&procServStart); // remember start time @@ -440,15 +449,13 @@ int main(int argc,char * argv[]) // Set up available server commands message PRINTF("Setting up messages\n"); - snprintf(infoMessage3, INFO3LEN,\ - "@@@ %s%c or %s%c restarts the child, %s%c quits the server", - CTL_SC(restartChar), CTL_SC(killChar), CTL_SC(quitChar)); + infoMessage3 = "@@@ "s + ctl_str(restartChar).c_str() + " or " + + ctl_str(killChar).c_str() + " restarts the child, " + + ctl_str(quitChar).c_str() + " quits the server"; if (logoutChar) { - snprintf(buff, BUFLEN, ", %s%c closes this connection", - CTL_SC(logoutChar)); - strncat(infoMessage3, buff, INFO3LEN-strlen(infoMessage3)-1); + infoMessage3 += ", "s + ctl_str(logoutChar).c_str() + " closes this connection"; } - strncat(infoMessage3, NL, INFO3LEN-strlen(infoMessage3)-1); + infoMessage3 += NL; if (singleEndpointStyle) { ctlSpecs.push_back(argv[optind++]); @@ -569,29 +576,19 @@ int main(int argc,char * argv[]) } // Record some useful data for managers - snprintf(infoMessage1, INFO1LEN, - "@@@ procServ server PID: %ld" NL - "@@@ Server startup directory: %s" NL - "@@@ Child startup directory: %s" NL, - (long) getpid(), - myDir, - chDir); + infoMessage1 = "@@@ procServ server PID: "s + std::to_string(getpid()) + NL + + "@@@ Server startup directory: " + myDir + NL + + "@@@ Child startup directory: " + chDir + NL; if ( strcmp( childName, command ) ) - snprintf(buff, BUFLEN, "@@@ Child \"%s\" started as: %s" NL, - childName, command ); + infoMessage1 += "@@@ Child \""s + childName + "\" started as: " + command + NL; else - snprintf(buff, BUFLEN, "@@@ Child started as: %s" NL, - command ); - strncat(infoMessage1, buff, INFO1LEN-strlen(infoMessage1)-1); - snprintf(infoMessage2, INFO2LEN, "@@@ Child \"%s\" is SHUT DOWN" NL, childName); + infoMessage1 += "@@@ Child started as: "s + command + NL; + infoMessage2 = "@@@ Child \""s + childName + "\" is SHUT DOWN" NL; if ( logFile ) { if ( -1 == logFileFD ) - snprintf(buff, BUFLEN, "@@@ Child log file: unable to open log file %s" NL, - logFile ); + infoMessage1 += "@@@ Child log file: unable to open log file "s + logFile + NL; else - snprintf(buff, BUFLEN, "@@@ Child log file: %s" NL, - logFile ); - strncat(infoMessage1, buff, INFO1LEN-strlen(infoMessage1)-1); + infoMessage1 += "@@@ Child log file: "s + logFile + NL; } firstRun = true; diff --git a/procServ.h b/procServ.h index 0dcb381..08f7ca7 100644 --- a/procServ.h +++ b/procServ.h @@ -51,12 +51,9 @@ extern char restartChar; extern char quitChar; extern char logoutChar; extern int killSig; -extern const size_t INFO1LEN; -extern const size_t INFO2LEN; -extern const size_t INFO3LEN; -extern char infoMessage1[]; -extern char infoMessage2[]; -extern char infoMessage3[]; +extern std::string infoMessage1; +extern std::string infoMessage2; +extern std::string infoMessage3; extern pid_t procservPid; extern rlim_t coreSize; extern char *chDir; @@ -64,7 +61,7 @@ extern time_t holdoffTime; #define NL "\r\n" -#define CTL_SC(c) c > 0 && c < 32 ? "^" : "", c > 0 && c < 32 ? c + 64 : c +std::string ctl_str(const char c); class connectionItem; diff --git a/processFactory.cc b/processFactory.cc index f86e4dd..8a8091f 100644 --- a/processFactory.cc +++ b/processFactory.cc @@ -38,6 +38,8 @@ extern "C" int forkpty(int*, char*, void*, void*); #include "procServ.h" #include "processClass.h" +using namespace std::literals::string_literals; + #define LINEBUF_LENGTH 1024 static void hideWindow(); @@ -106,12 +108,12 @@ processClass::~processClass() "oneshot mode: server will exit")); // Update client connect message - snprintf(infoMessage2, INFO2LEN, "@@@ Child \"%s\" is SHUT DOWN" NL, childName); + infoMessage2 = "@@@ Child \""s + childName + "\" is SHUT DOWN" NL; SendToAll( now_buf, strlen(now_buf), this ); SendToAll( goodbye, strlen(goodbye), this ); if (restartMode != oneshot) - SendToAll( infoMessage3, strlen(infoMessage3), this ); + SendToAll( infoMessage3.c_str(), infoMessage3.length() + 1, this ); // Negative PID sends signal to all members of process group if ( _pid > 0 ) kill( -_pid, SIGKILL ); @@ -188,7 +190,7 @@ processClass::processClass(char *exe, char *argv[]) _restartTime = holdoffTime + time(0); // Update client connect message - snprintf(infoMessage2, INFO2LEN, "@@@ Child \"%s\" PID: %ld" NL, childName, (long) _pid); + infoMessage2 = "@@@ Child \""s + childName + "\" PID: " + std::to_string(_pid) + NL; snprintf(buf, BUFLEN, "@@@ The PID of new child \"%s\" is: %ld" NL, childName, (long) _pid); SendToAll( buf, strlen(buf), this ); From f10d5b2192309f4c68dbd31d179a31b04ef57c4c Mon Sep 17 00:00:00 2001 From: Martin Konrad Date: Tue, 27 Oct 2020 11:43:23 -0400 Subject: [PATCH 2/5] Make code C++11 compatible --- clientFactory.cc | 8 +++----- procServ.cc | 21 +++++++++++---------- processFactory.cc | 6 ++---- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/clientFactory.cc b/clientFactory.cc index 24950e7..24c789d 100644 --- a/clientFactory.cc +++ b/clientFactory.cc @@ -18,8 +18,6 @@ #include "processClass.h" #include "libtelnet.h" -using namespace std::literals::string_literals; - // Wrapper to ignore return values template inline void ignore_result(T /* unused result */) {} @@ -109,7 +107,7 @@ clientItem::clientItem(int socketIn, bool readonly) : } else { greeting2 += "@@@ Kill command disabled, "; } - greeting2 += "auto restart mode is "s + restartModeString() + ", "; + greeting2 += std::string("auto restart mode is ") + restartModeString() + ", "; if ( toggleRestartChar ) { greeting2 += "use " + ctl_str(toggleRestartChar) + " to toggle auto restart" NL; } else { @@ -127,10 +125,10 @@ clientItem::clientItem(int socketIn, bool readonly) : strftime( IOCStart_buf, sizeof(IOCStart_buf)-1, timeFormat, &IOCStart_tm ); - std::string buf1 = "@@@ procServ server started at: "s + procServStart_buf + NL; + std::string buf1 = std::string("@@@ procServ server started at: ") + procServStart_buf + NL; if ( processClass::exists() ) { - buf1 += "@@@ Child \""s + childName + "\" started at: " + IOCStart_buf + NL; + buf1 += std::string("@@@ Child \"") + childName + "\" started at: " + IOCStart_buf + NL; } std::string buf2 = "@@@ " + std::to_string(_users) + " user(s) and " + std::to_string(_loggers) + " logger(s) connected (plus you)" NL; diff --git a/procServ.cc b/procServ.cc index 77f3a2b..59dd3d8 100644 --- a/procServ.cc +++ b/procServ.cc @@ -36,8 +36,6 @@ #include "procServ.h" -using namespace std::literals::string_literals; - // Wrapper to ignore return values template inline void ignore_result(T /* unused result */) {} @@ -449,11 +447,12 @@ int main(int argc,char * argv[]) // Set up available server commands message PRINTF("Setting up messages\n"); - infoMessage3 = "@@@ "s + ctl_str(restartChar).c_str() + " or " + + infoMessage3 = std::string("@@@ ") + ctl_str(restartChar).c_str() + " or " + ctl_str(killChar).c_str() + " restarts the child, " + ctl_str(quitChar).c_str() + " quits the server"; if (logoutChar) { - infoMessage3 += ", "s + ctl_str(logoutChar).c_str() + " closes this connection"; + infoMessage3 += std::string(", ") + ctl_str(logoutChar).c_str() + + " closes this connection"; } infoMessage3 += NL; @@ -576,19 +575,21 @@ int main(int argc,char * argv[]) } // Record some useful data for managers - infoMessage1 = "@@@ procServ server PID: "s + std::to_string(getpid()) + NL + + infoMessage1 = std::string("@@@ procServ server PID: ") + std::to_string(getpid()) + NL + "@@@ Server startup directory: " + myDir + NL + "@@@ Child startup directory: " + chDir + NL; if ( strcmp( childName, command ) ) - infoMessage1 += "@@@ Child \""s + childName + "\" started as: " + command + NL; + infoMessage1 += std::string("@@@ Child \"") + childName + "\" started as: " + + command + NL; else - infoMessage1 += "@@@ Child started as: "s + command + NL; - infoMessage2 = "@@@ Child \""s + childName + "\" is SHUT DOWN" NL; + infoMessage1 += std::string("@@@ Child started as: ") + command + NL; + infoMessage2 = std::string("@@@ Child \"") + childName + "\" is SHUT DOWN" NL; if ( logFile ) { if ( -1 == logFileFD ) - infoMessage1 += "@@@ Child log file: unable to open log file "s + logFile + NL; + infoMessage1 += std::string("@@@ Child log file: unable to open log file ") + + logFile + NL; else - infoMessage1 += "@@@ Child log file: "s + logFile + NL; + infoMessage1 += std::string("@@@ Child log file: ") + logFile + NL; } firstRun = true; diff --git a/processFactory.cc b/processFactory.cc index 8a8091f..bae8ce3 100644 --- a/processFactory.cc +++ b/processFactory.cc @@ -38,8 +38,6 @@ extern "C" int forkpty(int*, char*, void*, void*); #include "procServ.h" #include "processClass.h" -using namespace std::literals::string_literals; - #define LINEBUF_LENGTH 1024 static void hideWindow(); @@ -108,7 +106,7 @@ processClass::~processClass() "oneshot mode: server will exit")); // Update client connect message - infoMessage2 = "@@@ Child \""s + childName + "\" is SHUT DOWN" NL; + infoMessage2 = std::string("@@@ Child \"") + childName + "\" is SHUT DOWN" NL; SendToAll( now_buf, strlen(now_buf), this ); SendToAll( goodbye, strlen(goodbye), this ); @@ -190,7 +188,7 @@ processClass::processClass(char *exe, char *argv[]) _restartTime = holdoffTime + time(0); // Update client connect message - infoMessage2 = "@@@ Child \""s + childName + "\" PID: " + std::to_string(_pid) + NL; + infoMessage2 = std::string("@@@ Child \"") + childName + "\" PID: " + std::to_string(_pid) + NL; snprintf(buf, BUFLEN, "@@@ The PID of new child \"%s\" is: %ld" NL, childName, (long) _pid); SendToAll( buf, strlen(buf), this ); From 4b2aa0b9248786c1f3ca3815cac76e67a2555e7a Mon Sep 17 00:00:00 2001 From: Martin Konrad Date: Mon, 2 Nov 2020 11:34:47 -0500 Subject: [PATCH 3/5] Require C++11 compiler --- configure.ac | 1 + 1 file changed, 1 insertion(+) diff --git a/configure.ac b/configure.ac index 03fd213..f6093ab 100644 --- a/configure.ac +++ b/configure.ac @@ -44,6 +44,7 @@ AC_TYPE_MODE_T AC_TYPE_PID_T AC_C_RESTRICT AC_STRUCT_TM +AX_CXX_COMPILE_STDCXX_11([noext], [mandatory]) # Checks for library functions. AC_FUNC_FORK From c1ba2e765a399146433e3380999d41e1546607e5 Mon Sep 17 00:00:00 2001 From: Martin Konrad Date: Mon, 2 Nov 2020 13:42:54 -0500 Subject: [PATCH 4/5] Install autoconv-archive on Travis --- .travis.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.travis.yml b/.travis.yml index b6ff8e6..5b746e5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,6 +3,15 @@ cache: directories: - $HOME/.cache +addons: + apt: + packages: + - autoconf-archive + homebrew: + packages: + - autoconf-archive + update: true + jobs: include: - os: linux From 78bba231a9a9bdedf6ed8aeaa26b7f0bf8b79090 Mon Sep 17 00:00:00 2001 From: Martin Konrad Date: Mon, 2 Nov 2020 14:28:01 -0500 Subject: [PATCH 5/5] Set CXXFLAGS for EPICS build --- Makefile.Epics.in | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile.Epics.in b/Makefile.Epics.in index 711f942..9cd0715 100644 --- a/Makefile.Epics.in +++ b/Makefile.Epics.in @@ -15,6 +15,7 @@ procServ_SRCS = procServ.cc connectionItem.cc acceptFactory.cc \ procServ_OBJS = @LIBOBJS@ USR_CXXFLAGS += @DEFS@ +USR_CXXFLAGS += @CXXFLAGS@ procServ_SYS_LIBS += $(subst -l,,@LIBS@) include $(TOP)/configure/RULES