From c85ac8bccf13f6aba024fef47453f3e1b6c3c615 Mon Sep 17 00:00:00 2001
From: "E. Scott Daniels" <daniels@research.att.com>
Date: Wed, 19 Aug 2020 09:51:33 -0400
Subject: [PATCH] Correct sonar bugs and address smells

Three bugs related to misplaced memory deallocation were fixed.
Several "smells" were addressed in message, messaging, config
and json source.

Issue-ID: RIC-629

Signed-off-by: E. Scott Daniels <daniels@research.att.com>
Change-Id: Ib2e7d51c96f3b4f88761af6b3058fc32d0005321
---
 CHANGES                         |   3 +
 CMakeLists.txt                  |   4 +-
 src/alarm/alarm.cpp             | 124 +++++++++++++++++-----------------------
 src/alarm/alarm.hpp             |  42 +++++++-------
 src/config/config.cpp           |  73 +++++++++++------------
 src/config/config.hpp           |  28 ++++-----
 src/config/config_cb.cpp        |  11 ++--
 src/config/config_cb.hpp        |   2 +-
 src/json/jhash.cpp              |  15 +++--
 src/json/jhash.hpp              |   6 +-
 src/json/jwrapper.c             |  26 ++++++---
 src/messaging/callback.cpp      |  10 ++--
 src/messaging/callback.hpp      |   4 +-
 src/messaging/default_cb.cpp    |   8 +++
 src/messaging/message.cpp       |  56 ++++++++++--------
 src/messaging/message.hpp       |  16 +++---
 src/messaging/messenger.cpp     |  31 +++++-----
 src/messaging/messenger.hpp     |  10 ++--
 src/messaging/msg_component.hpp |   2 +-
 19 files changed, 234 insertions(+), 237 deletions(-)

diff --git a/CHANGES b/CHANGES
index 864388f..bde4d75 100644
--- a/CHANGES
+++ b/CHANGES
@@ -6,6 +6,9 @@
 # squished to one.
 
 release = Cherry
+2020 20 August; version 2.3.0
+	Address sonar flagged "bugs" in the config class (RIC-629).
+
 2020 13 August; version 2.2.2
 	Correct potential memory leaks in xapp class (RIC-629)
 
diff --git a/CMakeLists.txt b/CMakeLists.txt
index b4f7012..88d09a9 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -29,8 +29,8 @@ project( ricxfcpp )
 cmake_minimum_required( VERSION 3.5 )
 
 set( major_version "2" )		# should be automatically populated from git tag later, but until CI process sets a tag we use this
-set( minor_version "2" )
-set( patch_level "2" )
+set( minor_version "3" )
+set( patch_level "0" )
 
 set( install_root "${CMAKE_INSTALL_PREFIX}" )
 set( install_inc "include/ricxfcpp" )
diff --git a/src/alarm/alarm.cpp b/src/alarm/alarm.cpp
index 2648ed1..fc13c7c 100644
--- a/src/alarm/alarm.cpp
+++ b/src/alarm/alarm.cpp
@@ -35,7 +35,9 @@
 
 #include <rmr/RIC_message_types.h>
 #ifndef RIC_ALARM
-	#define RIC_ALARM 110
+	// this _should_ come from the message types header, but if not ensure we have something
+	constexpr int ric_alarm_value = 110;
+	#define RIC_ALARM ric_alarm_value
 #endif
 
 #include <iostream>
@@ -44,7 +46,7 @@
 #include "message.hpp"
 #include "alarm.hpp"
 
-extern char* __progname;			// runtime lib supplied since we don't get argv[0]
+extern const char* __progname;			// runtime lib supplied since we don't get argv[0]
 
 namespace xapp {
 
@@ -63,7 +65,7 @@ namespace xapp {
 	then we assume 4560 (the defacto RMR listen port).
 */
 static std::string endpoint_addr( ) {
-	char*	et;									// environment token
+	const char*	et;									// environment token
 	std::string addr = "localhost";
 	std::string port = "4560";
 
@@ -98,8 +100,7 @@ static long long now( void ) {
 	Returns the length of the payload inserted.
 */
 int xapp::Alarm::build_alarm( int action_id, xapp::Msg_component payload, int payload_len ) {
-	//char	wbuf[4096];
-	std::string action;
+	std::string maction;				// message action is a text string
 	int used;
 
 	if( app_id.compare( "" ) == 0 ) {
@@ -112,20 +113,18 @@ int xapp::Alarm::build_alarm( int action_id, xapp::Msg_component payload, int pa
 
 	switch( action_id ) {
 		case	Alarm::ACT_CLEAR:
-			action = "CLEAR";
+			maction = "CLEAR";
 			break;
 
 		case	Alarm::ACT_CLEAR_ALL:
-			action = "CLEARALL";
+			maction = "CLEARALL";
 			break;
 
 		default:
-			action = "RAISE";
+			maction = "RAISE";
 			break;
 	}
 
-	//memset( wbuf, 0, sizeof( wbuf ) );
-	//snprintf( wbuf, sizeof( wbuf ),
 	used = snprintf( (char *) payload.get(), payload_len,
 			"{  "
 			"\"managedObjectId\": \"%s\", "
@@ -144,7 +143,7 @@ int xapp::Alarm::build_alarm( int action_id, xapp::Msg_component payload, int pa
 			severity.c_str(),
 			info.c_str(),
 			add_info.c_str(),
-			action.c_str(),
+			maction.c_str(),
 			now()
 	);
 
@@ -162,41 +161,23 @@ int xapp::Alarm::build_alarm( int action_id, xapp::Msg_component payload, int pa
 */
 xapp::Alarm::Alarm( std::shared_ptr<Message> msg ) :
 	msg( msg ),
-	endpoint( endpoint_addr() ),
-	whid( -1 ),
-	app_id( "" ),
-	me_id( "" ),
-	problem_id( -1 ),
-	info( "" ),
-	add_info( "" ),
-	action( "" )
+	endpoint( endpoint_addr() )
 { /* empty body */ }
 
 /*
 	Parameterised constructor (avoids calling setters after creation).
 */
-xapp::Alarm::Alarm( std::shared_ptr<Message> msg, int prob_id, std::string meid  ) :
+xapp::Alarm::Alarm( std::shared_ptr<Message> msg, int prob_id, const std::string& meid  ) :
 	msg( msg ),
 	endpoint( endpoint_addr() ),
-	whid( -1 ),
-	app_id( "" ),
 	me_id( meid ),
-	problem_id( prob_id ),
-	info( "" ),
-	add_info( "" ),
-	action( "" )
+	problem_id( prob_id )
 { /* empty body */ }
 
-xapp::Alarm::Alarm( std::shared_ptr<Message> msg, std::string meid  ) :
+xapp::Alarm::Alarm( std::shared_ptr<Message> msg, const std::string& meid  ) :
 	msg( msg ),
 	endpoint( endpoint_addr() ),
-	whid( -1 ),
-	app_id( "" ),
-	me_id( meid ),
-	problem_id( -1 ),
-	info( "" ),
-	add_info( "" ),
-	action( "" )
+	me_id( meid )
 { /* empty body */ }
 
 
@@ -207,18 +188,18 @@ xapp::Alarm::Alarm( std::shared_ptr<Message> msg, std::string meid  ) :
 	Copy builder.  Given a source object instance (soi), create a copy.
 	Creating a copy should be avoided as it can be SLOW!
 */
-xapp::Alarm::Alarm( const Alarm& soi ) {
-	msg = soi.msg;
-	endpoint = soi.endpoint;
-	whid = soi.whid;
-
-	me_id = soi.me_id;				// user stuff
-	app_id = soi.app_id;
-	problem_id = soi.problem_id;
-	severity = soi.severity;
-	info = soi.info;
-	add_info = soi.add_info;
-}
+xapp::Alarm::Alarm( const Alarm& soi ) :
+	msg(  soi.msg ),
+	endpoint(  soi.endpoint ),
+	whid(  soi.whid ),
+
+	me_id(  soi.me_id ),				// user stuff
+	app_id(  soi.app_id ),
+	problem_id(  soi.problem_id ),
+	severity(  soi.severity ),
+	info(  soi.info ),
+	add_info(  soi.add_info )
+{ /* empty body */ }
 
 /*
 	Assignment operator. Simiolar to the copycat, but "this" object exists and
@@ -229,7 +210,6 @@ Alarm& xapp::Alarm::operator=( const Alarm& soi ) {
 		msg = soi.msg;
 		endpoint = soi.endpoint;
 		whid = soi.whid;
-
 		me_id = soi.me_id;
 		app_id = soi.app_id;
 		problem_id = soi.problem_id;
@@ -246,17 +226,17 @@ Alarm& xapp::Alarm::operator=( const Alarm& soi ) {
 	the soi ensuring that the destriction of the soi doesn't trash things from
 	under us.
 */
-xapp::Alarm::Alarm( Alarm&& soi ) {
-	msg = soi.msg;		// capture pointers and copy data before setting soruce things to nil
-	endpoint = soi.endpoint;
-	whid = soi.whid;
-
-	me_id = soi.me_id;
-	app_id = soi.app_id;
-	problem_id = soi.problem_id;
-	severity = soi.severity;
-	info = soi.info;
-	add_info = soi.add_info;
+xapp::Alarm::Alarm( Alarm&& soi )  :
+	msg(  soi.msg ),					// order must match .hpp else sonar complains
+	endpoint(  soi.endpoint ),
+	whid(  soi.whid ),
+	me_id(  soi.me_id ),
+	app_id(  soi.app_id ),
+	problem_id(  soi.problem_id ),
+	severity(  soi.severity ),
+	info(  soi.info ),
+	add_info(  soi.add_info )
+{
 
 	soi.msg = NULL;		// prevent closing of RMR stuff on soi destroy
 }
@@ -298,7 +278,7 @@ xapp::Alarm::~Alarm() {
 
 // ---- setters -------------------------------------------------
 
-void xapp::Alarm::Set_meid( std::string new_meid ) {
+void xapp::Alarm::Set_meid( const std::string& new_meid ) {
 	me_id = new_meid;
 }
 
@@ -330,7 +310,7 @@ void xapp::Alarm::Set_severity( int new_sev ) {
 	}
 }
 
-void xapp::Alarm::Set_appid( std::string new_id ) {
+void xapp::Alarm::Set_appid( const std::string& new_id ) {
 	app_id = new_id;
 }
 
@@ -338,11 +318,11 @@ void xapp::Alarm::Set_problem( int new_id ) {
 	problem_id = new_id;
 }
 
-void xapp::Alarm::Set_info( std::string new_info ) {
+void xapp::Alarm::Set_info( const std::string& new_info ) {
 	info = new_info;
 }
 
-void xapp::Alarm::Set_additional( std::string new_info ) {
+void xapp::Alarm::Set_additional( const std::string& new_info ) {
 	add_info = new_info;
 }
 
@@ -350,7 +330,7 @@ void xapp::Alarm::Set_whid( int new_whid ) {
 	whid = new_whid;
 }
 
-void xapp::Alarm::Dump() {
+const void xapp::Alarm::Dump() {
 	fprintf( stderr, "Alarm: prob id: %d\n", problem_id );
 	fprintf( stderr, "Alarm: meid: %s\n", me_id.c_str() );
 	fprintf( stderr, "Alarm: app: %s\n", app_id.c_str() );
@@ -363,7 +343,7 @@ void xapp::Alarm::Dump() {
 /*
 	Return the enpoint address string we have.
 */
-std::string xapp::Alarm::Get_endpoint( ) {
+const std::string xapp::Alarm::Get_endpoint( ) {
 	return endpoint;
 }
 
@@ -384,16 +364,16 @@ bool xapp::Alarm::Raise( ) {
 	problem ID. Info and addional_info are user supplied data that is just passed
 	through.
 */
-bool xapp::Alarm::Raise( int severity, int problem, std::string info ) {
-	this->severity = severity;
+bool xapp::Alarm::Raise( int new_severity, int problem, const std::string& info ) {
+	Set_severity( new_severity );
 	problem_id = problem;
 	this->info = info;
 
 	Raise();
 }
 
-bool xapp::Alarm::Raise( int severity, int problem, std::string info, std::string additional_info ) {
-	this->severity = severity;
+bool xapp::Alarm::Raise( int new_severity, int problem, const std::string& info, const std::string& additional_info ) {
+	Set_severity( new_severity );
 	problem_id = problem;
 	this->info = info;
 	this->add_info = additional_info;
@@ -417,16 +397,16 @@ bool xapp::Alarm::Clear( ) {
 	problem ID. Info and addional_info are user supplied data that is just passed
 	through.
 */
-bool xapp::Alarm::Clear( int severity, int problem, std::string info ) {
-	this->severity = severity;
+bool xapp::Alarm::Clear( int new_severity, int problem, const std::string& info ) {
+	Set_severity( new_severity );
 	problem_id = problem;
 	this->info = info;
 
 	Clear();
 }
 
-bool xapp::Alarm::Clear( int severity, int problem, std::string info, std::string additional_info ) {
-	this->severity = severity;
+bool xapp::Alarm::Clear( int new_severity, int problem, const std::string& info, const std::string& additional_info ) {
+	Set_severity( new_severity );
 	problem_id = problem;
 	this->info = info;
 	this->add_info = additional_info;
diff --git a/src/alarm/alarm.hpp b/src/alarm/alarm.hpp
index 539e385..18b9ba2 100644
--- a/src/alarm/alarm.hpp
+++ b/src/alarm/alarm.hpp
@@ -44,17 +44,17 @@ class Alarm {
 	private:
 		std::shared_ptr<Message> msg;		// message to send
 		std::shared_ptr<char> psp;			// shared pointer to the payload to give out
-		std::string endpoint;				// the ip:port addr:port of the alarm collector
-		int			whid;
+		std::string endpoint =  "";			// the ip:port addr:port of the alarm collector
+		int			whid = -1;
 
 											// data for the payload
-		std::string	me_id;					// managed element ID
-		std::string app_id;					// application ID
-		int			problem_id;				// problem ID (specific problem)
-		std::string	severity;				// SEV_* constants
-		std::string	action;					// ACT_* constants
-		std::string info;					// info string supplied by user
-		std::string add_info;				// additional information supplied by user
+		std::string	me_id = "";				// managed element ID
+		std::string app_id = "";			// application ID
+		int			problem_id  = -1;		// problem ID (specific problem)
+		std::string	severity = "";			// set_sev() xlates from SEV_* consts to collector's string values
+		int			action = 0;				// ACT_* constants
+		std::string info = "";				// info string supplied by user
+		std::string add_info = "";			// additional information supplied by user
 
 		int build_alarm( int action_id, xapp::Msg_component payload, int payload_len );
 
@@ -71,8 +71,8 @@ class Alarm {
 		static const int	ACT_CLEAR_ALL = 3;
 
 		Alarm( std::shared_ptr<Message> msg );		// builders
-		Alarm( std::shared_ptr<Message> msg, std::string meid  );
-		Alarm( std::shared_ptr<Message> msg, int prob_id, std::string meid  );
+		Alarm( std::shared_ptr<Message> msg, const std::string& meid  );
+		Alarm( std::shared_ptr<Message> msg, int prob_id, const std::string& meid  );
 
 		Alarm( const Alarm& soi );					// copy to newly created instance
 		Alarm& operator=( const Alarm& soi );		// copy operator
@@ -81,29 +81,29 @@ class Alarm {
 		~Alarm();									// destroyer
 
 
-		std::string Get_endpoint( );
+		const std::string Get_endpoint( );
 
-		void Set_additional( std::string new_info );
-		void Set_appid( std::string new_id );
-		void Set_info( std::string new_info );
-		void Set_meid( std::string new_meid );
+		void Set_additional( const std::string& new_info );
+		void Set_appid( const std::string& new_id );
+		void Set_info( const std::string& new_info );
+		void Set_meid( const std::string& new_meid );
 		void Set_problem( int new_id );
 		void Set_severity( int new_sev );
 		void Set_whid( int whid );
 
 
 		bool Raise( );
-		bool Raise( int severity, int problem, std::string info );
-		bool Raise( int severity, int problem, std::string info, std::string addional_info );
+		bool Raise( int severity, int problem, const std::string& info );
+		bool Raise( int severity, int problem, const std::string& info, const std::string& additional_info );
 		bool Raise_again( );
 
 		bool Clear( );
-		bool Clear( int severity, int problem, std::string info );
-		bool Clear( int severity, int problem, std::string info, std::string addional_info );
+		bool Clear( int severity, int problem, const std::string& info );
+		bool Clear( int severity, int problem, const std::string& info, const std::string& additional_info );
 		bool Clear_all( );
 
 
-		void Dump();
+		const void Dump();
 };
 
 } // namespace
diff --git a/src/config/config.cpp b/src/config/config.cpp
index 9cfa420..21b9e65 100644
--- a/src/config/config.cpp
+++ b/src/config/config.cpp
@@ -71,10 +71,10 @@ namespace xapp {
 	and not directory entries.
 */
 void xapp::Config::Listener( ) {
-	struct inotify_event*	ie;		// event that popped
+	const struct inotify_event*	ie;	// event that popped
 	int ifd;						// the inotify file des
 	int	wfd;						// the watched file des
-	int	n;
+	ssize_t	n;
 	char	rbuf[4096];				// large read buffer as the event is var len
 	char*	dname;					// directory name
 	char*	bname;					// basename
@@ -96,10 +96,12 @@ void xapp::Config::Listener( ) {
 		bname = strdup( fname.c_str() );
 	}
 
-	wfd = inotify_add_watch( ifd, (char *) dname, IN_MOVED_TO | IN_CLOSE_WRITE );		// we only care about close write changes
+	wfd = inotify_add_watch( ifd, dname, IN_MOVED_TO | IN_CLOSE_WRITE );		// we only care about close write changes
+	free( dname );
 
 	if( wfd < 0 ) {
 		fprintf( stderr, "<XFCPP> ### ERR ### unable to add watch on config file %s: %s\n", fname.c_str(), strerror( errno ) );
+		free( bname );
 		return;
 	}
 
@@ -116,9 +118,9 @@ void xapp::Config::Listener( ) {
 
 		ie = (inotify_event *) rbuf;
 		if( ie->len > 0 && strcmp( bname, ie->name ) == 0  ) {
-			// TODO: lock
+			// future: lock
 			auto njh = jparse( fname );							// reparse the file
-			// TODO: unlock
+			// future: unlock
 
 			if( njh != NULL && cb != NULL ) {				// good parse, save and drive user callback
 				jh = njh;
@@ -126,9 +128,6 @@ void xapp::Config::Listener( ) {
 			}
 		}
 	}
-
-	free( dname );
-	free( bname );
 }
 
 
@@ -162,10 +161,10 @@ std::shared_ptr<xapp::Jhash> xapp::Config::jparse( std::string ufname ) {
 	if it's a directory name or not if it comes to it.
 */
 std::shared_ptr<xapp::Jhash> xapp::Config::jparse( ) {
-	char*	data;
+	const char*	data;
 
-	if( (data = getenv( (char *) "XAPP_DESCRIPTOR_PATH" )) == NULL ) {
-		data =  (char *) "./config-file.json";
+	if( (data = getenv( (const char *) "XAPP_DESCRIPTOR_PATH" )) == NULL ) {
+		data = (const char *) "./config-file.json";
 	}
 
 	return jparse( std::string( data ) );
@@ -182,16 +181,14 @@ std::shared_ptr<xapp::Jhash> xapp::Config::jparse( ) {
 	the common things should be fairly painless to extract from the json goop.
 */
 xapp::Config::Config() :
-	jh( jparse() ),
-	listener( NULL )
+	jh( jparse() )
 { /* empty body */ }
 
 /*
 	Similar, except that it allows the xAPP to supply the filename (testing?)
 */
-xapp::Config::Config( std::string fname) :
-	jh( jparse( fname ) ),
-	listener( NULL )
+xapp::Config::Config( const std::string& fname) :
+	jh( jparse( fname ) )
 { /* empty body */ }
 
 
@@ -199,7 +196,7 @@ xapp::Config::Config( std::string fname) :
 	Read and return the raw file blob as a single string. User can parse, or do
 	whatever they need (allows non-json things if necessary).
 */
-std::string xapp::Config::Get_contents( ) {
+std::string xapp::Config::Get_contents( ) const {
 	std::string rv = "";
 
 	if( ! fname.empty() ) {
@@ -218,7 +215,7 @@ std::string xapp::Config::Get_contents( ) {
 	Suss out the port for the named "interface". The interface is likely the application
 	name.
 */
-std::string xapp::Config::Get_port( std::string name ) {
+std::string xapp::Config::Get_port( const std::string& name ) const {
 	int i;
 	int	nele = 0;
 	double value;
@@ -230,13 +227,13 @@ std::string xapp::Config::Get_port( std::string name ) {
 	}
 
 	jh->Unset_blob();
-	if( jh->Set_blob( (char *) "messaging" ) ) {
-		nele = jh->Array_len( (char *) "ports" );
+	if( jh->Set_blob( (const char *) "messaging" ) ) {
+		nele = jh->Array_len( (const char *) "ports" );
 		for( i = 0; i < nele; i++ ) {
-			if( jh->Set_blob_ele( (char *) "ports", i ) ) {
-				pname = jh->String( (char *) "name" );
+			if( jh->Set_blob_ele( (const char *) "ports", i ) ) {
+				pname = jh->String( (const char *) "name" );
 				if( pname.compare( name ) == 0 ) {				// this element matches the name passed in
-					value = jh->Value( (char *) "port" );
+					value = jh->Value( (const char *) "port" );
 					rv = std::to_string( (int) value );
 					jh->Unset_blob( );							// leave hash in a known state
 					return rv;
@@ -244,7 +241,7 @@ std::string xapp::Config::Get_port( std::string name ) {
 			}
 
 			jh->Unset_blob( );								// Jhash requires bump to root, and array reselct to move to next ele
-			jh->Set_blob( (char *) "messaging" );
+			jh->Set_blob( (const char *) "messaging" );
 		}
 	}
 
@@ -256,7 +253,7 @@ std::string xapp::Config::Get_port( std::string name ) {
 	Suss out the named string from the controls object. If the resulting value is
 	missing or "", then the default is returned.
 */
-std::string xapp::Config::Get_control_str( std::string name, std::string defval ) {
+std::string xapp::Config::Get_control_str( const std::string& name, const std::string& defval ) const {
 	std::string value;
 	std::string rv;				// result value
 
@@ -266,7 +263,7 @@ std::string xapp::Config::Get_control_str( std::string name, std::string defval
 	}
 
 	jh->Unset_blob();
-	if( jh->Set_blob( (char *) "controls" ) ) {
+	if( jh->Set_blob( (const char *) "controls" ) ) {
 		if( jh->Exists( name.c_str() ) )  {
 			value = jh->String( name.c_str() );
 			if( value.compare( "" ) != 0 ) {
@@ -283,7 +280,7 @@ std::string xapp::Config::Get_control_str( std::string name, std::string defval
 	Convenience funciton without default. "" returned if not found.
 	No default value; returns "" if not set.
 */
-std::string xapp::Config::Get_control_str( std::string name ) {
+std::string xapp::Config::Get_control_str( const std::string& name ) const {
 	return Get_control_str( name, "" );
 }
 
@@ -291,8 +288,7 @@ std::string xapp::Config::Get_control_str( std::string name ) {
 	Suss out the named field from the controls object with the assumption that it is a boolean.
 	If the resulting value is missing then the defval is used.
 */
-bool xapp::Config::Get_control_bool( std::string name, bool defval ) {
-	bool value;
+bool xapp::Config::Get_control_bool( const std::string& name, bool defval ) const {
 	bool rv;				// result value
 
 	rv = defval;
@@ -301,10 +297,8 @@ bool xapp::Config::Get_control_bool( std::string name, bool defval ) {
 	}
 
 	jh->Unset_blob();
-	if( jh->Set_blob( (char *) "controls" ) ) {
-		if( jh->Exists( name.c_str() ) )  {
-			rv = jh->Bool( name.c_str() );
-		}
+	if( jh->Set_blob( (const char *) "controls" )  &&  jh->Exists( name.c_str() ) )  {
+		rv = jh->Bool( name.c_str() );
 	}
 
 	jh->Unset_blob();
@@ -315,7 +309,7 @@ bool xapp::Config::Get_control_bool( std::string name, bool defval ) {
 /*
 	Convenience function without default.
 */
-bool xapp::Config::Get_control_bool( std::string name ) {
+bool xapp::Config::Get_control_bool( const std::string& name ) const {
 	return Get_control_bool( name, false );
 }
 
@@ -324,8 +318,7 @@ bool xapp::Config::Get_control_bool( std::string name ) {
 	Suss out the named field from the controls object with the assumption that it is a value (float/int).
 	If the resulting value is missing then the defval is used.
 */
-double xapp::Config::Get_control_value( std::string name, double defval ) {
-	double value;
+double xapp::Config::Get_control_value( const std::string& name, double defval ) const {
 
 	auto rv = defval;				// return value; set to default
 	if( jh == NULL ) {
@@ -333,10 +326,8 @@ double xapp::Config::Get_control_value( std::string name, double defval ) {
 	}
 
 	jh->Unset_blob();
-	if( jh->Set_blob( (char *) "controls" ) ) {
-		if( jh->Exists( name.c_str() ) )  {
-			rv = jh->Value( name.c_str() );
-		}
+	if( jh->Set_blob( (const char *) "controls" )  && jh->Exists( name.c_str() ) )  {
+		rv = jh->Value( name.c_str() );
 	}
 
 	jh->Unset_blob();
@@ -347,7 +338,7 @@ double xapp::Config::Get_control_value( std::string name, double defval ) {
 /*
 	Convenience function. If value is undefined, then 0 is returned.
 */
-double xapp::Config::Get_control_value( std::string name ) {
+double xapp::Config::Get_control_value( const std::string& name ) const {
 	return Get_control_value( name, 0.0 );
 }
 
diff --git a/src/config/config.hpp b/src/config/config.hpp
index 5dac6a3..88234dc 100644
--- a/src/config/config.hpp
+++ b/src/config/config.hpp
@@ -42,12 +42,12 @@ namespace xapp {
 #define MAX_PFNAME	(4096 + 256)		// max path name and max filname + nil for buffer allocation
 
 class Config {
-	std::string	fname;				// the file name that we'll listen to
-	std::thread* listener;			// listener thread info
+	std::string	fname = "";					// the file name that we'll listen to
+	std::thread* listener = NULL;			// listener thread info
 
-	std::shared_ptr<Jhash>	jh;		// the currently parsed json from the config
-	std::unique_ptr<Config_cb> cb;	// info needed to drive user code when config change noticed
-	void*	user_cb_data;			// data that the caller wants passed on notification callback
+	std::shared_ptr<Jhash>	jh = NULL;		// the currently parsed json from the config
+	std::unique_ptr<Config_cb> cb = NULL;	// info needed to drive user code when config change noticed
+	void*	user_cb_data = NULL;			// data that the caller wants passed on notification callback
 
 	// -----------------------------------------------------------------------
 	private:
@@ -57,20 +57,20 @@ class Config {
 
 	public:
 		Config();						// builders
-		Config( std::string fname);
+		Config( const std::string& fname);
 
-		bool Get_control_bool( std::string name, bool defval );
-		bool Get_control_bool( std::string name );
+		bool Get_control_bool( const std::string& name, bool defval ) const;
+		bool Get_control_bool( const std::string& name ) const;
 
-		std::string Get_contents( );
+		std::string Get_contents( ) const;
 
-		std::string Get_control_str( std::string name, std::string defval );
-		std::string Get_control_str( std::string name );
+		std::string Get_control_str( const std::string& name, const std::string& defval ) const;
+		std::string Get_control_str( const std::string& name ) const;
 
-		double Get_control_value( std::string name, double defval );
-		double Get_control_value( std::string name );
+		double Get_control_value( const std::string& name, double defval ) const;
+		double Get_control_value( const std::string& name ) const;
 
-		std::string Get_port( std::string name );
+		std::string Get_port( const std::string& name ) const;
 
 		void Set_callback( notify_callback usr_func, void* usr_data );
 };
diff --git a/src/config/config_cb.cpp b/src/config/config_cb.cpp
index 309ddba..9fd417b 100644
--- a/src/config/config_cb.cpp
+++ b/src/config/config_cb.cpp
@@ -37,9 +37,9 @@ namespace xapp {
 /*
 	Builder.
 */
-Config_cb::Config_cb( notify_callback ufun, void* udata ) :
+Config_cb::Config_cb( notify_callback ufun, void* user_data ) :
 		user_fun( ufun ),
-		udata( udata )
+		udata( user_data )
 { /* empty body */  }
 
 
@@ -50,13 +50,14 @@ Config_cb::Config_cb( notify_callback ufun, void* udata ) :
 
 /*
 	Drive_cb will invoke the callback and pass along the stuff passed here.
+	User_data may be nil which causes the data registered with the callback
+	to be used.
 */
-void xapp::Config_cb::Drive_cb( xapp::Config& c, void* udata ) {
+void xapp::Config_cb::Drive_cb( xapp::Config& c, void* user_data ) const {
 	if( user_fun != NULL ) {
-		user_fun( c, udata );
+		user_fun( c, user_data == NULL ? udata : user_data );
 	}
 }
 
 
-
 } // namespace
diff --git a/src/config/config_cb.hpp b/src/config/config_cb.hpp
index f94d129..e71dac8 100644
--- a/src/config/config_cb.hpp
+++ b/src/config/config_cb.hpp
@@ -55,7 +55,7 @@ class Config_cb {
 
 	public:
 		Config_cb( notify_callback cbfun, void* user_data );		// builder
-		void Drive_cb( xapp::Config& c, void* udata );				// invoker
+		void Drive_cb( xapp::Config& c, void* user_data ) const;	// invokers
 };
 
 } // namespace
diff --git a/src/json/jhash.cpp b/src/json/jhash.cpp
index d81c617..25efc45 100644
--- a/src/json/jhash.cpp
+++ b/src/json/jhash.cpp
@@ -49,7 +49,6 @@ namespace xapp {
 	suss out the values.
 */
 xapp::Jhash::Jhash( const char* jbuf ) :
-	master_st( NULL ),
 	st( jw_new( jbuf ) )
 { /* empty body */ }
 
@@ -57,10 +56,10 @@ xapp::Jhash::Jhash( const char* jbuf ) :
 /*
 	Move constructor.
 */
-Jhash::Jhash( Jhash&& soi ) {
-	master_st = soi.master_st;
-	st = soi.st;
-
+Jhash::Jhash( Jhash&& soi ) :
+	master_st( soi.master_st ),
+	st( soi.st )
+{
 	soi.st = NULL;						// prevent closing of RMR stuff on soi destroy
 	soi.master_st = NULL;
 }
@@ -142,7 +141,7 @@ void xapp::Jhash::Unset_blob( ) {
 	Right now we don't have much to work with other than checking for a
 	nil table.
 */
-bool xapp::Jhash::Parse_errors( ) {
+const bool xapp::Jhash::Parse_errors( ) {
 	return st == NULL;
 }
 
@@ -232,7 +231,7 @@ bool xapp::Jhash::Bool( const char* name ) {
 */
 std::string xapp::Jhash::String( const char* name ) {
 	std::string rv = "";
-	char*	hashv;
+	const char*	hashv;
 
 	if( (hashv = jw_string( st, name )) != NULL ) {
 		rv = std::string( hashv );
@@ -282,7 +281,7 @@ bool xapp::Jhash::Set_blob_ele( const char* name, int eidx ) {
 */
 std::string xapp::Jhash::String_ele( const char* name, int eidx ) {
 	std::string rv = "";
-	char*	hashv;
+	const char*	hashv;
 
 	if( (hashv = jw_string_ele( st, name, eidx )) != NULL ) {
 		rv = std::string( hashv );
diff --git a/src/json/jhash.hpp b/src/json/jhash.hpp
index 6ba1e50..8394acf 100644
--- a/src/json/jhash.hpp
+++ b/src/json/jhash.hpp
@@ -40,8 +40,8 @@ namespace xapp {
 
 class Jhash {
 	private:
-		void*	st;							// the resulting symbol table generated by parse
-		void*	master_st;					// if user switches to a sub-blob; this tracks the original root st
+		void*	st = NULL;							// the resulting symbol table generated by parse
+		void*	master_st = NULL;					// if user switches to a sub-blob; this tracks the original root st
 
 		Jhash& operator=( const Jhash& soi );	// jhashes cannot be copied because of underlying symbol table goo
 		Jhash( const Jhash& soi );
@@ -57,7 +57,7 @@ class Jhash {
 		void Unset_blob( );
 		bool Set_blob_ele( const char* name, int eidx );	// set from an array element
 
-		bool Parse_errors( );
+		const bool Parse_errors( );
 		void Dump();
 
 		std::string String( const char* name );				// value fetching
diff --git a/src/json/jwrapper.c b/src/json/jwrapper.c
index 510899b..865856a 100644
--- a/src/json/jwrapper.c
+++ b/src/json/jwrapper.c
@@ -88,7 +88,7 @@ typedef struct jthing {
 	we don't create a bunch of small buffers that must be found and freed; we
 	can just release the json string and we'll be done (read won't leak).
 */
-static char* extract( char* buf, jsmntok_t *jtoken ) {
+static char* extract( char* buf, const jsmntok_t *jtoken ) {
 	buf[jtoken->end] = 0;
 	return &buf[jtoken->start];
 }
@@ -160,8 +160,8 @@ static jthing_t* suss_element( void* st, const char* name, int idx ) {
 	if(    (jtp = suss_array( st, name )) != NULL		// have pointer
 		&& idx >= 0										// and in range
 		&& idx < jtp->nele
-		&& (jarray = jtp->v.pv) != NULL ) {				// and exists
-
+		&& jtp->v.pv != NULL ) {						// and exists
+			jarray = jtp->v.pv;
 			rv = &jarray[idx];
 	}
 
@@ -175,6 +175,10 @@ static jthing_t* suss_element( void* st, const char* name, int idx ) {
 
 	Only the element passed is used, but this is a prototype which is required by
 	the RMR symtab implementaion, so we play games in the code to keep sonar quiet.
+
+
+	Sonar will grumble aobut the parms needing to be marked const. Until RMR changes
+	the signature we can't and sonar will just have to unbunch its knickers.
 */
 static void nix_things( void* st, void* se, const char* name, void* ele, void *data ) {
 	jthing_t*	j;
@@ -224,6 +228,9 @@ static void nix_things( void* st, void* se, const char* name, void* ele, void *d
 
 	Silly games played to keep sonar from complaining. This is driven by RMR
 	symtab code which defines the set of params and we use what we need.
+
+	Sonar will grumble aobut the parms needing to be marked const. Until RMR changes
+	the signature we can't and sonar will just have to unbunch its knickers.
 */
 static void nix_mgt( void* st, void* se, const char* name,  void* ele, void *data ) {
 
@@ -241,6 +248,9 @@ static void nix_mgt( void* st, void* se, const char* name,  void* ele, void *dat
 /*
 	Invoked for each thing and prints what we can to stderr.
 	Most parms ignored, but symtab code in RMR defines the prototype so they are required.
+
+	Sonar will grumble aobut the parms needing to be marked const. Until RMR changes
+	the signature we can't and sonar will just have to unbunch its knickers.
 */
 static void dump_things( void* st, void* se, const char* name,  void* ele, void *data ) {
 	const jthing_t*	j;
@@ -274,12 +284,10 @@ void* parse_jobject( void* st, char *json, char* prefix ) {
 	char	*data;				// data string from the json
 	jthing_t*	jarray;			// array of jthings we'll coonstruct
 	int		size;
-	int		osize;
 	int		njtokens;			// tokens actually sussed out
 	jsmn_parser jp;				// 'parser' object
 	jsmntok_t *jtokens;			// pointer to tokens returned by the parser
 	char	pname[1024];		// name with prefix
-	char	wbuf[256];			// temp buf to build a working name in
 	char*	dstr;				// dup'd string
 	int		step = 0;			// parsing step value to skip tokens picked up
 	int		data_idx;			// index into tokens for the next bit of data
@@ -342,9 +350,11 @@ void* parse_jobject( void* st, char *json, char* prefix ) {
 			case JSMN_OBJECT:				// save object in two ways: as an object 'blob' and in the current symtab using name as a base (original)
 				if( DEBUG ) fprintf( stderr, "<DBUG> [%d] %s (object) has %d things\n",  data_idx, name, jtokens[data_idx].size );
 
-				if( (jtp = mk_thing( st, name, jtokens[data_idx].type )) != NULL &&		// create thing and reference it in current symtab
-					(jtp->v.pv = (void *) rmr_sym_alloc( 255 ) ) != NULL ) {			// object is just a blob
+				if( (jtp = mk_thing( st, name, jtokens[data_idx].type )) != NULL ) {	// create thing and reference it
+					jtp->v.pv =  rmr_sym_alloc( 255 );									// object is just a blob; make it
+				}
 
+				if( jtp != NULL && jtp->v.pv != NULL ) {				// double check allows for better coverage and keeps sonar happy
 					dstr = strdup( extract( json, &jtokens[data_idx] ) );
 					rmr_sym_put( jtp->v.pv, JSON_SYM_NAME, MGT_SPACE, dstr );		// must stash json so it is freed during nuke()
 					parse_jobject( jtp->v.pv,  dstr, "" );							// recurse across the object and build a new symtab
@@ -388,7 +398,7 @@ void* parse_jobject( void* st, char *json, char* prefix ) {
 
 					switch( jtokens[data_idx].type ) {
 						case JSMN_OBJECT:
-							jarray[n].v.pv = (void *) rmr_sym_alloc( 255 );
+							jarray[n].v.pv = rmr_sym_alloc( 255 );
 							if( DEBUG ) fprintf( stderr, "<DBUG> %s[%d] is object size=%d\n", name, n, jtokens[data_idx].size );
 							if( jarray[n].v.pv != NULL ) {
 								jarray[n].jsmn_type = JSMN_OBJECT;
diff --git a/src/messaging/callback.cpp b/src/messaging/callback.cpp
index 9fd603f..6f1ed62 100644
--- a/src/messaging/callback.cpp
+++ b/src/messaging/callback.cpp
@@ -36,10 +36,10 @@ namespace xapp {
 /*
 	Builder.
 */
-Callback::Callback( user_callback ufun, void* data ) {		// builder
-	user_fun = ufun;
-	udata = data;
-}
+Callback::Callback( user_callback ufun, void* data ) :		// builder
+	user_fun( ufun ),
+	udata( data )
+{ /* empty body */ }
 
 /*
 	there is nothing to be done from a destruction perspective, so no
@@ -51,7 +51,7 @@ Callback::Callback( user_callback ufun, void* data ) {		// builder
 */
 void xapp::Callback::Drive_cb( Message& m ) {
 	if( user_fun != NULL ) {
-		user_fun( m, m.Get_mtype(), m.Get_subid(), m.Get_len(), m.Get_payload(),  udata );
+		user_fun( m, m.Get_mtype(), m.Get_subid(), m.Get_len(), m.Get_payload(), udata );
 	}
 }
 
diff --git a/src/messaging/callback.hpp b/src/messaging/callback.hpp
index 0c892a8..b35d0b4 100644
--- a/src/messaging/callback.hpp
+++ b/src/messaging/callback.hpp
@@ -48,8 +48,8 @@ class Callback {
 		void*	udata;									// user data
 
 	public:
-		Callback( user_callback, void* data );					// builder
-		void Drive_cb( Message& m );							// invoker
+		Callback( user_callback, void* data );			// builder
+		void Drive_cb( Message& m );					// invoker
 };
 
 } // namespace
diff --git a/src/messaging/default_cb.cpp b/src/messaging/default_cb.cpp
index 2156ad3..681f5f6 100644
--- a/src/messaging/default_cb.cpp
+++ b/src/messaging/default_cb.cpp
@@ -48,10 +48,18 @@ namespace xapp {
 
 	The mr paramter is obviously ignored, but to add this as a callback
 	the function sig must match.
+
+	This is a callback function; sonar will complain that we don't use payload or
+	data -- we don't, but this is a standard function proto so we cannot just
+	drop them.
 */
 void Health_ck_cb( Message& mbuf, int mtype, int sid, int len, Msg_component payload, void* data ) {
 	unsigned char response[128];
 
+	if( len < 0  || mtype < 0 ) {
+		return;
+	}
+
 	snprintf( (char* ) response, sizeof( response ), "OK\n" );
 	mbuf.Send_response( RIC_HEALTH_CHECK_RESP, sid, strlen( (char *) response )+1, response );
 }
diff --git a/src/messaging/message.cpp b/src/messaging/message.cpp
index a8b8089..30c7548 100644
--- a/src/messaging/message.cpp
+++ b/src/messaging/message.cpp
@@ -52,24 +52,27 @@ namespace xapp {
 /*
 	Create a new message wrapper for an existing RMR msg buffer.
 */
-xapp::Message::Message( rmr_mbuf_t* mbuf, void* mrc ) {
-	this->mrc = mrc;			// the message router context for sends
-	this->mbuf = mbuf;
-}
-
-xapp::Message::Message( void* mrc, int payload_len ) {
-	this->mrc = mrc;
-	this->mbuf = rmr_alloc_msg( mrc, payload_len );
-}
+xapp::Message::Message( rmr_mbuf_t* mbuf, void* mrc ) :
+	mrc(  mrc ),			// the message router context for sends
+	mbuf(  mbuf )
+{  /* empty body */ }
+
+xapp::Message::Message( void* mrctx, int payload_len ) :
+	mrc(  mrctx ),										// the message router context for sends
+	mbuf(  rmr_alloc_msg( mrc, payload_len ) )
+{ /* empty body */ }
+//	this->mrc = mrc;
+	//this->mbuf = rmr_alloc_msg( mrc, payload_len );
 
 /*
 	Copy builder.  Given a source object instance (soi), create a copy.
 	Creating a copy should be avoided as it can be SLOW!
 */
-xapp::Message::Message( const Message& soi ) {
+xapp::Message::Message( const Message& soi ) :
+	mrc( soi.mrc )
+{
 	int payload_size;
 
-	mrc = soi.mrc;
 	payload_size = rmr_payload_size( soi.mbuf );		// rmr can handle a nil pointer
 	mbuf = rmr_realloc_payload( soi.mbuf, payload_size, RMR_COPY, RMR_CLONE );
 }
@@ -99,10 +102,10 @@ Message& xapp::Message::operator=( const Message& soi ) {
 	the soi ensuring that the destriction of the soi doesn't trash things from
 	under us.
 */
-xapp::Message::Message( Message&& soi ) {
-	mrc = soi.mrc;
-	mbuf = soi.mbuf;
-
+xapp::Message::Message( Message&& soi ) :
+	mrc(  soi.mrc ),
+	mbuf(  soi.mbuf )
+{
 	soi.mrc = NULL;		// prevent closing of RMR stuff on soi destroy
 	soi.mbuf = NULL;
 }
@@ -165,7 +168,7 @@ std::unique_ptr<unsigned char> xapp::Message::Copy_payload( ){
 /*
 	Makes a copy of the MEID and returns a smart pointer to it.
 */
-std::unique_ptr<unsigned char> xapp::Message::Get_meid(){
+std::unique_ptr<unsigned char> xapp::Message::Get_meid() const {
 	unsigned char* m = NULL;
 
 	m = (unsigned char *) malloc( sizeof( unsigned char ) * RMR_MAX_MEID );
@@ -180,11 +183,11 @@ std::unique_ptr<unsigned char> xapp::Message::Get_meid(){
 	If mbuf isn't valid (nil, or message has a broken header) the return
 	will be -1.
 */
-int xapp::Message::Get_available_size(){
+int xapp::Message::Get_available_size() const {
 	return rmr_payload_size( mbuf );		// rmr can handle a nil pointer
 }
 
-int	xapp::Message::Get_mtype(){
+int	xapp::Message::Get_mtype() const {
 	int rval = INVALID_MTYPE;
 
 	if( mbuf != NULL ) {
@@ -197,8 +200,8 @@ int	xapp::Message::Get_mtype(){
 /*
 	Makes a copy of the source field and returns a smart pointer to it.
 */
-std::unique_ptr<unsigned char> xapp::Message::Get_src(){
-	unsigned char* m = new unsigned char[RMR_MAX_SRC]; 
+std::unique_ptr<unsigned char> xapp::Message::Get_src() const {
+	unsigned char* m = new unsigned char[RMR_MAX_SRC];
 
 	if( m != NULL ) {
 		rmr_get_src( mbuf, m );
@@ -207,7 +210,7 @@ std::unique_ptr<unsigned char> xapp::Message::Get_src(){
 	return std::unique_ptr<unsigned char>( m );
 }
 
-int	xapp::Message::Get_state( ){
+int	xapp::Message::Get_state( ) const {
 	int state = INVALID_STATUS;
 
 	if( mbuf != NULL ) {
@@ -217,7 +220,7 @@ int	xapp::Message::Get_state( ){
 	return state;
 }
 
-int	xapp::Message::Get_subid(){
+int	xapp::Message::Get_subid() const {
 	int	rval = INVALID_SUBID;
 
 	if( mbuf != NULL ) {
@@ -231,7 +234,7 @@ int	xapp::Message::Get_subid(){
 	Return the amount of the payload (bytes) which is used. See
 	Get_available_size() to get the total usable space in the payload.
 */
-int	xapp::Message::Get_len(){
+int	xapp::Message::Get_len() const {
 	int rval = 0;
 
 	if( mbuf != NULL ) {
@@ -249,7 +252,7 @@ int	xapp::Message::Get_len(){
 	length by calling Message:Get_available_size(), and ensuring that
 	writing beyond the indicated size does not happen.
 */
-Msg_component xapp::Message::Get_payload(){
+Msg_component xapp::Message::Get_payload() const {
 	if( mbuf != NULL ) {
 		return std::unique_ptr<unsigned char, unfreeable>( mbuf->payload );
 	}
@@ -259,7 +262,7 @@ Msg_component xapp::Message::Get_payload(){
 
 void xapp::Message::Set_meid( std::shared_ptr<unsigned char> new_meid ) {
 	if( mbuf != NULL ) {
-		rmr_str2meid( mbuf, (unsigned char *) new_meid.get() );
+		rmr_str2meid( mbuf, new_meid.get() );
 	}
 }
 
@@ -362,6 +365,9 @@ bool xapp::Message::Send( int mtype, int subid, int payload_len, unsigned char*
 			case WORMHOLE_MSG:
 				mbuf = rmr_wh_send_msg( mrc, whid,  mbuf );
 				break;
+
+			default:
+				break;					// because sonar doesn't like defaultless switches even when there is no work :(
 		}
 
 		state = mbuf->state == RMR_OK;
diff --git a/src/messaging/message.hpp b/src/messaging/message.hpp
index df71d88..838d9ee 100644
--- a/src/messaging/message.hpp
+++ b/src/messaging/message.hpp
@@ -81,14 +81,14 @@ class Message {
 
 		std::unique_ptr<unsigned char>  Copy_payload( );		// copy the payload; deletable smart pointer
 
-		std::unique_ptr<unsigned char> Get_meid();				// returns a copy of the meid bytes
-		int Get_available_size();
-		int	Get_len();
-		int	Get_mtype();
-		Msg_component Get_payload();
-		std::unique_ptr<unsigned char>  Get_src();
-		int	Get_state( );
-		int	Get_subid();
+		std::unique_ptr<unsigned char> Get_meid() const;				// returns a copy of the meid bytes
+		int Get_available_size() const;
+		int Get_len() const;
+		int Get_mtype() const;
+		Msg_component Get_payload() const;
+		std::unique_ptr<unsigned char>  Get_src() const;
+		int	Get_state( ) const;
+		int	Get_subid() const;
 
 		void Set_meid( std::shared_ptr<unsigned char> new_meid );
 		void Set_mtype( int new_type );
diff --git a/src/messaging/messenger.cpp b/src/messaging/messenger.cpp
index ede76d3..446c3c4 100644
--- a/src/messaging/messenger.cpp
+++ b/src/messaging/messenger.cpp
@@ -90,13 +90,13 @@ xapp::Messenger::Messenger( const char* uport, bool wait4table ) {
 	the new object, and then DELETE what was moved so that when the
 	user frees the soi, it doesn't destroy what we snarfed.
 */
-xapp::Messenger::Messenger( Messenger&& soi ) {
-	mrc = soi.mrc;
-	listen_port = soi.listen_port;
-	ok_2_run = soi.ok_2_run;
-	gate = soi.gate;
-		cb_hash = soi.cb_hash;				// this seems dodgy
-
+xapp::Messenger::Messenger( Messenger&& soi ) :
+	mrc(  soi.mrc ),
+	listen_port(  soi.listen_port ),
+	ok_2_run(  soi.ok_2_run ),
+	gate(  soi.gate ),
+	cb_hash(  soi.cb_hash )			// this seems dodgy
+{
 	soi.gate = NULL;
 	soi.listen_port = NULL;
 	soi.mrc = NULL;
@@ -112,7 +112,7 @@ xapp::Messenger& Messenger::operator=( Messenger&& soi ) {
 			rmr_close( mrc );
 		}
 		if( listen_port != NULL ) {
-			free( listen_port );
+			delete( listen_port );
 		}
 
 		mrc = soi.mrc;
@@ -138,7 +138,7 @@ xapp::Messenger::~Messenger() {
 	}
 
 	if( listen_port != NULL ) {
-		free( listen_port );
+		delete( listen_port );
 	}
 }
 
@@ -147,7 +147,7 @@ xapp::Messenger::~Messenger() {
 	message is received.  The user may pass an optional data pointer which
 	will be passed to the function when it is called.  The function signature
 	must be:
-		void fun( Messenger* mr, rmr_mbuf_t* mbuf,  void* data );
+		void fun( Messenger* mr, rmr_mbuf_t* mbuf,  void* data )
 
 	The user can also invoke this function to set the "default" callback by
 	passing Messenger::DEFAULT_CALLBACK as the mtype. If no other callback
@@ -181,7 +181,7 @@ std::unique_ptr<Message> xapp::Messenger::Alloc_msg( int payload_size ) {
 	just a message, but to avoid having the user pass the framework
 	object in, we'll just supply a "factory" function.
 */
-std::unique_ptr<xapp::Alarm> xapp::Messenger::Alloc_alarm( int prob_id, std::string meid ) {
+std::unique_ptr<xapp::Alarm> xapp::Messenger::Alloc_alarm( int prob_id, const std::string& meid ) {
 	std::shared_ptr<Message> m;
 	Alarm* a;
 
@@ -192,7 +192,7 @@ std::unique_ptr<xapp::Alarm> xapp::Messenger::Alloc_alarm( int prob_id, std::str
 	return std::unique_ptr<Alarm>( a );
 }
 
-std::unique_ptr<xapp::Alarm> xapp::Messenger::Alloc_alarm( std::string meid ) {
+std::unique_ptr<xapp::Alarm> xapp::Messenger::Alloc_alarm( const std::string& meid ) {
 	return Alloc_alarm( -1, meid );
 }
 
@@ -209,14 +209,14 @@ std::unique_ptr<xapp::Metrics> xapp::Messenger::Alloc_metrics( ) {
 	return std::unique_ptr<xapp::Metrics>( new xapp::Metrics( m ) );
 }
 
-std::unique_ptr<xapp::Metrics> xapp::Messenger::Alloc_metrics( std::string source ) {
+std::unique_ptr<xapp::Metrics> xapp::Messenger::Alloc_metrics( const std::string& source ) {
 	std::shared_ptr<Message> m;
 
 	m = Alloc_msg( 4096 );
 	return std::unique_ptr<xapp::Metrics>( new xapp::Metrics( m, source ) );
 }
 
-std::unique_ptr<xapp::Metrics> xapp::Messenger::Alloc_metrics( std::string reporter, std::string source ) {
+std::unique_ptr<xapp::Metrics> xapp::Messenger::Alloc_metrics( const std::string& reporter, const std::string& source ) {
 	std::shared_ptr<Message> m;
 
 	m = Alloc_msg( 4096 );
@@ -235,7 +235,6 @@ std::unique_ptr<xapp::Metrics> xapp::Messenger::Alloc_metrics( std::string repor
 	Concurrently executing listeners are allowed.
 */
 void xapp::Messenger::Listen( ) {
-	int count = 0;
 	rmr_mbuf_t*	mbuf = NULL;
 	std::map<int,Callback*>::iterator mi;	// map iterator; silly indirect way to point at the value
 	Callback*	dcb = NULL;					// default callback so we don't search
@@ -348,7 +347,7 @@ bool xapp::Messenger::Wait_for_cts( int max_wait ) {
 /*
 	Open a wormhole to the indicated endpoint and return the wormhole ID.
 */
-int xapp::Messenger::Wormhole_open( std::string endpoint ) {
+int xapp::Messenger::Wormhole_open( const std::string& endpoint ) {
 	rmr_whid_t whid;
 
 	whid = rmr_wh_open( mrc, endpoint.c_str() );
diff --git a/src/messaging/messenger.hpp b/src/messaging/messenger.hpp
index 82d8a49..457e24b 100644
--- a/src/messaging/messenger.hpp
+++ b/src/messaging/messenger.hpp
@@ -80,19 +80,19 @@ class Messenger {
 		std::unique_ptr<Message> Alloc_msg( int payload_size );			// message allocation
 
 		std::unique_ptr<xapp::Alarm> Alloc_alarm( );					// alarm allocation
-		std::unique_ptr<xapp::Alarm> Alloc_alarm( std::string meid );
-		std::unique_ptr<xapp::Alarm> Alloc_alarm( int prob_id, std::string meid );
+		std::unique_ptr<xapp::Alarm> Alloc_alarm( const std::string& meid );
+		std::unique_ptr<xapp::Alarm> Alloc_alarm( int prob_id, const std::string& meid );
 
 		std::unique_ptr<xapp::Metrics> Alloc_metrics( );					// metrics allocation
-		std::unique_ptr<xapp::Metrics> Alloc_metrics( std::string source );
-		std::unique_ptr<xapp::Metrics> Alloc_metrics( std::string reporter, std::string source );
+		std::unique_ptr<xapp::Metrics> Alloc_metrics( const std::string& source );
+		std::unique_ptr<xapp::Metrics> Alloc_metrics( const std::string& reporter, const std::string& source );
 
 		void Listen( );													// lisen driver
 		std::unique_ptr<Message> Receive( int timeout );				// receive 1 message
 		void Stop( );													// force to stop
 		bool Wait_for_cts( int max_wait );
 
-		int	Wormhole_open( std::string endpoint );
+		int	Wormhole_open( const std::string& endpoint );
 };
 
 
diff --git a/src/messaging/msg_component.hpp b/src/messaging/msg_component.hpp
index 40662a2..95e445c 100644
--- a/src/messaging/msg_component.hpp
+++ b/src/messaging/msg_component.hpp
@@ -47,7 +47,7 @@ namespace xapp {
 	such a smart pointer, and does _nothing_ when called.
 */
 typedef struct {
-	void operator()( unsigned char * p ){}
+	void operator()( unsigned char * p ) const { /* empty to prevent free */ }
 } unfreeable;
 
 /*
-- 
2.16.6