You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by GitBox <gi...@apache.org> on 2022/11/06 06:39:56 UTC
[GitHub] [incubator-pegasus] acelyc111 opened a new pull request, #1226: refactor(macro): use CHECK_* to replace dassert_* (part 3)
acelyc111 opened a new pull request, #1226:
URL: https://github.com/apache/incubator-pegasus/pull/1226
https://github.com/apache/incubator-pegasus/issues/1204
- all `dassert` and `dassert_f` have been removed
- `dassert` -> `CHECK`
- `dassert/dassert_f(a == b, msg)` -> `CHECK_EQ_MSG(a, b, msg)`
- `dassert/dassert_f(a > b, msg)` -> `CHECK_GT_MSG(a, b, msg)`
- `dassert/dassert_f(a >= b, msg)` -> `CHECK_GE_MSG(a, b, msg)`
- `dassert/dassert_f(a < b, msg)` -> `CHECK_LT_MSG(a, b, msg)`
- `dassert/dassert_f(a <= b, msg)` -> `CHECK_LE_MSG(a, b, msg)`
- remove some verbose and meanless messages
TODO: refactor dangling-else `CHECK(false, ...)`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] empiredan commented on a diff in pull request #1226: refactor(macro): use CHECK_* to replace dassert_* (part 3)
Posted by GitBox <gi...@apache.org>.
empiredan commented on code in PR #1226:
URL: https://github.com/apache/incubator-pegasus/pull/1226#discussion_r1014822947
##########
src/runtime/rpc/network.cpp:
##########
@@ -548,7 +548,7 @@ void network::on_recv_reply(uint64_t id, message_ex *msg, int delay_ms)
message_parser *network::new_message_parser(network_header_format hdr_format)
{
message_parser *parser = message_parser_manager::instance().create_parser(hdr_format);
- dassert(parser, "message parser '%s' not registerd or invalid!", hdr_format.to_string());
+ CHECK(parser, "message parser '{}' not registerd or invalid!", hdr_format);
Review Comment:
```suggestion
CHECK_NOTNULL(parser, "message parser '{}' not registerd or invalid!", hdr_format);
```
##########
src/replica/prepare_list.cpp:
##########
@@ -196,7 +196,7 @@ void prepare_list::commit(decree d, commit_type ct)
return;
}
default:
- dassert(false, "invalid commit type %d", (int)ct);
+ CHECK(false, "invalid commit type {}", (int)ct);
Review Comment:
```suggestion
CHECK(false, "invalid commit type {}", ct);
```
##########
src/runtime/rpc/asio_net_provider.cpp:
##########
@@ -396,9 +394,7 @@ error_code asio_udp_provider::start(rpc_channel channel, int port, bool client_o
boost::asio::io_service::work work(_io_service);
boost::system::error_code ec;
_io_service.run(ec);
- if (ec) {
- dassert(false, "boost::asio::io_service run failed: err(%s)", ec.message().data());
- }
+ CHECK(!ec, "boost::asio::io_service run failed: err({})", ec.message().data());
Review Comment:
```suggestion
CHECK(!ec, "boost::asio::io_service run failed: err({})", ec.message());
```
##########
src/replica/storage/simple_kv/test/simple_kv.server.impl.cpp:
##########
@@ -115,9 +115,9 @@ ::dsn::error_code simple_kv_service_impl::stop(bool clear_state)
dsn::zauto_lock l(_lock);
if (clear_state) {
- if (!dsn::utils::filesystem::remove_path(data_dir().c_str())) {
- dassert(false, "Fail to delete directory %s.", data_dir().c_str());
- }
+ CHECK(dsn::utils::filesystem::remove_path(data_dir().c_str()),
Review Comment:
```suggestion
CHECK(dsn::utils::filesystem::remove_path(data_dir()),
```
##########
src/replica/replica_2pc.cpp:
##########
@@ -482,13 +482,8 @@ void replica::on_prepare(dsn::message_ex *request)
if (partition_status::PS_POTENTIAL_SECONDARY == status() ||
partition_status::PS_SECONDARY == status()) {
- dassert(mu->data.header.decree <=
- last_committed_decree() + _options->max_mutation_count_in_prepare_list,
- "%" PRId64 " VS %" PRId64 "(%" PRId64 " + %d)",
- mu->data.header.decree,
- last_committed_decree() + _options->max_mutation_count_in_prepare_list,
- last_committed_decree(),
- _options->max_mutation_count_in_prepare_list);
+ CHECK_LE(mu->data.header.decree,
+ last_committed_decree() + _options->max_mutation_count_in_prepare_list);
Review Comment:
Print value for each addend ?
##########
src/runtime/rpc/network.cpp:
##########
@@ -580,9 +580,8 @@ uint32_t network::get_local_ipv4()
if (0 == ip) {
char name[128];
- if (gethostname(name, sizeof(name)) != 0) {
- dassert(false, "gethostname failed, err = %s", strerror(errno));
- }
+ CHECK_EQ_MSG(
+ gethostname(name, sizeof(name)), 0, "gethostname failed, err = {}", strerror(errno));
Review Comment:
```suggestion
gethostname(name, sizeof(name)), 0, "gethostname failed, err = {}", utils::safe_strerror(errno));
```
##########
src/runtime/rpc/rpc_engine.h:
##########
@@ -206,7 +206,7 @@ rpc_engine::call_address(rpc_address addr, message_ex *request, const rpc_respon
call_group(addr, request, call);
break;
default:
- dassert(false, "invalid target address type %d", (int)request->server_address.type());
+ CHECK(false, "invalid target address type {}", (int)request->server_address.type());
Review Comment:
```suggestion
CHECK(false, "invalid target address type {}", request->server_address.type());
```
##########
src/runtime/service_api_c.cpp:
##########
@@ -395,18 +395,14 @@ bool run(const char *config_file,
// setup data dir
auto &data_dir = spec.data_dir;
- dassert(!dsn::utils::filesystem::file_exists(data_dir),
- "%s should not be a file.",
- data_dir.c_str());
+ CHECK(!dsn::utils::filesystem::file_exists(data_dir), "{} should not be a file.", data_dir);
if (!dsn::utils::filesystem::directory_exists(data_dir.c_str())) {
Review Comment:
```suggestion
if (!dsn::utils::filesystem::directory_exists(data_dir)) {
```
##########
src/runtime/rpc/rpc_holder.h:
##########
@@ -257,13 +257,13 @@ class rpc_holder
static mail_box_t &mail_box()
{
- dassert(_mail_box != nullptr, "call this function only when you are in mock mode");
+ CHECK_NOTNULL(_mail_box, "call this function only when you are in mock mode");
Review Comment:
```suggestion
CHECK(_mail_box, "call this function only when you are in mock mode");
```
##########
src/runtime/rpc/rpc_holder.h:
##########
@@ -257,13 +257,13 @@ class rpc_holder
static mail_box_t &mail_box()
{
- dassert(_mail_box != nullptr, "call this function only when you are in mock mode");
+ CHECK_NOTNULL(_mail_box, "call this function only when you are in mock mode");
return *_mail_box.get();
}
static mail_box_t &forward_mail_box()
{
- dassert(_forward_mail_box != nullptr, "call this function only when you are in mock mode");
+ CHECK_NOTNULL(_forward_mail_box, "call this function only when you are in mock mode");
Review Comment:
```suggestion
CHECK(_forward_mail_box, "call this function only when you are in mock mode");
```
##########
src/runtime/scheduler.cpp:
##########
@@ -285,7 +285,7 @@ void scheduler::schedule()
t->release_ref(); // added by previous t->enqueue from app
} else {
- dassert(e.system_task != nullptr, "app and system tasks cannot be both empty");
+ CHECK_NOTNULL(e.system_task, "app and system tasks cannot be both empty");
Review Comment:
```suggestion
CHECK(e.system_task, "app and system tasks cannot be both empty");
```
##########
src/runtime/task/simple_task_queue.cpp:
##########
@@ -52,10 +52,7 @@ void simple_timer_service::start()
boost::asio::io_service::work work(_ios);
boost::system::error_code ec;
_ios.run(ec);
- if (ec) {
- dassert(
- false, "io_service in simple_timer_service run failed: %s", ec.message().data());
- }
+ CHECK(!ec, "io_service in simple_timer_service run failed: {}", ec.message().data());
Review Comment:
```suggestion
CHECK(!ec, "io_service in simple_timer_service run failed: {}", ec.message());
```
##########
src/runtime/rpc/asio_net_provider.cpp:
##########
@@ -88,9 +88,7 @@ error_code asio_network_provider::start(rpc_channel channel, int port, bool clie
boost::asio::io_service::work work(*_io_services[i]);
boost::system::error_code ec;
_io_services[i]->run(ec);
- if (ec) {
- dassert(false, "boost::asio::io_service run failed: err(%s)", ec.message().data());
- }
+ CHECK(!ec, "boost::asio::io_service run failed: err({})", ec.message().data());
Review Comment:
```suggestion
CHECK(!ec, "boost::asio::io_service run failed: err({})", ec.message());
```
##########
src/runtime/service_api_c.cpp:
##########
@@ -451,7 +447,7 @@ bool run(const char *config_file,
for (auto it = spec.toollets.begin(); it != spec.toollets.end(); ++it) {
auto tlet =
dsn::tools::internal_use_only::get_toollet(it->c_str(), ::dsn::PROVIDER_TYPE_MAIN);
- dassert(tlet, "toolet not found");
+ CHECK(tlet, "toolet not found");
Review Comment:
```suggestion
CHECK_NOTNULL(tlet, "toolet not found");
```
##########
src/utils/errors.h:
##########
@@ -194,13 +195,13 @@ class error_with
const T &get_value() const
{
- dassert(_err.is_ok(), "%s", get_error().description().data());
+ CHECK(_err.is_ok(), get_error().description().data());
Review Comment:
```suggestion
CHECK(_err.is_ok(), get_error().description());
```
##########
src/utils/errors.h:
##########
@@ -194,13 +195,13 @@ class error_with
const T &get_value() const
{
- dassert(_err.is_ok(), "%s", get_error().description().data());
+ CHECK(_err.is_ok(), get_error().description().data());
return *_value;
}
T &get_value()
{
- dassert(_err.is_ok(), "%s", get_error().description().data());
+ CHECK(_err.is_ok(), get_error().description().data());
Review Comment:
```suggestion
CHECK(_err.is_ok(), get_error().description());
```
##########
src/runtime/rpc/rpc_engine.cpp:
##########
@@ -635,10 +631,10 @@ void rpc_engine::call_group(rpc_address addr,
call_ip(request->server_address.group_address()->random_member(), request, call);
break;
case GRPC_TO_ALL:
- dassert(false, "to be implemented");
+ CHECK(false, "to be implemented");
break;
default:
- dassert(false, "invalid group rpc mode %d", (int)(sp->grpc_mode));
+ CHECK(false, "invalid group rpc mode {}", (int)(sp->grpc_mode));
Review Comment:
```suggestion
CHECK(false, "invalid group rpc mode {}", sp->grpc_mode);
```
##########
src/runtime/rpc/network.cpp:
##########
@@ -261,7 +261,7 @@ void rpc_session::send_message(message_ex *msg)
return;
}
- dassert(_parser, "parser should not be null when send");
+ CHECK(_parser, "parser should not be null when send");
Review Comment:
```suggestion
CHECK_NOTNULL(_parser, "parser should not be null when send");
```
##########
src/runtime/rpc/rpc_holder.h:
##########
@@ -290,7 +290,7 @@ class rpc_holder
int thread_hash)
: thrift_request(std::move(req)), auto_reply(false)
{
- dassert(thrift_request != nullptr, "req should not be null");
+ CHECK_NOTNULL(thrift_request, "req should not be null");
Review Comment:
```suggestion
CHECK(thrift_request, "req should not be null");
```
##########
src/runtime/service_api_c.cpp:
##########
@@ -395,18 +395,14 @@ bool run(const char *config_file,
// setup data dir
auto &data_dir = spec.data_dir;
- dassert(!dsn::utils::filesystem::file_exists(data_dir),
- "%s should not be a file.",
- data_dir.c_str());
+ CHECK(!dsn::utils::filesystem::file_exists(data_dir), "{} should not be a file.", data_dir);
if (!dsn::utils::filesystem::directory_exists(data_dir.c_str())) {
- if (!dsn::utils::filesystem::create_directory(data_dir)) {
- dassert(false, "Fail to create %s.", data_dir.c_str());
- }
+ CHECK(dsn::utils::filesystem::create_directory(data_dir), "Fail to create {}", data_dir);
}
std::string cdir;
- if (!dsn::utils::filesystem::get_absolute_path(data_dir.c_str(), cdir)) {
- dassert(false, "Fail to get absolute path from %s.", data_dir.c_str());
- }
+ CHECK(dsn::utils::filesystem::get_absolute_path(data_dir.c_str(), cdir),
Review Comment:
```suggestion
CHECK(dsn::utils::filesystem::get_absolute_path(data_dir, cdir),
```
##########
src/runtime/task/task.cpp:
##########
@@ -90,17 +90,17 @@ __thread uint16_t tls_dsn_lower32_task_id_mask = 0;
if (service_engine::instance().spec().enable_default_app_mimic) {
Review Comment:
Just assert the condition by `CHECK(service_engine::instance().spec().enable_default_app_mimic, ...)` ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] acelyc111 merged pull request #1226: refactor(macro): use CHECK_* to replace dassert_* (part 3)
Posted by GitBox <gi...@apache.org>.
acelyc111 merged PR #1226:
URL: https://github.com/apache/incubator-pegasus/pull/1226
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org