Emc race and cpcheck warnings

src/emc/usr_intf/schedrmt.cc

@@ -45,6 +45,8 @@
 #include "emcsched.hh"
 #include <rtapi_string.h>
 
+#include <atomic>		// for sessions counter
+
 /*
   Using schedrmt:
 
@@ -252,18 +254,17 @@ typedef struct {
   char progName[256];} connectionRecType;
 
 int port = 5008;
-int server_sockfd, client_sockfd;
-socklen_t server_len, client_len;
+int server_sockfd;
+socklen_t server_len;
 struct sockaddr_in server_address;
-struct sockaddr_in client_address;
 bool useSockets = true;
 int tokenIdx;
 const char *delims = " \n\r\0";
 int enabledConn = -1;
 char pwd[16] = "EMC\0";
 char enablePWD[16] = "EMCTOO\0";
 char serverName[24] = "EMCNETSVR\0";
-int sessions = 0;
+std::atomic_int sessions = 0;
 int maxSessions = -1;
 float pollDelay = 1.0;
 
@@ -1143,43 +1144,32 @@ void *checkQueue(void * /*arg*/)
   return 0;
 }  
 
-void *readClient(void * /*arg*/)
+void *readClient(void *arg)
 {
+  connectionRecType *context = (connectionRecType *)arg;
+
   char str[1600];
-  char buf[1600];
-  unsigned int i, j;
-  int len;
-  connectionRecType *context;
-
-
-//  res = 1;
-  context = (connectionRecType *) malloc(sizeof(connectionRecType));
-  context->cliSock = client_sockfd;
-  context->linked = false;
-  context->echo = true;
-  context->verbose = false;
-  rtapi_strxcpy(context->version, "1.0");
-  rtapi_strxcpy(context->hostName, "Default");
-  context->enabled = false;
-  context->commMode = 0;
-  context->commProt = 0;
-  context->inBuf[0] = 0;
-  buf[0] = 0;
-
+  char buf[1600] = {0};
+
   while (1) {
-    len = read(context->cliSock, &str, 1600);
+    unsigned int j = 0;
+    int len = read(context->cliSock, &str, sizeof(str)-1);
     if (len <= 0) goto finished;
     str[len] = 0;
     rtapi_strxcat(buf, str);
-    if (!memchr(str, 0x0d, strlen(str))) continue;
-    if (context->echo && context->linked)
+    if (!memchr(str, '\r', len)) continue;
+    if (context->echo && context->linked) {
       if(write(context->cliSock, buf, strlen(buf)) != (ssize_t)strlen(buf)) {
         fprintf(stderr, "emcrsh: write() failed: %s", strerror(errno));
       }
-    i = 0;
-    j = 0;
-    while (i <= strlen(buf)) {
+    }
+    // Iterate over lines in buffer and parse commands
+    size_t buflen = strlen(buf); // avoid multiple execution of strlen within loop
+    for (unsigned int i=0,j=0; i <= buflen; i++) {
       if ((buf[i] != '\n') && (buf[i] != '\r')) {
+        if (j>=sizeof(context->inBuf) - 1) {
+          goto finished;
+        }
         context->inBuf[j] = buf[i];
 	j++;
       }
@@ -1189,39 +1179,54 @@ void *readClient(void * /*arg*/)
         if (parseCommand(context) == -1) goto finished;
         j = 0;
       }
-      i++;
     }
-  buf[0] = 0;
+    buf[0] = 0;
   }
 
 finished:
   close(context->cliSock);
   free(context);
-  pthread_exit((void *)0);
-  sessions--;  // FIXME: not reached
+  --sessions; // std::atomic_int to avoid race condition
+  return(NULL); // equivalent to pthread_exit()
 }
 
-int sockMain()
+void sockMain()
 {
     pthread_t thrd;
-    int res;
 
     while (1) {
-
-      client_len = sizeof(client_address);
-      client_sockfd = accept(server_sockfd,
-        (struct sockaddr *)&client_address, &client_len);
-      if (client_sockfd < 0) exit(0);
-      sessions++;
-      if ((maxSessions == -1) || (sessions <= maxSessions))
-        res = pthread_create(&thrd, NULL, readClient, (void *)NULL);
-      else res = -1;
-      if (res != 0) {
-        close(client_sockfd);
-        sessions--;
+        int res = -1;
+        struct sockaddr_in client_address;
+        socklen_t client_len = sizeof(client_address);
+        int client_sockfd = accept(server_sockfd, (struct sockaddr *)&client_address, &client_len);
+        if (client_sockfd < 0) {
+            perror("sockMain: accept failed\n");
+            exit(0);
         }
-     }
-    return 0;
+        ++sessions;
+        if ((maxSessions == -1) || (sessions <= maxSessions)) {
+            connectionRecType *context = (connectionRecType *)calloc(1, sizeof(connectionRecType));
+            if (context) {
+                // Initialize necessary context fields, rest are set to zero in calloc
+                context->cliSock = client_sockfd;
+                rtapi_strxcpy(context->hostName, "Default");
+                rtapi_strxcpy(context->version, "1.0");
+                context->echo = true;
+                res = pthread_create(&thrd, NULL, readClient, context);
+                if (pthread_detach(thrd)) {
+                   // no errno set by pthread_detach
+                   fprintf(stderr, "sockMain: error by pthread_detach - ignored\n");
+                }
+                if (res != 0) {
+                    free(context);
+                }
+            }
+        }
+        if (res != 0) {
+            close(client_sockfd);
+            --sessions;
+        }
+    }
 }
 
 static void initMain()
@@ -1287,6 +1292,7 @@ int main(int argc, char *argv[])
 
     schedInit();
     res = pthread_create(&updateThread, NULL, checkQueue, (void *)NULL);
+    pthread_detach(updateThread);
     if(res != 0) { perror("pthread_create"); return 1; }
     if (useSockets) sockMain();