From 5cef95c9f662331e4b800c1f85eee45d0498528b Mon Sep 17 00:00:00 2001 From: Ravin Perera <33562092+ravinsp@users.noreply.github.com> Date: Fri, 28 Jan 2022 16:03:34 +0530 Subject: [PATCH] Enabled and fixed compiler warnings. (#359) --- CMakeLists.txt | 2 +- examples/c_contract/hotpocket_contract.h | 26 ++++++------ src/comm/comm_server.hpp | 10 ++--- src/comm/comm_session.cpp | 14 ++++--- src/comm/hpws.hpp | 52 ++++++++++++------------ src/conf.cpp | 16 ++++---- src/consensus.cpp | 4 +- src/crypto.cpp | 4 +- src/crypto.hpp | 2 +- src/hpfs/hpfs_mount.cpp | 4 +- src/hpfs/hpfs_serve.cpp | 8 ++-- src/hpfs/hpfs_sync.cpp | 13 +++--- src/ledger/ledger.cpp | 4 ++ src/ledger/sqlite.cpp | 4 ++ src/main.cpp | 3 +- src/msg/bson/usrmsg_bson.cpp | 2 +- src/msg/json/usrmsg_json.cpp | 6 +-- src/p2p/p2p.cpp | 4 ++ src/sc/hpfs_log_sync.cpp | 4 +- src/sc/sc.cpp | 21 +++++++--- src/sc/sc.hpp | 2 +- src/usr/input_nonce_map.cpp | 3 +- src/usr/usr.cpp | 4 +- src/usr/usr.hpp | 2 +- src/util/buffer_store.cpp | 4 +- src/util/merkle_hash_tree.cpp | 2 +- src/util/util.cpp | 12 +++--- test/local-cluster/cluster-create.sh | 2 +- 28 files changed, 132 insertions(+), 102 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 40fd06f5..260fd8cd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -10,7 +10,7 @@ set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY build) set(CMAKE_LIBRARY_OUTPUT_DIRECTORY build) set(CMAKE_RUNTIME_OUTPUT_DIRECTORY build) -set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unused-result -Wreturn-type") +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Werror") #-------hpcore------- diff --git a/examples/c_contract/hotpocket_contract.h b/examples/c_contract/hotpocket_contract.h index b3bfb6c3..83b5f08c 100644 --- a/examples/c_contract/hotpocket_contract.h +++ b/examples/c_contract/hotpocket_contract.h @@ -233,7 +233,7 @@ int hp_init_contract() } char buf[4096]; - const size_t len = read(STDIN_FILENO, buf, sizeof(buf)); + const ssize_t len = read(STDIN_FILENO, buf, sizeof(buf)); if (len == -1) { perror("Error when reading stdin."); @@ -271,14 +271,14 @@ int hp_deinit_contract() // Cleanup user and npl fd. close(cctx->users.in_fd); - for (int i = 0; i < cctx->users.count; i++) + for (size_t i = 0; i < cctx->users.count; i++) close(cctx->users.list[i].outfd); close(cctx->unl.npl_fd); // Cleanup user list allocation. if (cctx->users.list) { - for (int i = 0; i < cctx->users.count; i++) + for (size_t i = 0; i < cctx->users.count; i++) __HP_FREE(cctx->users.list[i].inputs.list); __HP_FREE(cctx->users.list); @@ -682,7 +682,7 @@ int __hp_encode_json_string_array(char *buf, const char *elems[], const size_t c struct hp_config *__hp_read_from_patch_file(const int fd) { char buf[4096]; - const size_t len = read(fd, buf, sizeof(buf)); + const ssize_t len = read(fd, buf, sizeof(buf)); if (len == -1) return NULL; @@ -722,22 +722,22 @@ int __hp_write_to_patch_file(const int fd, const struct hp_config *config) const size_t unl_buf_size = 20 + (69 * config->unl.count - (config->unl.count ? 1 : 0)) + (9 * config->unl.count); char unl_buf[unl_buf_size]; - strncpy(unl_buf, " \"unl\": [", 12); + memcpy(unl_buf, " \"unl\": [", 12); size_t pos = 12; - for (int i = 0; i < config->unl.count; i++) + for (size_t i = 0; i < config->unl.count; i++) { if (i > 0) unl_buf[pos++] = ','; - strncpy(unl_buf + pos, "\n ", 9); + memcpy(unl_buf + pos, "\n ", 9); pos += 9; unl_buf[pos++] = '"'; - strncpy(unl_buf + pos, config->unl.list[i].data, HP_KEY_SIZE); + memcpy(unl_buf + pos, config->unl.list[i].data, HP_KEY_SIZE); pos += HP_KEY_SIZE; unl_buf[pos++] = '"'; } - strncpy(unl_buf + pos, "\n ],\n", 8); + memcpy(unl_buf + pos, "\n ],\n", 8); iov_vec[1].iov_base = unl_buf; iov_vec[1].iov_len = unl_buf_size; @@ -825,7 +825,7 @@ void __hp_populate_patch_from_json_object(struct hp_config *config, const struct if (unl_count > 0) { struct json_array_element_s *unl_elem = unl_array->start; - for (int i = 0; i < unl_count; i++) + for (size_t i = 0; i < unl_count; i++) { __HP_ASSIGN_STRING(config->unl.list[i].data, unl_elem); unl_elem = unl_elem->next; @@ -956,7 +956,7 @@ void __hp_parse_args_json(const struct json_object_s *object) if (user_count > 0) { struct json_object_element_s *user_elem = user_object->start; - for (int i = 0; i < user_count; i++) + for (size_t i = 0; i < user_count; i++) { struct hp_user *user = &cctx->users.list[i]; memcpy(user->pubkey.data, user_elem->name->string, HP_KEY_SIZE); @@ -973,7 +973,7 @@ void __hp_parse_args_json(const struct json_object_s *object) // Subsequent elements are tupels of [offset, size] of input messages for this user. user->inputs.count = arr->length - 1; user->inputs.list = user->inputs.count ? (struct hp_user_input *)malloc(user->inputs.count * sizeof(struct hp_user_input)) : NULL; - for (int i = 0; i < user->inputs.count; i++) + for (size_t i = 0; i < user->inputs.count; i++) { if (arr_elem->value->type == json_type_array) { @@ -1010,7 +1010,7 @@ void __hp_parse_args_json(const struct json_object_s *object) if (unl_count > 0) { struct json_object_element_s *unl_elem = unl_obj->start; - for (int i = 0; i < unl_count; i++) + for (size_t i = 0; i < unl_count; i++) { // Each element(key) is named by the pubkey. strncpy(cctx->unl.list[i].pubkey.data, unl_elem->name->string, unl_elem->name->string_size); diff --git a/src/comm/comm_server.hpp b/src/comm/comm_server.hpp index 3bffe279..73e46a7c 100644 --- a/src/comm/comm_server.hpp +++ b/src/comm/comm_server.hpp @@ -11,7 +11,7 @@ namespace comm { - constexpr uint32_t DEFAULT_MAX_MSG_SIZE = 5 * 1024 * 1024; + constexpr uint64_t DEFAULT_MAX_MSG_SIZE = 5 * 1024 * 1024; constexpr uint64_t DEFAULT_MAX_CONNECTIONS = 99999; constexpr uint16_t MAX_INBOUND_HIGH_PRIO_BTACH = 2; // Maximum no. of incomning high priority messages to process at a time. @@ -257,13 +257,13 @@ namespace comm comm_server(std::string_view name, const uint16_t port, const uint64_t (&metric_thresholds)[5], const uint64_t max_msg_size, const uint64_t max_in_connections, const uint64_t max_in_connections_per_host, const bool use_priority_queues) - : name(name), - listen_port(port), - metric_thresholds(metric_thresholds), + : metric_thresholds(metric_thresholds), max_msg_size(max_msg_size > 0 ? max_msg_size : DEFAULT_MAX_MSG_SIZE), max_in_connections(max_in_connections > 0 ? max_in_connections : DEFAULT_MAX_CONNECTIONS), max_in_connections_per_host(max_in_connections_per_host > 0 ? max_in_connections_per_host : DEFAULT_MAX_CONNECTIONS), - use_priority_queues(use_priority_queues) + use_priority_queues(use_priority_queues), + name(name), + listen_port(port) { } diff --git a/src/comm/comm_session.cpp b/src/comm/comm_session.cpp index 45868c7b..8eee1f37 100644 --- a/src/comm/comm_session.cpp +++ b/src/comm/comm_session.cpp @@ -15,13 +15,13 @@ namespace comm comm_session::comm_session(corebill::tracker &violation_tracker, std::string_view host_address, hpws::client &&hpws_client, const bool is_ipv4, const bool is_inbound, const uint64_t (&metric_thresholds)[5]) : violation_tracker(violation_tracker), - uniqueid(host_address), - host_address(host_address), hpws_client(std::move(hpws_client)), - is_ipv4(is_ipv4), - is_inbound(is_inbound), in_msg_queue1(MAX_IN_MSG_QUEUE_SIZE), - in_msg_queue2(MAX_IN_MSG_QUEUE_SIZE) + in_msg_queue2(MAX_IN_MSG_QUEUE_SIZE), + uniqueid(host_address), + is_inbound(is_inbound), + is_ipv4(is_ipv4), + host_address(host_address) { // Create new session_thresholds and insert it to thresholds vector. // Have to maintain the SESSION_THRESHOLDS enum order in inserting new thresholds to thresholds vector @@ -69,7 +69,9 @@ namespace comm should_disconnect = true; const hpws::error error = std::get(read_result); if (error.first != 1) // 1 indicates channel has closed. + { LOG_DEBUG << "hpws client read failed:" << error.first << " " << error.second; + } } else { @@ -101,7 +103,9 @@ namespace comm enqueued = in_msg_queue2.try_enqueue(std::move(msg)); if (!enqueued) + { LOG_WARNING << "Failed to enqueue comm msg."; + } } // Signal the hpws client that we are ready for next message. diff --git a/src/comm/hpws.hpp b/src/comm/hpws.hpp index aa287bff..bc73eaab 100644 --- a/src/comm/hpws.hpp +++ b/src/comm/hpws.hpp @@ -65,13 +65,13 @@ namespace hpws private: pid_t child_pid = 0; // if this client was created by a connect this is set // this value can't be changed once it's established between the processes - uint32_t max_buffer_size; + uint32_t max_buffer_size = 0; bool moved = false; - addr_t endpoint; - std::string get; // the get req this websocket was opened with - int control_line_fd[2]; // see below in client constructor - int buffer_fd[4]; // 0 1 - in buffers, 2 3 - out buffers - int buffer_lock[2] = {0, 0}; // this records if buffers 2 and 3 have been sent out awaiting an ack or not + addr_t endpoint = {}; + std::string get; // the get req this websocket was opened with + int control_line_fd[2] = {0, 0}; // see below in client constructor + int buffer_fd[4] = {0, 0, 0, 0}; // 0 1 - in buffers, 2 3 - out buffers + int buffer_lock[2] = {0, 0}; // this records if buffers 2 and 3 have been sent out awaiting an ack or not void *buffer[4]; // private constructor @@ -83,10 +83,11 @@ namespace hpws uint32_t max_buffer_size, pid_t child_pid, int buffer_fd[4], - void *buffer[4]) : endpoint(endpoint), - is_ipv4(endpoint.sa.sa_family == AF_INET), + void *buffer[4]) : child_pid(child_pid), max_buffer_size(max_buffer_size), - child_pid(child_pid), get(get) + endpoint(endpoint), + get(get), + is_ipv4(endpoint.sa.sa_family == AF_INET) { control_line_fd[0] = control_line_fd_0; control_line_fd[1] = control_line_fd_1; @@ -110,8 +111,8 @@ namespace hpws client(client &&old) : child_pid(old.child_pid), max_buffer_size(old.max_buffer_size), endpoint(old.endpoint), - is_ipv4(old.is_ipv4), - get(old.get) + get(old.get), + is_ipv4(old.is_ipv4) { old.moved = true; for (int i = 0; i <= 1; ++i) @@ -429,10 +430,10 @@ namespace hpws // first thing we'll receive is the sockaddr union addr_t child_addr; - int bytes_read = + ssize_t bytes_read = recv(child_fd[0], (unsigned char *)(&child_addr), sizeof(child_addr), 0); - if (bytes_read < sizeof(child_addr)) + if (bytes_read < (ssize_t)sizeof(child_addr)) HPWS_CONNECT_ERROR(202, "received message on control line was not sizeof(addr_t)"); if (HPWS_DEBUG) @@ -446,8 +447,7 @@ namespace hpws child_msg.msg_control = cmsgbuf; child_msg.msg_controllen = sizeof(cmsgbuf); - int bytes_read = - recvmsg(child_fd[0], &child_msg, 0); + recvmsg(child_fd[0], &child_msg, 0); struct cmsghdr *cmsg = CMSG_FIRSTHDR(&child_msg); if (cmsg == NULL || cmsg->cmsg_type != SCM_RIGHTS) HPWS_CONNECT_ERROR(203, "non-scm_rights message sent on accept child control line"); @@ -578,9 +578,9 @@ namespace hpws if (pid_child > 0) { - int ret1 = kill(pid_child, SIGTERM); + kill(pid_child, SIGTERM); int wstat; - int ret2 = waitpid(pid_child, &wstat, 0); + waitpid(pid_child, &wstat, 0); } } @@ -634,7 +634,8 @@ namespace hpws *(uint32_t *)&buf[len - 4] = ttl_sec; - write(this->master_control_fd_, buf, len); + if (write(this->master_control_fd_, buf, len) == -1) + ; // Do nothing. } void unban_ip(const uint32_t *addr, const bool ipv4) @@ -658,7 +659,8 @@ namespace hpws addr_buf[3] = addr[4]; } - write(this->master_control_fd_, buf, len); + if (write(this->master_control_fd_, buf, len) == -1) + ; } std::variant accept(const bool no_block = false) @@ -713,8 +715,7 @@ namespace hpws if (HPWS_DEBUG) fprintf(stderr, "[HPWS.HPP] Accept[3] called %d\n", calls); - int bytes_read = - recvmsg(this->master_control_fd_, &child_msg, 0); + recvmsg(this->master_control_fd_, &child_msg, 0); struct cmsghdr *cmsg = CMSG_FIRSTHDR(&child_msg); if (cmsg == NULL || cmsg->cmsg_type != SCM_RIGHTS) HPWS_ACCEPT_ERROR(200, "non-scm_rights message sent on master control line"); @@ -748,7 +749,7 @@ namespace hpws HPWS_ACCEPT_ERROR(202, "timeout waiting for hpws accept child message"); // first thing we'll receive is the pid of the client - if (recv(child_fd[0], (unsigned char *)(&pid), sizeof(pid), 0) < sizeof(pid)) + if (recv(child_fd[0], (unsigned char *)(&pid), sizeof(pid), 0) < (ssize_t)sizeof(pid)) HPWS_ACCEPT_ERROR(212, "did not receive expected 4 byte pid of child process on accept"); if (HPWS_DEBUG) @@ -756,13 +757,13 @@ namespace hpws // second thing we'll receive is IP address structure of the client addr_t buf; - int bytes_read = + ssize_t bytes_read = recv(child_fd[0], (unsigned char *)(&buf), sizeof(buf), 0); if (HPWS_DEBUG) fprintf(stderr, "[HPWS.HPP] Accept[8] called %d\n", calls); - if (bytes_read < sizeof(buf)) + if (bytes_read < (ssize_t)sizeof(buf)) HPWS_ACCEPT_ERROR(202, "received message on master control line was not sizeof(sockaddr_in6)"); // third thing we will receive is the four fds for the buffers @@ -776,8 +777,7 @@ namespace hpws if (HPWS_DEBUG) fprintf(stderr, "[HPWS.HPP] Accept[9] called %d\n", calls); - int bytes_read = - recvmsg(child_fd[0], &child_msg, 0); + recvmsg(child_fd[0], &child_msg, 0); struct cmsghdr *cmsg = CMSG_FIRSTHDR(&child_msg); if (cmsg == NULL || cmsg->cmsg_type != SCM_RIGHTS) HPWS_ACCEPT_ERROR(203, "non-scm_rights message sent on accept child control line"); diff --git a/src/conf.cpp b/src/conf.cpp index 5f5ac6dd..af7b688e 100644 --- a/src/conf.cpp +++ b/src/conf.cpp @@ -66,7 +66,9 @@ namespace conf if (init_success) { if (persist_updated_configs() == -1) + { LOG_ERROR << "Failed to persist config updates."; + } // Releases the config file lock at the termination. release_config_lock(); @@ -203,7 +205,8 @@ namespace conf // We don't mind if this command fails, because when running the contract we'll check and inform the user that // tls key files are missing, so they can create them manually. - system(tls_command.c_str()); + if (system(tls_command.c_str()) == -1) + std::cerr << errno << ": tls cert generation failed."; } std::cout << "Contract directory created at " << ctx.contract_dir << std::endl; @@ -217,24 +220,21 @@ namespace conf */ void set_contract_dir_paths(std::string exepath, std::string basedir) { + // resolving the path through realpath will remove any trailing slash if present + exepath = util::realpath(exepath); + if (exepath.empty()) { - // this code branch will never execute the way main is currently coded, but it might change in future std::cerr << "Executable path must be specified\n"; exit(1); } if (basedir.empty()) { - // this code branch will never execute the way main is currently coded, but it might change in future std::cerr << "a contract directory must be specified\n"; exit(1); } - // resolving the path through realpath will remove any trailing slash if present - basedir = util::realpath(basedir); - exepath = util::realpath(exepath); - // Take the parent directory path. ctx.exe_dir = dirname(exepath.data()); @@ -1096,7 +1096,7 @@ namespace conf if (!contract.bin_args.empty()) util::split_string(contract.runtime_binexec_args, contract.bin_args, " "); contract.runtime_binexec_args.insert(contract.runtime_binexec_args.begin(), contract.bin_path); - + // Uncomment for docker-based execution. // std::string volumearg; // volumearg.append("type=bind,source=").append(ctx.contract_hpfs_dir).append(",target=/hpfs"); diff --git a/src/consensus.cpp b/src/consensus.cpp index f2e8f39c..38ed22c8 100644 --- a/src/consensus.cpp +++ b/src/consensus.cpp @@ -247,7 +247,7 @@ namespace consensus std::map cp_root_hash; // Stores one of proposal to match its root hash. util::h32 majority_hash = util::h32_empty; - int stage3_prop_count = 0; // Keep track of the number of stage 3 proposals received. + uint32_t stage3_prop_count = 0; // Keep track of the number of stage 3 proposals received. // Count votes of all stage 3 proposal hashes. for (const auto &[pubkey, cp] : ctx.candidate_proposals) @@ -330,7 +330,9 @@ namespace consensus util::sequence_hash lcl_id = ledger::ctx.get_lcl_id(); if (!ctx.sync_ongoing) + { LOG_INFO << "****Ledger created**** (lcl:" << lcl_id << " state:" << cons_prop.state_hash << " patch:" << cons_prop.patch_hash << ")"; + } // Now that there's a new ledger, prune any newly-expired candidate inputs. expire_candidate_inputs(lcl_id); diff --git a/src/crypto.cpp b/src/crypto.cpp index 9014f8c8..e57c636b 100644 --- a/src/crypto.cpp +++ b/src/crypto.cpp @@ -29,10 +29,10 @@ namespace crypto // Currently using ed25519. So append prefix byte to represent that. pubkey.resize(crypto_sign_ed25519_PUBLICKEYBYTES + 1); - pubkey[0] = KEYPFX_ed25519; + pubkey[0] = crypto::KEYPFX_ed25519; seckey.resize(crypto_sign_ed25519_SECRETKEYBYTES + 1); - seckey[0] = KEYPFX_ed25519; + seckey[0] = crypto::KEYPFX_ed25519; crypto_sign_ed25519_keypair( reinterpret_cast(pubkey.data() + 1), // +1 to skip the prefix byte. diff --git a/src/crypto.hpp b/src/crypto.hpp index 630eeb3e..30ea7a2c 100644 --- a/src/crypto.hpp +++ b/src/crypto.hpp @@ -11,7 +11,7 @@ namespace crypto { // Prefix byte to append to ed25519 keys. - static unsigned char KEYPFX_ed25519 = 0xED; + constexpr const unsigned char KEYPFX_ed25519 = 0xED; int init(); diff --git a/src/hpfs/hpfs_mount.cpp b/src/hpfs/hpfs_mount.cpp index d00c253c..21adb5cd 100644 --- a/src/hpfs/hpfs_mount.cpp +++ b/src/hpfs/hpfs_mount.cpp @@ -152,7 +152,7 @@ namespace hpfs (char *)(is_full_history ? NULL : "-g"), NULL}; - const int ret = execv(execv_args[0], execv_args); + execv(execv_args[0], execv_args); std::cerr << errno << ": hpfs process execv failed at mount " << mount_dir << ".\n"; exit(1); @@ -586,7 +586,7 @@ namespace hpfs return -1; } - if (pread(fd, &hash, sizeof(util::h32), offset) < sizeof(util::h32)) + if (pread(fd, &hash, sizeof(util::h32), offset) < (ssize_t)sizeof(util::h32)) { LOG_ERROR << errno << ": Error reading hash from the given offset " << offset; close(fd); diff --git a/src/hpfs/hpfs_serve.cpp b/src/hpfs/hpfs_serve.cpp index b5d6d09e..1661a985 100644 --- a/src/hpfs/hpfs_serve.cpp +++ b/src/hpfs/hpfs_serve.cpp @@ -70,8 +70,6 @@ namespace hpfs prev_requests_processed = !hpfs_requests.empty(); const uint64_t time_start = util::get_epoch_milliseconds(); - const util::sequence_hash lcl_id = ledger::ctx.get_lcl_id(); - const util::sequence_hash last_primary_shard_id = ledger::ctx.get_last_primary_shard_id(); const uint32_t request_batch_timeout = hpfs::get_request_resubmit_timeout() * 0.9; if (hpfs_requests.empty()) @@ -474,12 +472,12 @@ namespace hpfs } else { - const size_t read_len = MIN(hpfs::BLOCK_SIZE, (st.st_size - block_offset)); + const size_t read_len = MIN(hpfs::BLOCK_SIZE, (size_t)(st.st_size - block_offset)); block.resize(read_len); lseek(fd, block_offset, SEEK_SET); - const int res = read(fd, block.data(), read_len); - if (res < read_len) + const ssize_t res = read(fd, block.data(), read_len); + if (res == -1 || (size_t)res < read_len) { LOG_ERROR << errno << ": Read failed (result:" << res << " off:" << block_offset << " len:" << read_len << "). " << file_path; diff --git a/src/hpfs/hpfs_sync.cpp b/src/hpfs/hpfs_sync.cpp index d0d82447..c80bbda4 100644 --- a/src/hpfs/hpfs_sync.cpp +++ b/src/hpfs/hpfs_sync.cpp @@ -355,7 +355,7 @@ namespace hpfs std::set responded_block_ids; { - const flatbuffers::Vector *fbvec = file_resp.responded_block_ids(); + file_resp.responded_block_ids(); const uint32_t *ptr = file_resp.responded_block_ids()->data(); const size_t count = file_resp.responded_block_ids()->size(); for (size_t i = 0; i < count; i++) @@ -498,7 +498,7 @@ namespace hpfs content_hash ^= crypto::get_hash(mode_bytes, sizeof(mode_bytes)); // Then XOR the block hashes to the initial hash. - for (int32_t block_id = 0; block_id < hash_count; block_id++) + for (size_t block_id = 0; block_id < hash_count; block_id++) { content_hash ^= hashes[block_id]; } @@ -648,7 +648,7 @@ namespace hpfs std::string child_physical_path = fs_mount->physical_path(hpfs::RW_SESSION_NAME, child_vpath); if ((entry.is_file && unlink(child_physical_path.c_str()) == -1) || - !entry.is_file && util::remove_directory_recursively(child_physical_path.c_str()) == -1) + (!entry.is_file && util::remove_directory_recursively(child_physical_path.c_str()) == -1)) return -1; write_performed = true; @@ -681,7 +681,6 @@ namespace hpfs const size_t existing_hash_count = existing_hashes.size(); // Compare the block hashes and request any differences. - auto insert_itr = pending_requests.begin(); const int32_t max_block_id = MAX(existing_hash_count, hash_count) - 1; for (int32_t block_id = 0; block_id <= max_block_id; block_id++) { @@ -690,7 +689,7 @@ namespace hpfs // The peer has already responded with a hint response. So we must start watching for it. submit_request(sync_item{SYNC_ITEM_TYPE::BLOCK, std::string(vpath), block_id, hashes[block_id]}, true); } - else if (block_id >= existing_hash_count || existing_hashes[block_id] != hashes[block_id]) + else if (block_id >= (int32_t)existing_hash_count || existing_hashes[block_id] != hashes[block_id]) { pending_requests.emplace(sync_item{SYNC_ITEM_TYPE::BLOCK, std::string(vpath), block_id, hashes[block_id]}); } @@ -735,9 +734,9 @@ namespace hpfs } const off_t offset = block_id * hpfs::BLOCK_SIZE; - const int res = pwrite(fd, buf.data(), buf.length(), offset); + const ssize_t res = pwrite(fd, buf.data(), buf.length(), offset); close(fd); - if (res < buf.length()) + if (res == -1 || (size_t)res < buf.length()) { LOG_ERROR << errno << " Write failed " << file_physical_path; return -1; diff --git a/src/ledger/ledger.cpp b/src/ledger/ledger.cpp index b6b35892..269ad7cc 100644 --- a/src/ledger/ledger.cpp +++ b/src/ledger/ledger.cpp @@ -419,11 +419,15 @@ namespace ledger const std::string file_path = shard_path + file_name; int fd = open(file_path.data(), O_WRONLY | O_APPEND | O_CREAT, FILE_PERMS); if (fd == -1) + { LOG_ERROR << errno << ": Error when creating file " << file_path; + } struct stat st; if (fstat(fd, &st) == -1) + { LOG_ERROR << errno << ": Error when stat of file " << file_path; + } file_size = st.st_size; return fd; diff --git a/src/ledger/sqlite.cpp b/src/ledger/sqlite.cpp index 915eff96..014b74e1 100644 --- a/src/ledger/sqlite.cpp +++ b/src/ledger/sqlite.cpp @@ -149,7 +149,9 @@ namespace ledger::sqlite const int ret = exec_sql(db, sql); if (ret == -1) + { LOG_ERROR << "Error when creating sqlite table " << table_name; + } return ret; } @@ -170,7 +172,9 @@ namespace ledger::sqlite const int ret = exec_sql(db, sql); if (ret == -1) + { LOG_ERROR << "Error when creating sqlite index '" << index_name << "' in table " << table_name; + } return ret; } diff --git a/src/main.cpp b/src/main.cpp index 36f10c70..12d02f47 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -197,7 +197,8 @@ int main(int argc, char **argv) // Set HP process cwd to the contract directory. This will make both HP and contract process // both have the same cwd. - chdir(conf::ctx.contract_dir.c_str()); + if (chdir(conf::ctx.contract_dir.c_str()) != 0) + return -1; hplog::init(); diff --git a/src/msg/bson/usrmsg_bson.cpp b/src/msg/bson/usrmsg_bson.cpp index da9dda7a..d563194b 100644 --- a/src/msg/bson/usrmsg_bson.cpp +++ b/src/msg/bson/usrmsg_bson.cpp @@ -218,7 +218,7 @@ namespace msg::usrmsg::bson encoder.key(msg::usrmsg::FLD_OUTPUTS); encoder.begin_array(); - for (int i = 0; i < outputs.size(); i++) + for (size_t i = 0; i < outputs.size(); i++) encoder.byte_string_value(outputs[i]); encoder.end_array(); diff --git a/src/msg/json/usrmsg_json.cpp b/src/msg/json/usrmsg_json.cpp index 321a1816..c0a18829 100644 --- a/src/msg/json/usrmsg_json.cpp +++ b/src/msg/json/usrmsg_json.cpp @@ -407,7 +407,7 @@ namespace msg::usrmsg::json msg += msg::usrmsg::FLD_OUTPUTS; msg += "\":["; - for (int i = 0; i < outputs.size(); i++) + for (size_t i = 0; i < outputs.size(); i++) { std::string_view output = outputs[i]; @@ -446,7 +446,7 @@ namespace msg::usrmsg::json msg += msg::usrmsg::FLD_UNL_SIG; msg += "\":["; - for (int i = 0; i < unl_sig.size(); i++) + for (size_t i = 0; i < unl_sig.size(); i++) { const auto &sig = unl_sig[i]; // Pubkey and Signature pair. msg += "[\""; @@ -482,7 +482,7 @@ namespace msg::usrmsg::json msg += msg::usrmsg::FLD_UNL; msg += "\":["; - int i = 0; + size_t i = 0; for (std::string_view unl : unl_list) { msg += DOUBLE_QUOTE; diff --git a/src/p2p/p2p.cpp b/src/p2p/p2p.cpp index 145d1ff1..b39b7b4c 100644 --- a/src/p2p/p2p.cpp +++ b/src/p2p/p2p.cpp @@ -374,7 +374,9 @@ namespace p2p void handle_npl_message(const p2p::npl_message &npl) { if (!consensus::push_npl_message(npl)) + { LOG_DEBUG << "NPL message from self enqueue failure."; + } } /** @@ -457,7 +459,9 @@ namespace p2p send_message_to_random_peer(fbuf, target_pubkey); if (!target_pubkey.empty()) + { LOG_DEBUG << "Peer list requested from [" << target_pubkey.substr(0, 10) << "]"; + } } /** diff --git a/src/sc/hpfs_log_sync.cpp b/src/sc/hpfs_log_sync.cpp index 8c99e3ba..2facfdab 100644 --- a/src/sc/hpfs_log_sync.cpp +++ b/src/sc/hpfs_log_sync.cpp @@ -135,10 +135,12 @@ namespace sc::hpfs_log_sync std::string target_pubkey; p2p::send_message_to_random_peer(fbuf, target_pubkey, true); if (!target_pubkey.empty()) + { LOG_DEBUG << "Hpfs log sync: Requesting from [" << target_pubkey.substr(2, 8) << "]." << " min:" << sync_ctx.min_log_record.seq_no << " target:" << sync_ctx.target_log_seq_no; - + } + sync_ctx.target_requested_on = time_now; sync_ctx.request_submissions++; } diff --git a/src/sc/sc.cpp b/src/sc/sc.cpp index 07669c86..de2af645 100644 --- a/src/sc/sc.cpp +++ b/src/sc/sc.cpp @@ -197,13 +197,13 @@ namespace sc char *execv_args[execv_len]; int j = 0; - for (int i = 0; i < conf::cfg.contract.runtime_binexec_args.size(); i++, j++) + for (size_t i = 0; i < conf::cfg.contract.runtime_binexec_args.size(); i++, j++) execv_args[j] = conf::cfg.contract.runtime_binexec_args[i].data(); execv_args[execv_len - 1] = NULL; const int env_len = conf::cfg.contract.runtime_env_args.size() + 1; char *env_args[env_len]; - for (int i = 0; i < conf::cfg.contract.runtime_env_args.size(); i++) + for (size_t i = 0; i < conf::cfg.contract.runtime_env_args.size(); i++) env_args[i] = conf::cfg.contract.runtime_env_args[i].data(); env_args[env_len - 1] = NULL; @@ -459,7 +459,7 @@ namespace sc struct pollfd out_fds[out_fd_count]; auto user_itr = ctx.user_fds.begin(); - for (int i = 0; i < out_fd_count; i++) + for (size_t i = 0; i < out_fd_count; i++) { const int fd = (user_itr != ctx.user_fds.end()) ? (user_itr++)->second.hpfd : (i == control_fd_idx ? ctx.control_fds.hpfd : ctx.npl_fds.hpfd); @@ -469,7 +469,7 @@ namespace sc while (!ctx.is_shutting_down) { // Reset the revents because we are reusing same pollfd list. - for (int i = 0; i < out_fd_count; i++) + for (size_t i = 0; i < out_fd_count; i++) out_fds[i].revents = 0; if (poll(out_fds, out_fd_count, 20) == -1) @@ -814,7 +814,6 @@ namespace sc { // Get fds for the pubkey. std::string output; - fd_pair &fds = fdmap[pubkey]; // This returns the total bytes read from the socket. const int total_bytes_read = (pfds[i].fd == -1) ? 0 : read_iosocket(true, pfds[i], output); @@ -1026,7 +1025,9 @@ namespace sc output.resize(res); // Resize back to the actual bytes read. if (res == -1) + { LOG_ERROR << errno << ": Error reading from contract socket. stream:" << is_stream_socket; + } return res; } @@ -1140,7 +1141,7 @@ namespace sc * @param depth Depth of the recursion. Starts with zero and traverse down. * @return 0 on success and -1 on error. */ - int rename_and_cleanup_contract_log_files(const std::string &session_name, std::string_view postfix, const int depth) + int rename_and_cleanup_contract_log_files(const std::string &session_name, std::string_view postfix, const size_t depth) { const std::string prefix = (depth == 0) ? session_name : (session_name + "_" + std::to_string(depth)); const std::string fliename = conf::ctx.contract_log_dir + "/" + prefix + postfix.data(); @@ -1157,7 +1158,9 @@ namespace sc // Last allowed file. remove this to make room for the new one. const int res = util::remove_file(fliename); if (res == -1) + { LOG_ERROR << errno << ": Error removing " << fliename << " to make room for new log file."; + } return res; } @@ -1166,7 +1169,9 @@ namespace sc const std::string new_filename = conf::ctx.contract_log_dir + "/" + session_name + "_" + std::to_string(depth + 1) + postfix.data(); const int res = rename(fliename.data(), new_filename.data()); if (res == -1) + { LOG_ERROR << errno << ": Error occured while renaming " << fliename << " to " << new_filename; + } return res; } @@ -1185,14 +1190,18 @@ namespace sc while (util::is_file_exists(filename)) { if (util::remove_file(filename) == -1) + { LOG_ERROR << "Error removing " << filename << " during contract log file cleanup."; + } filename = fliename_common_part + std::to_string(++current) + postfix.data(); } const int removed_count = current - start_point; if (removed_count > 0) + { LOG_DEBUG << (current - start_point) << " " << postfix << " contract log files cleaned up with log file count change."; + } } } // namespace sc diff --git a/src/sc/sc.hpp b/src/sc/sc.hpp index 47e88fcf..94bcf848 100644 --- a/src/sc/sc.hpp +++ b/src/sc/sc.hpp @@ -213,7 +213,7 @@ namespace sc void handle_control_msg(execution_context &ctx, std::string_view msg); - int rename_and_cleanup_contract_log_files(const std::string &prefix, std::string_view postfix, const int depth = 0); + int rename_and_cleanup_contract_log_files(const std::string &prefix, std::string_view postfix, const size_t depth = 0); void clean_extra_contract_log_files(const std::string &session_name, std::string_view postfix, const int start_point); diff --git a/src/usr/input_nonce_map.cpp b/src/usr/input_nonce_map.cpp index 85d39593..ae5ecea2 100644 --- a/src/usr/input_nonce_map.cpp +++ b/src/usr/input_nonce_map.cpp @@ -18,8 +18,7 @@ namespace usr int input_nonce_map::check(const std::string &pubkey, const uint64_t &nonce, const std::string &sig, const uint64_t &max_ledger_seq_no, const bool no_add) { int result = 0; - - const uint64_t now = util::get_epoch_milliseconds(); + auto itr = nonce_map.find(pubkey); if (itr == nonce_map.end()) { diff --git a/src/usr/usr.cpp b/src/usr/usr.cpp index f7c54e4b..d47fd6ba 100644 --- a/src/usr/usr.cpp +++ b/src/usr/usr.cpp @@ -152,7 +152,9 @@ namespace usr if (parser.extract_read_request(content) != -1) { if (read_req::populate_read_req_queue(user.pubkey, std::move(content)) == -1) + { LOG_WARNING << "Failed to enqueue read request."; + } return 0; } else @@ -402,7 +404,7 @@ namespace usr int remove_user(const std::string &pubkey) { std::scoped_lock lock(ctx.users_mutex); - const auto itr = ctx.users.erase(pubkey); + ctx.users.erase(pubkey); return 0; } diff --git a/src/usr/usr.hpp b/src/usr/usr.hpp index 21e06cf7..6e9ceef3 100644 --- a/src/usr/usr.hpp +++ b/src/usr/usr.hpp @@ -49,7 +49,7 @@ namespace usr * @param pubkey The public key of the user in binary format. */ connected_user(usr::user_comm_session &session, std::string_view pubkey, util::PROTOCOL protocol) - : session(session), pubkey(pubkey), protocol(protocol) + : pubkey(pubkey), session(session), protocol(protocol) { // Default subscriptions. subscriptions[NOTIFICATION_CHANNEL::UNL_CHANGE] = false; diff --git a/src/util/buffer_store.cpp b/src/util/buffer_store.cpp index 1fca5a79..987a7cf4 100644 --- a/src/util/buffer_store.cpp +++ b/src/util/buffer_store.cpp @@ -23,7 +23,7 @@ namespace util const buffer_view buffer_store::write_buf(const void *buf, const uint32_t size) { buffer_view view = {0, 0}; - int res = pwrite(fd, buf, size, next_write_pos); + ssize_t res = pwrite(fd, buf, size, next_write_pos); if (res < size) { LOG_ERROR << errno << ": Error writing to buffer store fd " << fd; @@ -51,7 +51,7 @@ namespace util int buffer_store::read_buf(const buffer_view &view, std::string &buf) { buf.resize(view.size); - const int res = pread(fd, buf.data(), view.size, view.offset); + const ssize_t res = pread(fd, buf.data(), view.size, view.offset); if (res < view.size) { LOG_ERROR << errno << ": Error reading from buffer store fd " << fd; diff --git a/src/util/merkle_hash_tree.cpp b/src/util/merkle_hash_tree.cpp index 96b542ec..20e99849 100644 --- a/src/util/merkle_hash_tree.cpp +++ b/src/util/merkle_hash_tree.cpp @@ -24,7 +24,7 @@ namespace util std::list parents; const size_t blocks = (nodes.size() + block_size - 1) / block_size; - for (int i = 0; i < blocks; i++) + for (size_t i = 0; i < blocks; i++) { parents.push_back({}); merkle_hash_node &parent = parents.back(); diff --git a/src/util/util.cpp b/src/util/util.cpp index 28b6734d..853e6e66 100644 --- a/src/util/util.cpp +++ b/src/util/util.cpp @@ -61,7 +61,9 @@ namespace util const std::string realpath(const std::string &path) { std::array buffer; - ::realpath(path.c_str(), buffer.data()); + if (!::realpath(path.c_str(), buffer.data())) + return {}; + buffer[PATH_MAX] = '\0'; return buffer.data(); } @@ -187,11 +189,11 @@ namespace util DIR *dr; // Open the directory stream. - if (dr = opendir(path.data())) + if ((dr = opendir(path.data()))) { // Take next directory entry from the directory stream. struct dirent *en; - while (en = readdir(dr)) + while ((en = readdir(dr))) { // Push into the entries list if reading directory entry is not current directory entry // or previous directory entry. @@ -365,13 +367,13 @@ namespace util */ int read_from_fd(const int fd, void *buf, const size_t size, const off_t offset, std::string_view file_name) { - const int res = pread(fd, buf, size, offset); + const ssize_t res = pread(fd, buf, size, offset); if (res == -1) { LOG_ERROR << errno << ": Error when reading " << file_name; return -1; } - else if (res < size) + else if ((size_t)res < size) { LOG_ERROR << "Not enough bytes read from " << file_name; return -1; diff --git a/test/local-cluster/cluster-create.sh b/test/local-cluster/cluster-create.sh index dba8d675..9d19d948 100755 --- a/test/local-cluster/cluster-create.sh +++ b/test/local-cluster/cluster-create.sh @@ -23,7 +23,7 @@ iprange="172.1.1" if [ "$CONTRACT" = "cecho" ]; then # C echo contract echo "Using C echo contract." pushd $hpcore/examples/c_contract/ > /dev/null 2>&1 - gcc echo_contract.c -o echo_contract + gcc echo_contract.c -o echo_contract -Wall -Werror popd > /dev/null 2>&1 copyfiles="$hpcore/examples/c_contract/echo_contract" binary="echo_contract"