From 6c842a7515cae08123df2500c47598d1567755b2 Mon Sep 17 00:00:00 2001 From: Carsten Patzke <carsten.patzke@desy.de> Date: Fri, 9 Mar 2018 09:01:57 +0100 Subject: [PATCH] Fixed memory leak --- common/cpp/include/system_wrappers/io.h | 40 +++++++++---------- common/cpp/src/system_io/system_io.cpp | 32 +++++++-------- common/cpp/src/system_io/system_io_linux.cpp | 24 +++++------ .../json_parser/test_json_parser.cpp | 4 +- .../api/cpp/unittests/test_folder_broker.cpp | 22 +++++----- 5 files changed, 61 insertions(+), 61 deletions(-) diff --git a/common/cpp/include/system_wrappers/io.h b/common/cpp/include/system_wrappers/io.h index 1f3d447e3..acdd23103 100644 --- a/common/cpp/include/system_wrappers/io.h +++ b/common/cpp/include/system_wrappers/io.h @@ -14,26 +14,26 @@ namespace hidra2 { namespace IOErrorTemplate { -ErrorTemplate* const kUnknownError = new ErrorTemplate{"Unknown Error", ErrorType::kUnknownError}; - -ErrorTemplate* const kFileNotFound = new ErrorTemplate{"No such file or directory", ErrorType::kFileNotFound}; -ErrorTemplate* const kReadError = new ErrorTemplate{"Read error", ErrorType::kFileNotFound}; -ErrorTemplate* const kBadFileNumber = new ErrorTemplate{"Bad file number", ErrorType::kBadFileNumber}; -ErrorTemplate* const kResourceTemporarilyUnavailable = new ErrorTemplate{"Resource temporarily unavailable", ErrorType::kResourceTemporarilyUnavailable}; -ErrorTemplate* const kPermissionDenied = new ErrorTemplate{"Permission denied", ErrorType::kPermissionDenied}; -ErrorTemplate* const kUnsupportedAddressFamily = new ErrorTemplate{"Unsupported address family", ErrorType::kUnsupportedAddressFamily}; -ErrorTemplate* const kInvalidAddressFormat = new ErrorTemplate{"Invalid address format", ErrorType::kInvalidAddressFormat}; -ErrorTemplate* const kEndOfFile = new ErrorTemplate{"End of file", ErrorType::kEndOfFile}; -ErrorTemplate* const kAddressAlreadyInUse = new ErrorTemplate{"Address already in use", ErrorType::kAddressAlreadyInUse}; -ErrorTemplate* const kConnectionRefused = new ErrorTemplate{"Connection refused", ErrorType::kConnectionRefused}; -ErrorTemplate* const kConnectionResetByPeer = new ErrorTemplate{"kConnectionResetByPeer", ErrorType::kConnectionResetByPeer}; -ErrorTemplate* const kTimeout = new ErrorTemplate{"kTimeout", ErrorType::kTimeout}; -ErrorTemplate* const kFileAlreadyExists = new ErrorTemplate{"kFileAlreadyExists", ErrorType::kFileAlreadyExists}; -ErrorTemplate* const kNoSpaceLeft = new ErrorTemplate{"kNoSpaceLeft", ErrorType::kNoSpaceLeft}; -ErrorTemplate* const kSocketOperationOnNonSocket = new ErrorTemplate{"kSocketOperationOnNonSocket", ErrorType::kSocketOperationOnNonSocket}; -ErrorTemplate* const kMemoryAllocationError = new ErrorTemplate{"kMemoryAllocationError", ErrorType::kMemoryAllocationError}; -ErrorTemplate* const kInvalidMemoryAddress = new ErrorTemplate{"kInvalidMemoryAddress", ErrorType::kInvalidMemoryAddress}; -ErrorTemplate* const kUnableToResolveHostname = new ErrorTemplate{"kUnableToResolveHostname", ErrorType::kUnableToResolveHostname}; +auto const kUnknownError = ErrorTemplate{"Unknown Error", ErrorType::kUnknownError}; + +auto const kFileNotFound = ErrorTemplate{"No such file or directory", ErrorType::kFileNotFound}; +auto const kReadError = ErrorTemplate{"Read error", ErrorType::kFileNotFound}; +auto const kBadFileNumber = ErrorTemplate{"Bad file number", ErrorType::kBadFileNumber}; +auto const kResourceTemporarilyUnavailable = ErrorTemplate{"Resource temporarily unavailable", ErrorType::kResourceTemporarilyUnavailable}; +auto const kPermissionDenied = ErrorTemplate{"Permission denied", ErrorType::kPermissionDenied}; +auto const kUnsupportedAddressFamily = ErrorTemplate{"Unsupported address family", ErrorType::kUnsupportedAddressFamily}; +auto const kInvalidAddressFormat = ErrorTemplate{"Invalid address format", ErrorType::kInvalidAddressFormat}; +auto const kEndOfFile = ErrorTemplate{"End of file", ErrorType::kEndOfFile}; +auto const kAddressAlreadyInUse = ErrorTemplate{"Address already in use", ErrorType::kAddressAlreadyInUse}; +auto const kConnectionRefused = ErrorTemplate{"Connection refused", ErrorType::kConnectionRefused}; +auto const kConnectionResetByPeer = ErrorTemplate{"kConnectionResetByPeer", ErrorType::kConnectionResetByPeer}; +auto const kTimeout = ErrorTemplate{"kTimeout", ErrorType::kTimeout}; +auto const kFileAlreadyExists = ErrorTemplate{"kFileAlreadyExists", ErrorType::kFileAlreadyExists}; +auto const kNoSpaceLeft = ErrorTemplate{"kNoSpaceLeft", ErrorType::kNoSpaceLeft}; +auto const kSocketOperationOnNonSocket = ErrorTemplate{"kSocketOperationOnNonSocket", ErrorType::kSocketOperationOnNonSocket}; +auto const kMemoryAllocationError = ErrorTemplate{"kMemoryAllocationError", ErrorType::kMemoryAllocationError}; +auto const kInvalidMemoryAddress = ErrorTemplate{"kInvalidMemoryAddress", ErrorType::kInvalidMemoryAddress}; +auto const kUnableToResolveHostname = ErrorTemplate{"kUnableToResolveHostname", ErrorType::kUnableToResolveHostname}; } enum FileOpenMode { diff --git a/common/cpp/src/system_io/system_io.cpp b/common/cpp/src/system_io/system_io.cpp index dd874c365..bf6bb8afa 100644 --- a/common/cpp/src/system_io/system_io.cpp +++ b/common/cpp/src/system_io/system_io.cpp @@ -65,7 +65,7 @@ uint8_t* AllocateArray(uint64_t fsize, Error* err) { try { data_array = new uint8_t[fsize]; } catch (...) { - *err = IOErrorTemplate::kMemoryAllocationError->Copy(); + *err = IOErrorTemplate::kMemoryAllocationError.Copy(); return nullptr; } return data_array; @@ -144,7 +144,7 @@ void SystemIO::Skip(SocketDescriptor socket_fd, size_t length, Error* err) const try { buffer.reset(new uint8_t[kSkipBufferSize]); } catch(...) { - *err = IOErrorTemplate::kMemoryAllocationError->Copy(); + *err = IOErrorTemplate::kMemoryAllocationError.Copy(); return; } size_t already_skipped = 0; @@ -206,7 +206,7 @@ std::string SystemIO::ResolveHostnameToIp(const std::string& hostname, Error* er InitializeSocketIfNecessary(); hostent* record = gethostbyname(hostname.c_str()); if (record == nullptr) { - *err = IOErrorTemplate::kUnableToResolveHostname->Copy(); + *err = IOErrorTemplate::kUnableToResolveHostname.Copy(); return ""; } in_addr* address = (in_addr*)(record->h_addr); @@ -219,7 +219,7 @@ std::string SystemIO::ResolveHostnameToIp(const std::string& hostname, Error* er void hidra2::SystemIO::InetConnect(SocketDescriptor socket_fd, const std::string& address, Error* err) const { auto hostname_port_tuple = SplitAddressToHostnameAndPort(address); if (!hostname_port_tuple) { - *err = IOErrorTemplate::kInvalidAddressFormat->Copy(); + *err = IOErrorTemplate::kInvalidAddressFormat.Copy(); return; } std::string host; @@ -232,7 +232,7 @@ void hidra2::SystemIO::InetConnect(SocketDescriptor socket_fd, const std::string short family = AddressFamilyToPosixFamily(AddressFamilies::INET); if (family == -1) { - *err = IOErrorTemplate::kUnsupportedAddressFamily->Copy(); + *err = IOErrorTemplate::kUnsupportedAddressFamily.Copy(); return; } @@ -255,7 +255,7 @@ std::unique_ptr<std::tuple<std::string, SocketDescriptor>> SystemIO::InetAccept( Error* err) const { static short family = AddressFamilyToPosixFamily(AddressFamilies::INET); if (family == -1) { - *err = IOErrorTemplate::kUnsupportedAddressFamily->Copy(); + *err = IOErrorTemplate::kUnsupportedAddressFamily.Copy(); return nullptr; } @@ -328,7 +328,7 @@ size_t hidra2::SystemIO::Read(FileDescriptor fd, void* buf, size_t length, Error while(already_read < length) { ssize_t read_amount = _read(fd, (uint8_t*)buf + already_read, length - already_read); if(read_amount == 0) { - *err = IOErrorTemplate::kEndOfFile->Copy(); + *err = IOErrorTemplate::kEndOfFile.Copy(); return already_read; } if (read_amount == -1) { @@ -351,7 +351,7 @@ size_t hidra2::SystemIO::Write(FileDescriptor fd, const void* buf, size_t length while(already_wrote < length) { ssize_t write_amount = _write(fd, (uint8_t*)buf + already_wrote, length - already_wrote); if(write_amount == 0) { - *err = IOErrorTemplate::kEndOfFile->Copy(); + *err = IOErrorTemplate::kEndOfFile.Copy(); return already_wrote; } if (write_amount == -1) { @@ -397,19 +397,19 @@ SocketDescriptor SystemIO::CreateSocket(AddressFamilies address_family, Error* err) const { int domain = AddressFamilyToPosixFamily(address_family); if(domain == -1) { - *err = IOErrorTemplate::kUnsupportedAddressFamily->Copy(); + *err = IOErrorTemplate::kUnsupportedAddressFamily.Copy(); return -1; } int type = SocketTypeToPosixType(socket_type); if(type == -1) { - *err = IOErrorTemplate::kUnknownError->Copy(); + *err = IOErrorTemplate::kUnknownError.Copy(); return -1; } int protocol = SocketProtocolToPosixProtocol(socket_protocol); if(protocol == -1) { - *err = IOErrorTemplate::kUnknownError->Copy(); + *err = IOErrorTemplate::kUnknownError.Copy(); return -1; } @@ -432,13 +432,13 @@ void hidra2::SystemIO::InetBind(SocketDescriptor socket_fd, const std::string& a int family = AddressFamilyToPosixFamily(AddressFamilies::INET); if (family == -1) { - *err = IOErrorTemplate::kUnsupportedAddressFamily->Copy(); + *err = IOErrorTemplate::kUnsupportedAddressFamily.Copy(); return; } auto host_port_tuple = SplitAddressToHostnameAndPort(address); if (!host_port_tuple) { - *err = IOErrorTemplate::kInvalidAddressFormat->Copy(); + *err = IOErrorTemplate::kInvalidAddressFormat.Copy(); return; } std::string host; @@ -477,7 +477,7 @@ size_t hidra2::SystemIO::ReceiveTimeout(SocketDescriptor socket_fd, void* buf, s int res = ::select(socket_fd + 1, &read_fds, nullptr, nullptr, &timeout); if (res == 0) { - *err = IOErrorTemplate::kTimeout->Copy(); + *err = IOErrorTemplate::kTimeout.Copy(); return 0; } if (res == -1) { @@ -496,7 +496,7 @@ size_t hidra2::SystemIO::Receive(SocketDescriptor socket_fd, void* buf, size_t l while (already_received < length) { ssize_t received_amount = _recv(socket_fd, (uint8_t*) buf + already_received, length - already_received); if (received_amount == 0) { - *err = IOErrorTemplate::kEndOfFile->Copy(); + *err = IOErrorTemplate::kEndOfFile.Copy(); return already_received; } if (received_amount == -1) { @@ -526,7 +526,7 @@ size_t hidra2::SystemIO::Send(SocketDescriptor socket_fd, while (already_sent < length) { ssize_t send_amount = _send(socket_fd, (uint8_t*) buf + already_sent, length - already_sent); if (send_amount == 0) { - *err = IOErrorTemplate::kEndOfFile->Copy(); + *err = IOErrorTemplate::kEndOfFile.Copy(); return already_sent; } if (send_amount == -1) { diff --git a/common/cpp/src/system_io/system_io_linux.cpp b/common/cpp/src/system_io/system_io_linux.cpp index f76302b00..6cb4e8c61 100644 --- a/common/cpp/src/system_io/system_io_linux.cpp +++ b/common/cpp/src/system_io/system_io_linux.cpp @@ -29,31 +29,31 @@ Error GetLastErrorFromErrno() { case 0: return nullptr; case EBADF: - return IOErrorTemplate::kBadFileNumber->Copy(); + return IOErrorTemplate::kBadFileNumber.Copy(); case EAGAIN: - return IOErrorTemplate::kResourceTemporarilyUnavailable->Copy(); + return IOErrorTemplate::kResourceTemporarilyUnavailable.Copy(); case ENOENT: case ENOTDIR: - return IOErrorTemplate::kFileNotFound->Copy(); + return IOErrorTemplate::kFileNotFound.Copy(); case EACCES: - return IOErrorTemplate::kPermissionDenied->Copy(); + return IOErrorTemplate::kPermissionDenied.Copy(); case EFAULT: - return IOErrorTemplate::kInvalidMemoryAddress->Copy(); + return IOErrorTemplate::kInvalidMemoryAddress.Copy(); case EEXIST: - return IOErrorTemplate::kFileAlreadyExists->Copy(); + return IOErrorTemplate::kFileAlreadyExists.Copy(); case ENOSPC: - return IOErrorTemplate::kNoSpaceLeft->Copy(); + return IOErrorTemplate::kNoSpaceLeft.Copy(); case ECONNREFUSED: - return IOErrorTemplate::kConnectionRefused->Copy(); + return IOErrorTemplate::kConnectionRefused.Copy(); case EADDRINUSE: - return IOErrorTemplate::kAddressAlreadyInUse->Copy(); + return IOErrorTemplate::kAddressAlreadyInUse.Copy(); case ECONNRESET: - return IOErrorTemplate::kConnectionResetByPeer->Copy(); + return IOErrorTemplate::kConnectionResetByPeer.Copy(); case ENOTSOCK: - return IOErrorTemplate::kSocketOperationOnNonSocket->Copy(); + return IOErrorTemplate::kSocketOperationOnNonSocket.Copy(); default: std::cout << "[IOErrorsFromErrno] Unknown error code: " << errno << std::endl; - Error err = IOErrorTemplate::kUnknownError->Copy(); + Error err = IOErrorTemplate::kUnknownError.Copy(); (*err).Append("Unknown error code: " + std::to_string(errno)); return err; } diff --git a/common/cpp/unittests/json_parser/test_json_parser.cpp b/common/cpp/unittests/json_parser/test_json_parser.cpp index f03584685..f71389047 100644 --- a/common/cpp/unittests/json_parser/test_json_parser.cpp +++ b/common/cpp/unittests/json_parser/test_json_parser.cpp @@ -199,13 +199,13 @@ TEST_F(ParseFileTests, CannotReadFile) { std::string json = R"({"_id":2})"; EXPECT_CALL(mock_io, ReadFileToString_t("filename", _)). - WillOnce(DoAll(testing::SetArgPointee<1>(hidra2::IOErrorTemplate::kFileNotFound->Copy().release()), + WillOnce(DoAll(testing::SetArgPointee<1>(hidra2::IOErrorTemplate::kFileNotFound.Copy().release()), testing::Return(""))); uint64_t id; auto err = parser.GetUInt64("_id", &id); //TODO: @Sergey why not just checking error code? - ASSERT_THAT(err->Explain(), HasSubstr(hidra2::IOErrorTemplate::kFileNotFound->Copy()->Explain())); + ASSERT_THAT(err->Explain(), HasSubstr(hidra2::IOErrorTemplate::kFileNotFound.Copy()->Explain())); } diff --git a/worker/api/cpp/unittests/test_folder_broker.cpp b/worker/api/cpp/unittests/test_folder_broker.cpp index e8981ba76..c491ceca5 100644 --- a/worker/api/cpp/unittests/test_folder_broker.cpp +++ b/worker/api/cpp/unittests/test_folder_broker.cpp @@ -59,7 +59,7 @@ class FakeIO: public hidra2::MockIO { class IOFolderNotFound: public FakeIO { public: FileInfos FilesInFolder(const std::string& folder, Error* err) const override { - *err = hidra2::IOErrorTemplate::kFileNotFound->Copy(); + *err = hidra2::IOErrorTemplate::kFileNotFound.Copy(); return {}; } }; @@ -67,7 +67,7 @@ class IOFolderNotFound: public FakeIO { class IOFolderUnknownError: public FakeIO { public: FileInfos FilesInFolder(const std::string& folder, Error* err) const override { - *err = hidra2::IOErrorTemplate::kUnknownError->Copy(); + *err = hidra2::IOErrorTemplate::kUnknownError.Copy(); return {}; } }; @@ -83,7 +83,7 @@ class IOEmptyFolder: public FakeIO { class IOCannotOpenFile: public FakeIO { public: FileData GetDataFromFile(const std::string& fname, uint64_t fsize, Error* err) const noexcept override { - *err = hidra2::IOErrorTemplate::kPermissionDenied->Copy(); + *err = hidra2::IOErrorTemplate::kPermissionDenied.Copy(); return {}; }; }; @@ -121,7 +121,7 @@ TEST_F(FolderDataBrokerTests, CannotConnectWhenNoFolder) { auto return_code = data_broker->Connect(); - ASSERT_THAT(return_code->Explain(), Eq(hidra2::IOErrorTemplate::kFileNotFound->Copy()->Explain())); + ASSERT_THAT(return_code->Explain(), Eq(hidra2::IOErrorTemplate::kFileNotFound.Copy()->Explain())); } TEST_F(FolderDataBrokerTests, ConnectReturnsUnknownIOError) { @@ -129,7 +129,7 @@ TEST_F(FolderDataBrokerTests, ConnectReturnsUnknownIOError) { auto return_code = data_broker->Connect(); - ASSERT_THAT(return_code->Explain(), Eq(hidra2::IOErrorTemplate::kUnknownError->Copy()->Explain())); + ASSERT_THAT(return_code->Explain(), Eq(hidra2::IOErrorTemplate::kUnknownError.Copy()->Explain())); } TEST_F(FolderDataBrokerTests, GetNextWithoutConnectReturnsError) { @@ -175,7 +175,7 @@ TEST_F(FolderDataBrokerTests, GetNextFromEmptyFolderReturnsError) { FileInfo fi; auto err = data_broker->GetNext(&fi, nullptr); - ASSERT_THAT(err->GetErrorType(), Eq(hidra2::IOErrorTemplate::kEndOfFile->Copy()->GetErrorType())); + ASSERT_THAT(err->GetErrorType(), Eq(hidra2::IOErrorTemplate::kEndOfFile.Copy()->GetErrorType())); ASSERT_THAT(err->Explain(), Eq(hidra2::WorkerErrorMessage::kNoData)); } @@ -187,7 +187,7 @@ TEST_F(FolderDataBrokerTests, GetNextReturnsErrorWhenFilePermissionsDenied) { FileData data; auto err = data_broker->GetNext(&fi, &data); - ASSERT_THAT(err->Explain(), Eq(hidra2::IOErrorTemplate::kPermissionDenied->Copy()->Explain())); + ASSERT_THAT(err->Explain(), Eq(hidra2::IOErrorTemplate::kPermissionDenied.Copy()->Explain())); } @@ -230,22 +230,22 @@ TEST_F(GetDataFromFileTests, GetNextReturnsDataAndInfo) { TEST_F(GetDataFromFileTests, GetNextReturnsErrorWhenCannotReadData) { EXPECT_CALL(mock, GetDataFromFile_t(_, _, _)). - WillOnce(DoAll(testing::SetArgPointee<2>(hidra2::IOErrorTemplate::kReadError->Copy().release()), + WillOnce(DoAll(testing::SetArgPointee<2>(hidra2::IOErrorTemplate::kReadError.Copy().release()), testing::Return(nullptr))); auto err = data_broker->GetNext(&fi, &data); - ASSERT_THAT(err->Explain(), Eq(hidra2::IOErrorTemplate::kReadError->Copy()->Explain())); + ASSERT_THAT(err->Explain(), Eq(hidra2::IOErrorTemplate::kReadError.Copy()->Explain())); } TEST_F(GetDataFromFileTests, GetNextReturnsErrorWhenCannotAllocateData) { EXPECT_CALL(mock, GetDataFromFile_t(_, _, _)). - WillOnce(DoAll(testing::SetArgPointee<2>(hidra2::IOErrorTemplate::kMemoryAllocationError->Copy().release()), + WillOnce(DoAll(testing::SetArgPointee<2>(hidra2::IOErrorTemplate::kMemoryAllocationError.Copy().release()), testing::Return(nullptr))); auto err = data_broker->GetNext(&fi, &data); - ASSERT_THAT(err->Explain(), Eq(hidra2::IOErrorTemplate::kMemoryAllocationError->Copy()->Explain())); + ASSERT_THAT(err->Explain(), Eq(hidra2::IOErrorTemplate::kMemoryAllocationError.Copy()->Explain())); } -- GitLab