From a9de42e13be86009d75e18bb82f887d0192472d9 Mon Sep 17 00:00:00 2001 From: Toby Roseman Date: Fri, 11 Sep 2020 16:12:58 -0700 Subject: [PATCH 1/2] Fix compile errors --- src/core/storage/fileio/fs_utils.cpp | 2 +- src/core/storage/fileio/general_fstream.cpp | 2 +- src/core/storage/serialization/dir_archive.cpp | 2 +- src/core/storage/serialization/iarchive.hpp | 2 +- src/core/storage/sframe_data/sframe_index_file.cpp | 2 +- src/core/storage/sgraph_data/sgraph_triple_apply.cpp | 4 ++-- src/core/system/lambda/lambda_master.cpp | 10 +++++----- src/core/system/lambda/worker_pool.hpp | 2 +- 8 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/core/storage/fileio/fs_utils.cpp b/src/core/storage/fileio/fs_utils.cpp index b249fb5b69..fbdb463d4a 100644 --- a/src/core/storage/fileio/fs_utils.cpp +++ b/src/core/storage/fileio/fs_utils.cpp @@ -41,7 +41,7 @@ EXPORT std::string make_canonical_path(const std::string& path) { } else { return fs::canonical(fs::absolute(p)).string(); } - } catch (fs::filesystem_error e) { + } catch (fs::filesystem_error& e) { log_and_throw(std::string("Invalid path: ") + path + ". " + e.what()); } } diff --git a/src/core/storage/fileio/general_fstream.cpp b/src/core/storage/fileio/general_fstream.cpp index e14e69d9c0..fa7f7aa75d 100644 --- a/src/core/storage/fileio/general_fstream.cpp +++ b/src/core/storage/fileio/general_fstream.cpp @@ -91,7 +91,7 @@ general_ofstream::general_ofstream(std::string filename, bool gzip_compress) try : general_ofstream_base(filename, gzip_compress), opened_filename(filename) { exceptions(std::ios_base::badbit); -} catch (std::exception e) { +} catch (std::exception& e) { log_and_throw_io_failure("Cannot open " + sanitize_url(filename) + " for write. " + e.what()); } catch (std::string e) { diff --git a/src/core/storage/serialization/dir_archive.cpp b/src/core/storage/serialization/dir_archive.cpp index ba904d3daa..d99e5c1908 100644 --- a/src/core/storage/serialization/dir_archive.cpp +++ b/src/core/storage/serialization/dir_archive.cpp @@ -57,7 +57,7 @@ dir_archive_impl::archive_index_information read_index_file(std::string index_fi boost::property_tree::ptree data; try { boost::property_tree::ini_parser::read_ini(fin, data); - } catch(boost::property_tree::ini_parser_error e) { + } catch(boost::property_tree::ini_parser_error& e) { log_and_throw(std::string("Unable to parse archive index file ") + index_file); } diff --git a/src/core/storage/serialization/iarchive.hpp b/src/core/storage/serialization/iarchive.hpp index 48e59051be..daa6cab24d 100644 --- a/src/core/storage/serialization/iarchive.hpp +++ b/src/core/storage/serialization/iarchive.hpp @@ -117,7 +117,7 @@ namespace turi { template inline void read_into(T& c) { if (buf) { - memcpy(&c, buf + off, sizeof(T)); + memcpy(reinterpret_cast(&c), buf + off, sizeof(T)); off += sizeof(T); } else { in->read(reinterpret_cast(&c), sizeof(T)); diff --git a/src/core/storage/sframe_data/sframe_index_file.cpp b/src/core/storage/sframe_data/sframe_index_file.cpp index be8c26c04c..f77468dde9 100644 --- a/src/core/storage/sframe_data/sframe_index_file.cpp +++ b/src/core/storage/sframe_data/sframe_index_file.cpp @@ -28,7 +28,7 @@ sframe_index_file_information read_sframe_index_file(std::string index_file) { boost::property_tree::ptree data; try { boost::property_tree::ini_parser::read_ini(fin, data); - } catch(boost::property_tree::ini_parser_error e) { + } catch(boost::property_tree::ini_parser_error& e) { log_and_throw(std::string("Unable to parse frame index file ") + index_file); } diff --git a/src/core/storage/sgraph_data/sgraph_triple_apply.cpp b/src/core/storage/sgraph_data/sgraph_triple_apply.cpp index fc24979ffa..ec9417ed83 100644 --- a/src/core/storage/sgraph_data/sgraph_triple_apply.cpp +++ b/src/core/storage/sgraph_data/sgraph_triple_apply.cpp @@ -854,7 +854,7 @@ namespace { g.get_vertex_fields(), g.get_edge_fields(), m_srcid_column, m_dstid_column); - } catch (cppipc::ipcexception e) { + } catch (cppipc::ipcexception& e) { throw(lambda::reinterpret_comm_failure(e)); } @@ -943,7 +943,7 @@ namespace { try { mutated_edge_data = m_evaluator->proxy->eval_triple_apply(all_edge_data, m_src_partition, m_dst_partition, mutated_edge_field_ids); - } catch (cppipc::ipcexception e) { + } catch (cppipc::ipcexception& e) { throw(lambda::reinterpret_comm_failure(e)); } diff --git a/src/core/system/lambda/lambda_master.cpp b/src/core/system/lambda/lambda_master.cpp index 93abe73638..e1860927cb 100644 --- a/src/core/system/lambda/lambda_master.cpp +++ b/src/core/system/lambda/lambda_master.cpp @@ -109,7 +109,7 @@ static lambda_master* instance_ptr = nullptr; auto release_lambda_fn = [lambda_hash](std::unique_ptr& proxy) { try { proxy->release_lambda(lambda_hash); - } catch (std::exception e){ + } catch (std::exception& e){ logstream(LOG_ERROR) << "Error on releasing lambda: " << e.what() << std::endl; } catch (std::string e) { logstream(LOG_ERROR) << "Error on releasing lambda: " << e << std::endl; @@ -134,7 +134,7 @@ static lambda_master* instance_ptr = nullptr; // catch and reinterpret comm failure try { out = worker->proxy->bulk_eval(lambda_hash, args, skip_undefined, seed); - } catch (cppipc::ipcexception e) { + } catch (cppipc::ipcexception& e) { throw reinterpret_comm_failure(e); } } @@ -236,7 +236,7 @@ static lambda_master* instance_ptr = nullptr; logstream(LOG_WARNING) << "Unexpected SHMIPC failure. Falling back to CPPIPC" << std::endl; } out = worker->proxy->bulk_eval_rows(lambda_hash, args, skip_undefined, seed); - } catch (cppipc::ipcexception e) { + } catch (cppipc::ipcexception& e) { throw reinterpret_comm_failure(e); } } @@ -252,7 +252,7 @@ static lambda_master* instance_ptr = nullptr; // catch and reinterpret comm failure try { out = worker->proxy->bulk_eval_dict(lambda_hash, keys, values, skip_undefined, seed); - } catch (cppipc::ipcexception e) { + } catch (cppipc::ipcexception& e) { throw reinterpret_comm_failure(e); } } @@ -289,7 +289,7 @@ static lambda_master* instance_ptr = nullptr; logstream(LOG_WARNING) << "Unexpected SHMIPC failure. Falling back to CPPIPC" << std::endl; } out = worker->proxy->bulk_eval_dict_rows(lambda_hash, keys, rows, skip_undefined, seed); - } catch (cppipc::ipcexception e) { + } catch (cppipc::ipcexception& e) { throw reinterpret_comm_failure(e); } } diff --git a/src/core/system/lambda/worker_pool.hpp b/src/core/system/lambda/worker_pool.hpp index a421bfbb36..368d88c0af 100644 --- a/src/core/system/lambda/worker_pool.hpp +++ b/src/core/system/lambda/worker_pool.hpp @@ -385,7 +385,7 @@ class worker_pool { parallel_for(0, m_num_workers, [&](size_t i) { try { ret[i] = f(temp_workers[i]->proxy); - } catch (cppipc::ipcexception e) { + } catch (cppipc::ipcexception& e) { throw reinterpret_comm_failure(e); } }); From 296821b05a6ca830b1756a11f41560295614d6d0 Mon Sep 17 00:00:00 2001 From: Toby Roseman Date: Mon, 14 Sep 2020 12:50:39 -0700 Subject: [PATCH 2/2] Code review feedback. Use const references. --- src/core/storage/fileio/fs_utils.cpp | 2 +- src/core/storage/fileio/general_fstream.cpp | 2 +- src/core/storage/sframe_data/sframe_index_file.cpp | 2 +- src/core/system/lambda/lambda_master.cpp | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/storage/fileio/fs_utils.cpp b/src/core/storage/fileio/fs_utils.cpp index fbdb463d4a..78af3dcbab 100644 --- a/src/core/storage/fileio/fs_utils.cpp +++ b/src/core/storage/fileio/fs_utils.cpp @@ -41,7 +41,7 @@ EXPORT std::string make_canonical_path(const std::string& path) { } else { return fs::canonical(fs::absolute(p)).string(); } - } catch (fs::filesystem_error& e) { + } catch (const fs::filesystem_error& e) { log_and_throw(std::string("Invalid path: ") + path + ". " + e.what()); } } diff --git a/src/core/storage/fileio/general_fstream.cpp b/src/core/storage/fileio/general_fstream.cpp index fa7f7aa75d..83a957ab30 100644 --- a/src/core/storage/fileio/general_fstream.cpp +++ b/src/core/storage/fileio/general_fstream.cpp @@ -91,7 +91,7 @@ general_ofstream::general_ofstream(std::string filename, bool gzip_compress) try : general_ofstream_base(filename, gzip_compress), opened_filename(filename) { exceptions(std::ios_base::badbit); -} catch (std::exception& e) { +} catch (const std::exception& e) { log_and_throw_io_failure("Cannot open " + sanitize_url(filename) + " for write. " + e.what()); } catch (std::string e) { diff --git a/src/core/storage/sframe_data/sframe_index_file.cpp b/src/core/storage/sframe_data/sframe_index_file.cpp index f77468dde9..d6e55f93e1 100644 --- a/src/core/storage/sframe_data/sframe_index_file.cpp +++ b/src/core/storage/sframe_data/sframe_index_file.cpp @@ -28,7 +28,7 @@ sframe_index_file_information read_sframe_index_file(std::string index_file) { boost::property_tree::ptree data; try { boost::property_tree::ini_parser::read_ini(fin, data); - } catch(boost::property_tree::ini_parser_error& e) { + } catch(const boost::property_tree::ini_parser_error& e) { log_and_throw(std::string("Unable to parse frame index file ") + index_file); } diff --git a/src/core/system/lambda/lambda_master.cpp b/src/core/system/lambda/lambda_master.cpp index e1860927cb..7a795d402b 100644 --- a/src/core/system/lambda/lambda_master.cpp +++ b/src/core/system/lambda/lambda_master.cpp @@ -109,7 +109,7 @@ static lambda_master* instance_ptr = nullptr; auto release_lambda_fn = [lambda_hash](std::unique_ptr& proxy) { try { proxy->release_lambda(lambda_hash); - } catch (std::exception& e){ + } catch (const std::exception& e){ logstream(LOG_ERROR) << "Error on releasing lambda: " << e.what() << std::endl; } catch (std::string e) { logstream(LOG_ERROR) << "Error on releasing lambda: " << e << std::endl;