From: Frederic Suter Date: Thu, 26 Sep 2019 14:19:55 +0000 (+0200) Subject: [sonar] fix some bugs and smells related to disk addition X-Git-Tag: v3.24~55 X-Git-Url: http://info.iut-bm.univ-fcomte.fr/pub/gitweb/simgrid.git/commitdiff_plain/84f0fd4f321ba3d01daf3a6a64d5826008376548?ds=inline [sonar] fix some bugs and smells related to disk addition --- diff --git a/include/simgrid/plugins/file_system.h b/include/simgrid/plugins/file_system.h index 76cf0f782e..feb2a74b16 100644 --- a/include/simgrid/plugins/file_system.h +++ b/include/simgrid/plugins/file_system.h @@ -153,7 +153,10 @@ public: std::map* get_content() const { return content_.get(); } const char* get_mount_point() { return mount_point_.c_str(); } const char* get_mount_point(s4u::Host* remote_host) { return remote_mount_points_[remote_host].c_str(); } - void add_remote_mount(Host* host, std::string mount_point) { remote_mount_points_.insert({host, mount_point}); } + void add_remote_mount(Host* host, const std::string& mount_point) + { + remote_mount_points_.insert({host, mount_point}); + } sg_size_t get_size() const { return size_; } sg_size_t get_used_size() const { return used_size_; } void decr_used_size(sg_size_t size) { used_size_ -= size; } diff --git a/include/simgrid/s4u/Host.hpp b/include/simgrid/s4u/Host.hpp index 45038bc156..7679ffa6fd 100644 --- a/include/simgrid/s4u/Host.hpp +++ b/include/simgrid/s4u/Host.hpp @@ -117,7 +117,7 @@ public: std::vector get_disks() const; void add_disk(Disk* disk); - void remove_disk(std::string disk_name); + void remove_disk(const std::string& disk_name); std::vector get_attached_storages() const; diff --git a/src/plugins/file_system/s4u_FileSystem.cpp b/src/plugins/file_system/s4u_FileSystem.cpp index 9c3812d8f3..5eddc990c8 100644 --- a/src/plugins/file_system/s4u_FileSystem.cpp +++ b/src/plugins/file_system/s4u_FileSystem.cpp @@ -93,7 +93,7 @@ File::File(const std::string& fullpath, sg_host_t host, void* userdata) : fullpa ext->file_descriptor_table->pop_back(); XBT_DEBUG("\tOpen file '%s'", path_.c_str()); - std::map* content; + std::map* content = nullptr; if (local_storage_) content = local_storage_->extension()->get_content(); @@ -101,13 +101,15 @@ File::File(const std::string& fullpath, sg_host_t host, void* userdata) : fullpa content = local_disk_->extension()->get_content(); // if file does not exist create an empty file - auto sz = content->find(path_); - if (sz != content->end()) { - size_ = sz->second; - } else { - size_ = 0; - content->insert({path_, size_}); - XBT_DEBUG("File '%s' was not found, file created.", path_.c_str()); + if (content) { + auto sz = content->find(path_); + if (sz != content->end()) { + size_ = sz->second; + } else { + size_ = 0; + content->insert({path_, size_}); + XBT_DEBUG("File '%s' was not found, file created.", path_.c_str()); + } } } @@ -163,7 +165,7 @@ sg_size_t File::read(sg_size_t size) current_position_ += read_size; } - if (host->get_name() != Host::current()->get_name() && read_size > 0) { + if (host && host->get_name() != Host::current()->get_name() && read_size > 0) { /* the file is hosted on a remote host, initiate a communication between src and dest hosts for data transfer */ XBT_DEBUG("File is on %s remote host, initiate data transfer of %llu bytes.", host->get_cname(), read_size); host->send_to(Host::current(), read_size); @@ -190,7 +192,7 @@ sg_size_t File::write(sg_size_t size, int write_inside) if (local_disk_) host = local_disk_->get_host(); - if (host->get_name() != Host::current()->get_name()) { + if (host && host->get_name() != Host::current()->get_name()) { /* the file is hosted on a remote host, initiate a communication between src and dest hosts for data transfer */ XBT_DEBUG("File is on %s remote host, initiate data transfer of %llu bytes.", host->get_cname(), size); Host::current()->send_to(host, size); @@ -284,20 +286,22 @@ void File::move(const std::string& fullpath) { /* Check if the new full path is on the same mount point */ if (fullpath.compare(0, mount_point_.length(), mount_point_) == 0) { - std::map* content; + std::map* content = nullptr; if (local_storage_) content = local_storage_->extension()->get_content(); if (local_disk_) content = local_disk_->extension()->get_content(); - auto sz = content->find(path_); - if (sz != content->end()) { // src file exists - sg_size_t new_size = sz->second; - content->erase(path_); - std::string path = fullpath.substr(mount_point_.length(), fullpath.length()); - content->insert({path.c_str(), new_size}); - XBT_DEBUG("Move file from %s to %s, size '%llu'", path_.c_str(), fullpath.c_str(), new_size); - } else { - XBT_WARN("File %s doesn't exist", path_.c_str()); + if (content) { + auto sz = content->find(path_); + if (sz != content->end()) { // src file exists + sg_size_t new_size = sz->second; + content->erase(path_); + std::string path = fullpath.substr(mount_point_.length(), fullpath.length()); + content->insert({path.c_str(), new_size}); + XBT_DEBUG("Move file from %s to %s, size '%llu'", path_.c_str(), fullpath.c_str(), new_size); + } else { + XBT_WARN("File %s doesn't exist", path_.c_str()); + } } } else { XBT_WARN("New full path %s is not on the same mount point: %s.", fullpath.c_str(), mount_point_.c_str()); @@ -340,7 +344,7 @@ int File::unlink() int File::remote_copy(sg_host_t host, const char* fullpath) { /* Find the host where the file is physically located and read it */ - Host* src_host; + Host* src_host = nullptr; if (local_storage_) { src_host = local_storage_->get_host(); XBT_DEBUG("READ %s on disk '%s'", get_path(), local_storage_->get_cname()); @@ -405,9 +409,11 @@ int File::remote_copy(sg_host_t host, const char* fullpath) } } - XBT_DEBUG("Initiate data transfer of %llu bytes between %s and %s.", read_size, src_host->get_cname(), - dst_host->get_cname()); - src_host->send_to(dst_host, read_size); + if (src_host) { + XBT_DEBUG("Initiate data transfer of %llu bytes between %s and %s.", read_size, src_host->get_cname(), + dst_host->get_cname()); + src_host->send_to(dst_host, read_size); + } /* Create file on remote host, write it and close it */ File* fd = new File(fullpath, dst_host, nullptr); diff --git a/src/s4u/s4u_Host.cpp b/src/s4u/s4u_Host.cpp index f86b99071f..5cb23b1611 100644 --- a/src/s4u/s4u_Host.cpp +++ b/src/s4u/s4u_Host.cpp @@ -291,7 +291,8 @@ void Host::add_disk(Disk* disk) { kernel::actor::simcall([this, disk] { this->pimpl_->add_disk(disk); }); } -void Host::remove_disk(std::string disk_name) + +void Host::remove_disk(const std::string& disk_name) { kernel::actor::simcall([this, disk_name] { this->pimpl_->remove_disk(disk_name); }); } diff --git a/src/surf/HostImpl.cpp b/src/surf/HostImpl.cpp index c037bf9022..c5817fedad 100644 --- a/src/surf/HostImpl.cpp +++ b/src/surf/HostImpl.cpp @@ -164,7 +164,7 @@ void HostImpl::add_disk(s4u::Disk* disk) disks_.push_back(disk->get_impl()); } -void HostImpl::remove_disk(std::string disk_name) +void HostImpl::remove_disk(const std::string& disk_name) { auto position = disks_.begin(); for (auto const& d : disks_) { diff --git a/src/surf/HostImpl.hpp b/src/surf/HostImpl.hpp index 791b13500f..6583e660af 100644 --- a/src/surf/HostImpl.hpp +++ b/src/surf/HostImpl.hpp @@ -50,7 +50,7 @@ public: std::vector get_disks(); void add_disk(s4u::Disk* disk); - void remove_disk(std::string disk_name); + void remove_disk(const std::string& disk_name); /** @brief Get the vector of storages (by names) attached to the Host */ virtual std::vector get_attached_storages();