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
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 @@
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 @@
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 @@
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 @@
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 @@
severity.c_str(),
info.c_str(),
add_info.c_str(),
- action.c_str(),
+ maction.c_str(),
now()
);
@@ -162,41 +161,23 @@
*/
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 @@
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;
+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;
-}
+ 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 @@
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 @@
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 @@
// ---- 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_appid( std::string new_id ) {
+void xapp::Alarm::Set_appid( const std::string& new_id ) {
app_id = new_id;
}
@@ -338,11 +318,11 @@
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 @@
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 @@
/*
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 @@
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 @@
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 @@
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 @@
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 @@
~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 @@
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 @@
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 @@
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 @@
}
}
}
-
- free( dname );
- free( bname );
}
@@ -162,10 +161,10 @@
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 @@
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 @@
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 @@
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 @@
}
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 @@
}
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 @@
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 @@
}
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 @@
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 @@
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 @@
}
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 @@
/*
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 @@
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 @@
}
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 @@
/*
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 @@
#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 @@
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 @@
/*
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 @@
/*
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 @@
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 @@
suss out the values.
*/
xapp::Jhash::Jhash( const char* jbuf ) :
- master_st( NULL ),
st( jw_new( jbuf ) )
{ /* empty body */ }
@@ -57,10 +56,10 @@
/*
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 @@
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 @@
*/
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 @@
*/
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
/*
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 @@
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 @@
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 @@
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 @@
/*
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 @@
*/
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 @@
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 @@
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 @@
/*
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( rmr_mbuf_t* mbuf, void* mrc ) :
+ mrc( mrc ), // the message router context for sends
+ mbuf( mbuf )
+{ /* empty body */ }
-xapp::Message::Message( void* mrc, int payload_len ) {
- this->mrc = mrc;
- this->mbuf = rmr_alloc_msg( mrc, payload_len );
-}
+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 @@
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 @@
/*
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 @@
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 @@
/*
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 @@
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 @@
return state;
}
-int xapp::Message::Get_subid(){
+int xapp::Message::Get_subid() const {
int rval = INVALID_SUBID;
if( mbuf != NULL ) {
@@ -231,7 +234,7 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
rmr_close( mrc );
}
if( listen_port != NULL ) {
- free( listen_port );
+ delete( listen_port );
}
mrc = soi.mrc;
@@ -138,7 +138,7 @@
}
if( listen_port != NULL ) {
- free( listen_port );
+ delete( listen_port );
}
}
@@ -147,7 +147,7 @@
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 @@
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 @@
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 @@
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 @@
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 @@
/*
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 @@
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 @@
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;
/*