Fix Coverity Issues for v2.4.0 (IntelLabs#110)

* Fix CIDs 3428250, 3399517, 3399721 * Fix CID 3399499 * Fix CIDs 3399722 and 3399506 * Fix CID 3399609 and 3399442 * Fix CID 3399441 * Fix 3399465 and 3399606 * fix 3399466 * Fix CID 3399404 and 3399679 * Fix CID 3441993 Signed-off-by: tmcourie <[email protected]> Co-authored-by: tmcourie <[email protected]> Co-authored-by: Ragaad <[email protected]>
cwlacewe · Apr 6, 2023 · 3f1473f · 3f1473f
1 parent 12dd421
commit 3f1473f
Show file tree

Hide file tree

Showing 13 changed files with 120 additions and 110 deletions.
diff --git a/client/cpp/CSVParserUtil.cpp b/client/cpp/CSVParserUtil.cpp
@@ -164,58 +164,41 @@ bool VDMS::CSVParserUtil::isInt(const std::string &s)
 
 VDMS::CSVParserUtil::commandType VDMS::CSVParserUtil::get_query_type(const string &str)
 {
-    CSVParserUtil::commandType querytype;
+    CSVParserUtil::commandType querytype = commandType::UNKNOWN;
 
     std::lock_guard<std::mutex> lock(CSVParserUtil::querytype_mutex);
     std::map<std::string, QueryType>::iterator iter;
     iter = commands.find(str);
-    if (iter == commands.end()) {
-        return commandType::UNKNOWN;
-    } else {
+    if (iter != commands.end()) {
         switch (commands[str])
         {
-        case EntityClass:
-            querytype = commandType::AddEntity;
-            break;
-
-        case ConnectionClass:
-            querytype = commandType::AddConnection;
-            break;
-        case ImagePath:
-            querytype = commandType::AddImage;
-            break;
-        case VideoPath:
-            querytype = commandType::AddVideo;
-            break;
-        case DescriptorType:
-            querytype = commandType::AddDescriptorSet;
-            break;
-        case DescriptorClass:
-            querytype = commandType::AddDescriptor;
-            break;
-        case RectangleBound:
-            querytype = commandType::AddBoundingBox;
-
-            break;
-        // case EntityUpdate:
-        //     querytype = commandType::UpdateEntity;
-
-        //     break;
-        // case ConnectionUpdate:
-        //     querytype = commandType::UpdateConnection;
-
-        //     break;
-        // case ImageUpdate:
-        //     querytype = commandType::UpdateImage;
-        //     break;
-        // case RectangleUpdate:
-        //     querytype = commandType::UpdateBoundingBox;
-        //     break;
+            case EntityClass:
+                querytype = commandType::AddEntity;
+                break;
+            case ConnectionClass:
+                querytype = commandType::AddConnection;
+                break;
+            case ImagePath:
+                querytype = commandType::AddImage;
+                break;
+            case VideoPath:
+                querytype = commandType::AddVideo;
+                break;
+            case DescriptorType:
+                querytype = commandType::AddDescriptorSet;
+                break;
+            case DescriptorClass:
+                querytype = commandType::AddDescriptor;
+                break;
+            case RectangleBound:
+                querytype = commandType::AddBoundingBox;
+                break;
         }
-
         //   std::cout << " I executed queryType "<< querytype << std::endl;
-        return querytype;
+        // return querytype;
     }
+
+    return querytype;
 }
 
 VDMS::CSVParserUtil::DATATYPE VDMS::CSVParserUtil::getDataType(const string &str, const string &propname)

diff --git a/ext/custom_vcl/custom_vcl_process.cc b/ext/custom_vcl/custom_vcl_process.cc
@@ -9,6 +9,12 @@ int main(int argc, char* argv[])
     key_ctl_host_remote = ftok("../../vdms", 60);
     int msgid_ctl_host_remote = msgget(key_ctl_host_remote, 0666 | IPC_CREAT);
     data_message message_ctl_host_remote;
+    //need size of data message excluding message_type field for msgsnd and msgrcv
+    size_t data_message_size = sizeof(message_ctl_host_remote.data_rows) + 
+                                    sizeof(message_ctl_host_remote.data_cols) + 
+                                    sizeof(message_ctl_host_remote.data_type) + 
+                                    sizeof(message_ctl_host_remote.data_image_size) + 
+                                    sizeof(message_ctl_host_remote.data_json_size);
 
     key_t key_data_host_remote;
     key_data_host_remote = ftok("../../vdms", 61);
@@ -22,17 +28,18 @@ int main(int argc, char* argv[])
 
     heartbeat_message message_hb_host_remote;
     heartbeat_message message_hb_remote_host;
+    size_t heartbeat_message_size = sizeof(message_hb_host_remote.status);
 
     while(true)
     {
         //Handle handshake to indicate remote process is alive
-        int in_alive_msg_status = msgrcv(msgid_ctl_host_remote, &message_hb_host_remote,sizeof(heartbeat_message) , (long) vcl_message_type::VCL_MESSAGE_HEARTBEAT, 0);
+        int in_alive_msg_status = msgrcv(msgid_ctl_host_remote, &message_hb_host_remote, heartbeat_message_size, (long) vcl_message_type::VCL_MESSAGE_HEARTBEAT, 0);
 
         message_hb_remote_host.message_type = (long) vcl_message_type::VCL_MESSAGE_HEARTBEAT;
         message_hb_remote_host.status = 0;
-        int msg_send_result = msgsnd(msgid_ctl_remote_host, &message_hb_remote_host, sizeof(heartbeat_message), 0);
+        int msg_send_result = msgsnd(msgid_ctl_remote_host, &message_hb_remote_host, heartbeat_message_size, 0);
 
-        int msg_status = msgrcv(msgid_ctl_host_remote, &message_ctl_host_remote,sizeof(data_message) , (long) vcl_message_type::VCL_MESSAGE_DATA, 0);
+        int msg_status = msgrcv(msgid_ctl_host_remote, &message_ctl_host_remote, data_message_size, (long) vcl_message_type::VCL_MESSAGE_DATA, 0);
         if(msg_status > 0)
         {
             //Read image from shared memory
@@ -78,7 +85,7 @@ int main(int argc, char* argv[])
                 }
             }
 
-            int msg_send_result = msgsnd(msgid_ctl_remote_host, &message_ctl_remote_host, sizeof(data_message), 0);
+            int msg_send_result = msgsnd(msgid_ctl_remote_host, &message_ctl_remote_host, data_message_size, 0);
             if(msg_send_result < 0)
             { }
 

diff --git a/src/DescriptorsCommand.cc b/src/DescriptorsCommand.cc
@@ -212,8 +212,8 @@ Json::Value AddDescriptorSet::construct_responses(
     // We can probably set up a mechanism
     // to fix a broken link when detected later, same with images.
     try {
-       VCL::DescriptorParams* param = new VCL::DescriptorParams(_flinng_num_rows, _flinng_cells_per_row, _flinng_num_hash_tables,_flinng_hashes_per_table);
-       VCL::DescriptorSet desc_set(desc_set_path, dimensions, eng, metric, param);
+       VCL::DescriptorParams param(_flinng_num_rows, _flinng_cells_per_row, _flinng_num_hash_tables,_flinng_hashes_per_table);
+       VCL::DescriptorSet desc_set(desc_set_path, dimensions, eng, metric, &param);
        desc_set.store();
     }
     catch (VCL::Exception e) {

diff --git a/src/PMGDIterators.cc b/src/PMGDIterators.cc
@@ -97,10 +97,10 @@ bool PMGDQueryHandler::NodeEdgeIteratorImpl::next()
 bool PMGDQueryHandler::NodeEdgeIteratorImpl::_next()
 {
     while (_src_ni != NULL && bool(*_src_ni)) {
-        delete _edge_it;
+        // delete _edge_it;
         _src_ni->next();
         if (bool(*_src_ni)) {
-            _edge_it = new PMGD::EdgeIterator((*_src_ni)->get_edges(_dir, _expr.tag()));
+            _edge_it.reset( new PMGD::EdgeIterator((*_src_ni)->get_edges(_dir, _expr.tag())));
             while (_edge_it != NULL && bool(*_edge_it)) {
                 if (check_predicates())
                     return true;

diff --git a/src/PMGDIterators.h b/src/PMGDIterators.h
@@ -206,7 +206,8 @@ namespace VDMS {
         PMGD::Direction _dir;
         bool _check_dest;
 
-        PMGD::EdgeIterator *_edge_it;
+        // PMGD::EdgeIterator *_edge_it;
+        std::unique_ptr<PMGD::EdgeIterator> _edge_it;
 
         bool _next();
         bool check_predicates();
@@ -232,6 +233,8 @@ namespace VDMS {
                 PMGD::PropertyPredicate pp;
                 if (_num_predicates > 0)
                     pp = _expr.get_node_predicate(0);
+                else
+                    pp = PMGD::PropertyPredicate();
                 return _expr.db().get_edges(_expr.tag(), pp);
             }
             else {
@@ -245,9 +248,10 @@ namespace VDMS {
                              ReusableNodeIterator *dest_ni = NULL)
             : _expr(expr), _num_predicates(_expr.num_node_predicates()),
               _src_ni(src_ni), _dest_ni(dest_ni),
-              _pred_start(0), _check_dest(false),
-              _edge_it(new PMGD::EdgeIterator(return_iterator()))
+              _pred_start(0), _check_dest(false)
+
         {
+            _edge_it.reset(new PMGD::EdgeIterator(return_iterator()));
             // If the first criteria did not return any edges,
             // there is no node checking on either side.
             if (!bool(*_edge_it))

diff --git a/src/Server.cc b/src/Server.cc
@@ -61,16 +61,16 @@ Server::Server(std::string config_file)
     _autodelete_interval = VDMSConfig::instance()
                         ->get_int_value("autodelete_interval_s", DEFAULT_AUTODELETE_INTERVAL);
     _backup_flag =    VDMSConfig::instance()
-                        ->get_string_value("backup_flag", DEFAULT_AUTOREPLICATE_FLAG) ;                  
+                        ->get_string_value("backup_flag", DEFAULT_AUTOREPLICATE_FLAG) ;
 
     _autoreplecate_interval = VDMSConfig::instance()
                         ->get_int_value("autoreplicate_interval", DEFAULT_AUTOREPLICATE_INTERVAL);
     _replication_unit =  VDMSConfig::instance()
-                        ->get_string_value("unit", DEFAULT_AUTOREPLICATE_UNIT); 
+                        ->get_string_value("unit", DEFAULT_AUTOREPLICATE_UNIT);
     _backup_path =  VDMSConfig::instance()
-                        ->get_string_value("backup_path", DEFAULT_BACKUP_PATH);  
+                        ->get_string_value("backup_path", DEFAULT_BACKUP_PATH);
     _db_path =     VDMSConfig::instance()
-                        ->get_string_value("db_root_path", DEFAULT_DB_ROOT);                 
+                        ->get_string_value("db_root_path", DEFAULT_DB_ROOT);
 
     PMGDQueryHandler::init();
     QueryHandler::init();
@@ -109,7 +109,7 @@ void Server::process_requests()
                 new comm::Connection(server->accept());
             _cm->add_connection(conn_server);
 
-           
+
         }
         catch (comm::ExceptionComm e) {
             print_exception(e);
@@ -119,43 +119,43 @@ void Server::process_requests()
     delete server;
 }
 void Server::untar_data(std::string& name){
-    
-   
+
+
         std::string command="tar -xvSf" + name;
         system(command.c_str());
-    
+
 }
 void Server::auto_replicate_data(){
-    
-    long replication_period;
+
+    long replication_period = 0;
     QueryHandler qh;
- if(_backup_flag =="true"){
+    if(_backup_flag =="true"){
        if (_autoreplecate_interval >0 ){
             if (_replication_unit.compare("h") == 0){
                 replication_period =_autoreplecate_interval*60*60;
             }
             else if (_replication_unit.compare("m") == 0)
                 replication_period =_autoreplecate_interval*60;
-        
-            else     
-                 replication_period= _autoreplecate_interval;
-       }       
-          
+
+            else
+                replication_period= _autoreplecate_interval;
+        }
+
         if(_backup_path.empty()){
             _backup_path=_db_path; //set the defualt path to be db
-        } 
-
-
-     if(replication_period > 0) //check to ensure valid autodelete_interval
-      { 
-
-        while(!shutdown)
+        }
+
+
+        if(replication_period > 0) //check to ensure valid autodelete_interval
         {
-            sleep(replication_period);
-            qh.regualar_run_autoreplicate(_backup_path, _db_path, _server_port);
+
+            while(!shutdown)
+            {
+                sleep(replication_period);
+                qh.regualar_run_autoreplicate(_backup_path, _db_path, _server_port);
+            }
         }
-    } 
- } 
+    }
 }
 void Server::autodelete_expired_data()
 {

diff --git a/src/vcl/CustomVCL.cc b/src/vcl/CustomVCL.cc
@@ -6,8 +6,15 @@ int custom_vcl_function(VCL::Image& img, const Json::Value& ops)
     //create IPC structures for communicating between processes
     key_t key_ctl_host_remote;
     key_ctl_host_remote = ftok("vdms", 60);
+
     int msgid_ctl_host_remote = msgget(key_ctl_host_remote, 0666 | IPC_CREAT);
     data_message message_ctl_host_remote;
+    //need size of data message excluding message_type field for msgsnd and msgrcv
+    size_t data_message_size = sizeof(message_ctl_host_remote.data_rows) + 
+                                    sizeof(message_ctl_host_remote.data_cols) + 
+                                    sizeof(message_ctl_host_remote.data_type) + 
+                                    sizeof(message_ctl_host_remote.data_image_size) + 
+                                    sizeof(message_ctl_host_remote.data_json_size);
 
     key_t key_data_host_remote;
     key_data_host_remote = ftok("vdms", 61);
@@ -21,19 +28,20 @@ int custom_vcl_function(VCL::Image& img, const Json::Value& ops)
 
     heartbeat_message message_hb_host_remote;
     heartbeat_message message_hb_remote_host;
+    size_t heartbeat_message_size = sizeof(message_hb_host_remote.status);
 
     //Pass messages to ensure the remote process is functional
     message_hb_host_remote.message_type = (long) vcl_message_type::VCL_MESSAGE_HEARTBEAT;
     message_hb_host_remote.status = 0;
-    int out_alive_msg_status = msgsnd(msgid_ctl_host_remote, &message_hb_host_remote, sizeof(heartbeat_message), 0);
+    int out_alive_msg_status = msgsnd(msgid_ctl_host_remote, &message_hb_host_remote, heartbeat_message_size, 0);
 
     int hb_count = 0;
     int in_alive_msg_status = -1;
 
     //try 10 times to determine if process is running
     while(hb_count < 10 && in_alive_msg_status < 0)
     {
-        in_alive_msg_status = msgrcv(msgid_ctl_remote_host, &message_hb_remote_host,sizeof(heartbeat_message) , (long) vcl_message_type::VCL_MESSAGE_HEARTBEAT, IPC_NOWAIT);
+        in_alive_msg_status = msgrcv(msgid_ctl_remote_host, &message_hb_remote_host, heartbeat_message_size, (long) vcl_message_type::VCL_MESSAGE_HEARTBEAT, IPC_NOWAIT);
         hb_count++;
     }
 
@@ -54,14 +62,17 @@ int custom_vcl_function(VCL::Image& img, const Json::Value& ops)
 
         std::string* json_string = new std::string(ops.toStyledString());
         message_ctl_host_remote.data_json_size = json_string->size();
-
         //image size corresponds with first byte after the image
         memcpy(&(image_buffer[in_image_size]), json_string->c_str(), json_string->size());
-        int msg_send_result = msgsnd(msgid_ctl_host_remote, &message_ctl_host_remote, sizeof(data_message), 0);
+        int msg_send_result = msgsnd(msgid_ctl_host_remote, &message_ctl_host_remote, data_message_size, 0);
         if(msg_send_result < 0)
-        {}
+        {
+            delete json_string;
+            return -1;
+        }
+
+        int msg_recv_result = msgrcv(msgid_ctl_remote_host, &message_ctl_remote_host, data_message_size, (long)vcl_message_type::VCL_MESSAGE_DATA, 0);
 
-        int msg_recv_result = msgrcv(msgid_ctl_remote_host, &message_ctl_remote_host, sizeof(data_message), (long)vcl_message_type::VCL_MESSAGE_DATA, 0);
         if(msg_recv_result < 0)
         {}
 
@@ -87,5 +98,4 @@ int custom_vcl_function(VCL::Image& img, const Json::Value& ops)
     }
 
    return return_value;
-
-}
+}
diff --git a/src/vcl/DescriptorParams.h b/src/vcl/DescriptorParams.h
@@ -70,7 +70,5 @@ namespace VCL {
         this->sub_hash_bits = subhashbits;
         this->cut_off= cutoff;
     }
-
-    ~DescriptorParams();
 };
 };
diff --git a/src/vcl/DescriptorSetData.h b/src/vcl/DescriptorSetData.h
@@ -74,7 +74,10 @@ namespace VCL {
         inline bool dir_exist(const std::string& dir_name) {
             DIR* dir = opendir(dir_name.c_str());
             if (dir)
+            {
+                closedir(dir);
                 return true;
+            }
 
             return false;
         }
@@ -117,7 +120,7 @@ namespace VCL {
          */
         DescriptorSetData(const std::string &filename, unsigned dim);
 
-        ~DescriptorSetData();
+        virtual ~DescriptorSetData();
 
         DescriptorSetData(const DescriptorSetData&) = delete;
 
@@ -147,7 +150,7 @@ namespace VCL {
          */
         virtual long add(float* descriptors, unsigned n_descriptors,
                          long* labels = NULL) = 0;
-        
+
         virtual long add_and_store(float* descriptors, unsigned n_descriptors,
                          long* labels = NULL) {return 0;}
 
@@ -163,7 +166,7 @@ namespace VCL {
          */
         virtual void search(float* query, unsigned n, unsigned k,
                             long* descriptors, float* distances) = 0;
-        
+
         virtual void search(float* query, unsigned n, unsigned k,
                             long* descriptors) {}