Fix gcc -fsanitize detected problems

src/emc/ini/inifile.cc

@@ -855,6 +855,14 @@ IniFile::IniFile(const std::string &path)
 	Open(path);
 }
 
+IniFile::IniFile(const std::string *path)
+	: _inifilecontent(nullptr),
+	  _filepath{}
+{
+	if(path)
+		Open(*path);
+}
+
 bool IniFile::Open(const std::string &path)
 {
 	Close();

src/emc/ini/inifile.hh

@@ -144,6 +144,7 @@ class IniFile
 {
 public:
 	IniFile(const std::string &filePath);
+	IniFile(const std::string *filePath);
 
 	operator bool() const { return isOpen(); }
 	bool isOpen() const { return _inifilecontent != nullptr; }

src/emc/kinematics/genserfuncs.c

@@ -48,8 +48,8 @@
 #if __GNUC__ && !defined(__clang__)
 // The matrix and vector storage is just big.
 // genser_kin_jac_inv() is 2112
-// genserKinematicsInverse() is 2576
-  #pragma GCC diagnostic warning "-Wframe-larger-than=2600"
+// genserKinematicsInverse() is 2640
+  #pragma GCC diagnostic warning "-Wframe-larger-than=2648"
 #endif
 
 static struct haldata {

src/emc/rs274ngc/interp_o_word.cc

@@ -567,7 +567,7 @@ int Interp::control_back_to( block_pointer block, // pointer to block
                 logOword("filename too long: %s", op->filename);
                 ERS(NCE_UNABLE_TO_OPEN_FILE, op->filename);
             }
-            strncpy(settings->filename, op->filename, sizeof(settings->filename));
+            strncpy(settings->filename, op->filename, sizeof(settings->filename)-1);
 
 	    if (newFP) {
 		// close the old file...

src/emc/rs274ngc/interp_write.cc

@@ -226,7 +226,7 @@ int Interp::write_state_tag(block_pointer block,
     bool in_sub = (settings->call_level > 0 && settings->remap_level == 0);
     bool external_sub = strcmp(settings->filename,
 			       settings->sub_context[0].filename);
-    strncpy(state.filename, settings->filename, sizeof(state.filename));
+    rtapi_strxcpy(state.filename, settings->filename);
     state.filename[sizeof(state.filename)-1] = 0;
 
     state.flags[GM_FLAG_IN_REMAP] = in_remap;

src/emc/task/emccanon.cc

@@ -3433,7 +3433,7 @@ void MESSAGE(char *s)
     auto operator_display_msg = std::make_unique<EMC_OPERATOR_DISPLAY>();
 
     flush_segments();
-    strncpy(operator_display_msg->display, s, LINELEN);
+    strncpy(operator_display_msg->display, s, LINELEN-1);
     operator_display_msg->display[LINELEN - 1] = 0;
     interp_list.append(std::move(operator_display_msg));
 }

src/emc/usr_intf/axis/extensions/emcmodule.cc

@@ -74,7 +74,9 @@ using namespace linuxcnc;
 
 struct pyIniFile {
     PyObject_HEAD
-    std::string inifile;
+    // The 'inifile' member is a pointer because we don't
+    // have C++ constructor semantics here
+    std::string *inifile;
 };
 
 struct pyStatChannel {
@@ -98,12 +100,15 @@ struct pyErrorChannel {
 static PyObject *m = NULL, *error = NULL;
 
 static int Ini_init(pyIniFile *self, PyObject *a, PyObject * /*k*/) {
-    char *inifile;
+    const char *inifile = NULL;
     if(!PyArg_ParseTuple(a, "s", &inifile)) return -1;
 
-    self->inifile = inifile;
+    if(!inifile)
+        return -1;
+
+    self->inifile = new std::string(inifile);
 
-    IniFile ini(inifile); // Test open (will parse and cache the file)
+    IniFile ini(self->inifile); // Test open (will parse and cache the file)
     if (!ini) {
         PyErr_Format( error, "inifile.open(%s) failed", inifile);
         return -1;
@@ -688,6 +693,10 @@ static PyObject *Ini_get_jointtype(pyIniFile *self, PyObject *args, PyObject *kw
 }
 
 static void Ini_dealloc(pyIniFile *self) {
+    if(self->inifile) {
+        delete self->inifile;
+        self->inifile = NULL;
+    }
     PyObject_Del(self);
 }
 

src/emc/usr_intf/emclcd.cc

@@ -1686,9 +1686,9 @@ static void initMain()
     emcStatus = 0;
 
     emcErrorBuffer = 0;
-    error_string[LINELEN-1] = 0;
-    operator_text_string[LINELEN-1] = 0;
-    operator_display_string[LINELEN-1] = 0;
+    error_string.clear();
+    operator_text_string.clear();
+    operator_display_string.clear();
     programStartLine = 0;
 }
 

src/emc/usr_intf/emcrsh.cc

@@ -1876,12 +1876,11 @@ static cmdResponseType getError(connectionRecType &ctx)
 		return rtError;
 	}
 
-	// FIXME: static buffer 'error_string'
-	if (!error_string[0]) {
+	if (error_string.empty()) {
 		replynl(ctx, "ERROR OK");
 	} else {
 		replynl(ctx, fmt::format("ERROR {}", error_string));
-		error_string[0] = 0;
+		error_string.clear();
 	}
 	return rtOk;
 }
@@ -1894,12 +1893,11 @@ static cmdResponseType getOperatorDisplay(connectionRecType &ctx)
 		return rtError;
 	}
 
-	// FIXME: static buffer 'operator_display_string'
-	if (!operator_display_string[0])
+	if (operator_display_string.empty())
 		replynl(ctx, "OPERATOR_DISPLAY OK");
 	else {
 		replynl(ctx, fmt::format("OPERATOR_DISPLAY {}", operator_display_string));
-		operator_display_string[0] = 0;
+		operator_display_string.clear();
 	}
 	return rtOk;
 }
@@ -1912,12 +1910,11 @@ static cmdResponseType getOperatorText(connectionRecType &ctx)
 		return rtError;
 	}
 
-	// FIXME: static buffer 'operator_text_string'
-	if (!operator_text_string[0])
+	if (operator_text_string.empty())
 		replynl(ctx, "OPERATOR_TEXT OK");
 	else {
 		replynl(ctx, fmt::format("OPERATOR_TEXT {}", operator_text_string));
-		operator_text_string[0] = 0;
+		operator_text_string.clear();
 	}
 	return rtOk;
 }
@@ -3697,10 +3694,9 @@ int main(int argc, char *argv[])
 	emcStatusBuffer = NULL;
 	emcErrorBuffer = NULL;
 	emcStatus = NULL;
-	// FIXME: Get rid of static buffers
-	memset(error_string, 0, sizeof(error_string));
-	memset(operator_text_string, 0, sizeof(operator_text_string));
-	memset(operator_display_string, 0, sizeof(operator_display_string));
+	error_string.clear();
+	operator_text_string.clear();
+	operator_display_string.clear();
 	programStartLine = 0;
 
 	searchPath.push_back(getDefaultPath());

src/emc/usr_intf/emcsched.cc

@@ -300,7 +300,7 @@ void updateQueue() {
             queueStatus = qsError;
             }
           sendAuto();
-          rtapi_strxcpy(fileStr, defaultPath);
+          rtapi_strxcpy(fileStr, defaultPath.c_str());
           rtapi_strxcat(fileStr, q.front().getFileName().c_str());
           if (sendProgramOpen(fileStr) != 0) {
             queueStatus = qsError;

src/emc/usr_intf/emcsh.cc

@@ -868,17 +868,17 @@ static int emc_error(ClientData /*clientdata*/,
 
     CHECKEMC
     if (objc == 1) {
-	// get any new error, it's saved in global error_string[]
+	// get any new error, it's saved in global error_string
 	if (0 != updateError()) {
 	    setresult(interp,"emc_error: bad status from EMC");
 	    return TCL_ERROR;
 	}
 	// put error on result list
-	if (error_string[0] == 0) {
+	if (error_string.empty()) {
 	    setresult(interp,"ok");
 	} else {
-	    setresult(interp,error_string);
-	    error_string[0] = 0;
+	    setresult(interp,error_string.c_str());
+	    error_string.clear();
 	}
 	return TCL_OK;
     }
@@ -894,17 +894,17 @@ static int emc_operator_text(ClientData /*clientdata*/,
 
     CHECKEMC
     if (objc == 1) {
-	// get any new string, it's saved in global operator_text_string[]
+	// get any new string, it's saved in global operator_text_string
 	if (0 != updateError()) {
 	    setresult(interp,"emc_operator_text: bad status from EMC");
 	    return TCL_ERROR;
 	}
 	// put error on result list
-	if (operator_text_string[0] == 0) {
+	if (operator_text_string.empty()) {
 	    setresult(interp,"ok");
-	    operator_text_string[0] = 0;
 	} else {
-	    setresult(interp,operator_text_string);
+	    setresult(interp,operator_text_string.c_str());
+	    operator_text_string.clear();
 	}
 	return TCL_OK;
     }
@@ -920,17 +920,17 @@ static int emc_operator_display(ClientData /*clientdata*/,
 
     CHECKEMC
     if (objc == 1) {
-	// get any new string, it's saved in global operator_display_string[]
+	// get any new string, it's saved in global operator_display_string
 	if (0 != updateError()) {
 	    setresult(interp,"emc_operator_display: bad status from EMC");
 	    return TCL_ERROR;
 	}
 	// put error on result list
-	if (operator_display_string[0] == 0) {
+	if (operator_display_string.empty()) {
 	    setresult(interp,"ok");
 	} else {
-	    setresult(interp,operator_display_string);
-	    operator_display_string[0] = 0;
+	    setresult(interp,operator_display_string.c_str());
+	    operator_display_string.clear();
 	}
 	return TCL_OK;
     }
@@ -3640,9 +3640,9 @@ static void initMain()
     emcStatus = 0;
 
     emcErrorBuffer = 0;
-    error_string[LINELEN-1] = 0;
-    operator_text_string[LINELEN-1] = 0;
-    operator_display_string[LINELEN-1] = 0;
+    error_string.clear();
+    operator_text_string.clear();
+    operator_display_string.clear();
     programStartLine = 0;
 }
 

src/emc/usr_intf/schedrmt.cc

@@ -1236,9 +1236,9 @@ static void initMain()
     emcStatus = 0;
 
     emcErrorBuffer = 0;
-    error_string[LINELEN-1] = 0;
-    operator_text_string[LINELEN-1] = 0;
-    operator_display_string[LINELEN-1] = 0;
+    error_string.clear();
+    operator_text_string.clear();
+    operator_display_string.clear();
     programStartLine = 0;
 }
 
@@ -1258,7 +1258,7 @@ int main(int argc, char *argv[])
         case 'p': sscanf(optarg, "%d", &port); break;
         case 's': sscanf(optarg, "%d", &maxSessions); break;
         case 'w': snprintf(pwd, sizeof(pwd), "%s", optarg); break;
-        case 'd': snprintf(defaultPath, sizeof(defaultPath), "%s", optarg); break;
+        case 'd': defaultPath = optarg; break;
         }
       }
 

src/emc/usr_intf/shcom.cc

@@ -47,10 +47,10 @@ EMC_STAT *emcStatus;
 
 // the NML channel for errors
 NML *emcErrorBuffer;
-char error_string[NML_ERROR_LEN];
-char operator_text_string[NML_TEXT_LEN];
-char operator_display_string[NML_DISPLAY_LEN];
-char defaultPath[80] = DEFAULT_PATH;
+std::string error_string;
+std::string operator_text_string;
+std::string operator_display_string;
+std::string defaultPath = DEFAULT_PATH;
 // default value for timeout, 0 means wait forever
 double emcTimeout;
 int programStartLine;
@@ -173,6 +173,21 @@ int updateStatus()
 
 }
 
+// Copy a static char buffer from nml into a std::string.
+// We know the max size is the orginal buffer's size-1 (the len argument).
+// However, the content does not need to be that long and may be terminated
+// somewhere. Search the first embedded NUL to terminate early. If no embedded
+// NUL is found, then the string /might/ have been unterminated and will be
+// properly limited by the len argument. That way we don't depend on the
+// buffer's termination.
+static void cpy_str(std::string &target, const char *src, size_t len)
+{
+    target = std::string(src, len); // Copy entire buffer
+    size_t pos = target.find_first_of((char)0);  // Find first terminator (embedded NUL)
+    if(std::string::npos != pos)
+        target.erase(pos);          // Remove trailing if terminated before
+}
+
 /*
   updateError() updates "errors," which are true errors and also
   operator display and text messages.
@@ -192,53 +207,47 @@ int updateError()
 
     default:
 	// if not recognized, set the error string
-	snprintf(error_string, sizeof(error_string), "shcom:updateError(): unrecognized error %d", (int)type);
+	error_string = fmt::format("shcom:updateError(): unrecognized error {}", (int)type);
 	return 0;
 
     case 0:
 	// nothing new
 	break;
 
     case EMC_OPERATOR_ERROR_TYPE:
-	strncpy(error_string,
+	cpy_str(error_string,
 		(static_cast<EMC_OPERATOR_ERROR *>((emcErrorBuffer->get_address()))->error),
                 LINELEN - 1);
-	error_string[NML_ERROR_LEN - 1] = 0;
 	break;
 
     case EMC_OPERATOR_TEXT_TYPE:
-	strncpy(operator_text_string,
+	cpy_str(operator_text_string,
 		(static_cast<EMC_OPERATOR_TEXT *>((emcErrorBuffer->get_address()))->text),
                 LINELEN - 1);
-	operator_text_string[NML_TEXT_LEN - 1] = 0;
 	break;
 
     case EMC_OPERATOR_DISPLAY_TYPE:
-	strncpy(operator_display_string,
+	cpy_str(operator_display_string,
 		(static_cast<EMC_OPERATOR_DISPLAY *>((emcErrorBuffer->get_address()))->display),
 		LINELEN - 1);
-	operator_display_string[NML_DISPLAY_LEN - 1] = 0;
 	break;
 
     case NML_ERROR_TYPE:
-	strncpy(error_string,
+        cpy_str(error_string,
 		(static_cast<NML_ERROR *>((emcErrorBuffer->get_address()))->error),
 		NML_ERROR_LEN - 1);
-	error_string[NML_ERROR_LEN - 1] = 0;
 	break;
 
     case NML_TEXT_TYPE:
-	strncpy(operator_text_string,
+	cpy_str(operator_text_string,
 		(static_cast<NML_TEXT *>((emcErrorBuffer->get_address()))->text),
 		NML_TEXT_LEN - 1);
-	operator_text_string[NML_TEXT_LEN - 1] = 0;
 	break;
 
     case NML_DISPLAY_TYPE:
-	strncpy(operator_display_string,
+	cpy_str(operator_display_string,
 		(static_cast<NML_DISPLAY *>((emcErrorBuffer->get_address()))->display),
 		NML_DISPLAY_LEN - 1);
-	operator_display_string[NML_DISPLAY_LEN - 1] = 0;
 	break;
     }