From 3f1473fb294b1f2c77e97cfadef9022fa97b0397 Mon Sep 17 00:00:00 2001 From: "Chaunte W. Lacewell" Date: Wed, 5 Apr 2023 20:45:41 -0700 Subject: [PATCH] Fix Coverity Issues for v2.4.0 (#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 Co-authored-by: tmcourie Co-authored-by: Ragaad --- client/cpp/CSVParserUtil.cpp | 69 +++++++++++----------------- ext/custom_vcl/custom_vcl_process.cc | 15 ++++-- src/DescriptorsCommand.cc | 4 +- src/PMGDIterators.cc | 4 +- src/PMGDIterators.h | 10 ++-- src/Server.cc | 54 +++++++++++----------- src/vcl/CustomVCL.cc | 26 +++++++---- src/vcl/DescriptorParams.h | 2 - src/vcl/DescriptorSetData.h | 9 ++-- src/vcl/Image.cc | 2 + src/vcl/TDBDescriptorSet.cc | 2 + src/vcl/TDBDescriptorSet.h | 4 +- src/vdms.cc | 29 ++++++------ 13 files changed, 120 insertions(+), 110 deletions(-) diff --git a/client/cpp/CSVParserUtil.cpp b/client/cpp/CSVParserUtil.cpp index 207f1c92..75041c9f 100644 --- 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 lock(CSVParserUtil::querytype_mutex); std::map::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 index 42baface..55d04b04 100644 --- 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 index ceedae7f..86e19a12 100644 --- 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, ¶m); desc_set.store(); } catch (VCL::Exception e) { diff --git a/src/PMGDIterators.cc b/src/PMGDIterators.cc index 2cd40c7c..6d88eab4 100644 --- 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 index 3dcfa80f..7bf1fd92 100644 --- 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 _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 index 0b6e6962..12673f18 100644 --- 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 index 2f46911a..6ba37533 100644 --- 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,11 +28,12 @@ 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; @@ -33,7 +41,7 @@ int custom_vcl_function(VCL::Image& img, const Json::Value& ops) //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; - -} \ No newline at end of file +} diff --git a/src/vcl/DescriptorParams.h b/src/vcl/DescriptorParams.h index 698a2fed..ac5498fd 100644 --- 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 index cd66008b..a43f4635 100644 --- 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) {} diff --git a/src/vcl/Image.cc b/src/vcl/Image.cc index bbf03b68..dcb9dad2 100644 --- a/src/vcl/Image.cc +++ b/src/vcl/Image.cc @@ -494,6 +494,8 @@ Image::~Image() _operations.clear(); _operations.shrink_to_fit(); delete _tdb; + if(_bin) + free(_bin); } /* *********************** */ diff --git a/src/vcl/TDBDescriptorSet.cc b/src/vcl/TDBDescriptorSet.cc index 1237ce3f..3dfdc8db 100644 --- a/src/vcl/TDBDescriptorSet.cc +++ b/src/vcl/TDBDescriptorSet.cc @@ -145,6 +145,8 @@ void TDBDescriptorSet::classify(float* descriptors, unsigned n, } labels[j] = winner; } + delete[] distances; + delete[] ids_aux; } void TDBDescriptorSet::get_labels(long* ids, unsigned n, long* labels) diff --git a/src/vcl/TDBDescriptorSet.h b/src/vcl/TDBDescriptorSet.h index 8eb10331..edb16ae1 100644 --- a/src/vcl/TDBDescriptorSet.h +++ b/src/vcl/TDBDescriptorSet.h @@ -121,7 +121,7 @@ namespace VCL { TDBDenseDescriptorSet(const std::string &collection_path, unsigned dim, DistanceMetric metric); - ~TDBDenseDescriptorSet(); + ~TDBDenseDescriptorSet() {}; long add(float* descriptors, unsigned n_descriptors, long* classes); @@ -152,7 +152,7 @@ namespace VCL { TDBSparseDescriptorSet(const std::string &collection_path, unsigned dim, DistanceMetric metric); - ~TDBSparseDescriptorSet(); + ~TDBSparseDescriptorSet() {}; long add(float* descriptors, unsigned n_descriptors, long* classes); diff --git a/src/vdms.cc b/src/vdms.cc index f85b11f2..f50027d0 100644 --- a/src/vdms.cc +++ b/src/vdms.cc @@ -40,7 +40,7 @@ void printUsage() { std::cout << "Usage: vdms -cfg config-file.json" << std::endl; - + std::cout << "Usage: vdms -restore db.tar.gz" << std::endl; exit(0); } @@ -54,6 +54,7 @@ static void* start_request_thread(void* server) } static void* start_replication_thread(void* server){ ((VDMS::Server*)(server))->auto_replicate_data(); + return NULL; } @@ -79,36 +80,36 @@ int main(int argc, char **argv) if (argc == 3){ std::string option(argv[1]); - + if (option != "-cfg" && option!="-restore" && option!="-backup") printUsage(); if(option =="-cfg") config_file = std::string (argv[2]); - - - + + + else if (option=="-restore" ){ void* server; - + std::string db_name(argv[2]); size_t file_ext1 = db_name.find_last_of("."); - + std::string temp_name_1= db_name.substr(0,file_ext1); - + size_t file_ext2 = temp_name_1.find_last_of("."); std::string temp_name_2= temp_name_1.substr(0,file_ext2); - + ((VDMS::Server*)(server))->untar_data(db_name); - + config_file = temp_name_2+".json"; - + } } - + printf("Server will start processing requests... \n"); VDMS::Server server(config_file); @@ -116,13 +117,13 @@ int main(int argc, char **argv) request_thread_flag = pthread_create(&request_thread, NULL, start_request_thread, (void*)( &server ) ); autodelete_thread_flag = pthread_create(&autodelete_thread, NULL, start_autodelete_thread, (void*)( &server ) ); auto_replcation_flag = pthread_create(&auto_replicate_thread, NULL, start_replication_thread, (void*)( &server ) ); - + pthread_join(request_thread, NULL); pthread_join(autodelete_thread, NULL); pthread_join(auto_replicate_thread, NULL); - + printf("Server shutting down... \n");