From 3d67d38c5d53db9323f33313ed9484d2ee598fb9 Mon Sep 17 00:00:00 2001 From: Frederic Suter Date: Mon, 1 Aug 2016 10:20:40 +0200 Subject: [PATCH] catch some bugs and smells --- examples/simdag/test/sd_test.cpp | 34 ++++++++++++++------------------ src/msg/msg_environment.cpp | 5 +++-- src/msg/msg_global.cpp | 11 ++++++----- src/msg/msg_gos.cpp | 25 +++++++++++------------ src/msg/msg_host.cpp | 10 +++++----- src/simdag/sd_global.cpp | 9 ++++++--- 6 files changed, 46 insertions(+), 48 deletions(-) diff --git a/examples/simdag/test/sd_test.cpp b/examples/simdag/test/sd_test.cpp index a73be5f92e..73a4e74366 100644 --- a/examples/simdag/test/sd_test.cpp +++ b/examples/simdag/test/sd_test.cpp @@ -8,19 +8,19 @@ #include "xbt/ex.h" #include #include "xbt/log.h" +#include XBT_LOG_NEW_DEFAULT_CATEGORY(sd_test, "Logging specific to this SimDag example"); int main(int argc, char **argv) { unsigned int ctr; - SD_task_t checkB, checkD; + SD_task_t checkB; + SD_task_t checkD; xbt_dynar_t changed_tasks; - const int host_count = 2; sg_host_t host_list[2]; double computation_amount[2]; double communication_amount[4] = { 0 }; - double rate = -1.0; /* initialization of SD */ SD_init(&argc, argv); @@ -61,8 +61,8 @@ int main(int argc, char **argv) SD_task_t taskD = SD_task_create("Task D", NULL, 60.0); /* try to attach and retrieve user data to a task */ - SD_task_set_data(taskA, (void*) &comp_amount1); - if (comp_amount1 != (*((double*) SD_task_get_data(taskA)))) + SD_task_set_data(taskA, static_cast(&comp_amount1)); + if (fabs(comp_amount1 - (*(static_cast(SD_task_get_data(taskA))))) > 1e-12) XBT_ERROR("User data was corrupted by a simple set/get"); SD_task_dependency_add(NULL, NULL, taskB, taskA); @@ -74,8 +74,7 @@ int main(int argc, char **argv) try { SD_task_dependency_add(NULL, NULL, taskA, taskA); /* shouldn't work and must raise an exception */ xbt_die("Hey, I can add a dependency between Task A and Task A!"); - } - catch (xbt_ex& ex) { + } catch (xbt_ex& ex) { if (ex.category != arg_error) throw; /* this is a serious error */ } @@ -83,8 +82,7 @@ int main(int argc, char **argv) try { SD_task_dependency_add(NULL, NULL, taskB, taskA); /* shouldn't work and must raise an exception */ xbt_die("Oh oh, I can add an already existing dependency!"); - } - catch (xbt_ex& ex) { + } catch (xbt_ex& ex) { if (ex.category != arg_error) throw; } @@ -92,8 +90,7 @@ int main(int argc, char **argv) try { SD_task_dependency_remove(taskA, taskC); /* shouldn't work and must raise an exception */ xbt_die("Dude, I can remove an unknown dependency!"); - } - catch (xbt_ex& ex) { + } catch (xbt_ex& ex) { if (ex.category != arg_error) throw; } @@ -101,8 +98,7 @@ int main(int argc, char **argv) try { SD_task_dependency_remove(taskC, taskC); /* shouldn't work and must raise an exception */ xbt_die("Wow, I can remove a dependency between Task C and itself!"); - } - catch (xbt_ex& ex) { + } catch (xbt_ex& ex) { if (ex.category != arg_error) throw; } @@ -125,13 +121,13 @@ int main(int argc, char **argv) /* estimated time */ SD_task_t task = taskD; - XBT_INFO("Estimated time for '%s': %f", SD_task_get_name(task), SD_task_get_execution_time(task, host_count, - host_list, computation_amount, communication_amount)); + XBT_INFO("Estimated time for '%s': %f", SD_task_get_name(task), SD_task_get_execution_time(task, 2, host_list, + computation_amount, communication_amount)); - SD_task_schedule(taskA, host_count, host_list, computation_amount, communication_amount, rate); - SD_task_schedule(taskB, host_count, host_list, computation_amount, communication_amount, rate); - SD_task_schedule(taskC, host_count, host_list, computation_amount, communication_amount, rate); - SD_task_schedule(taskD, host_count, host_list, computation_amount, communication_amount, rate); + SD_task_schedule(taskA, 2, host_list, computation_amount, communication_amount, -1); + SD_task_schedule(taskB, 2, host_list, computation_amount, communication_amount, -1); + SD_task_schedule(taskC, 2, host_list, computation_amount, communication_amount, -1); + SD_task_schedule(taskD, 2, host_list, computation_amount, communication_amount, -1); changed_tasks = SD_simulate(-1.0); xbt_dynar_foreach(changed_tasks, ctr, task) { diff --git a/src/msg/msg_environment.cpp b/src/msg/msg_environment.cpp index 27a367350f..feeb629ea5 100644 --- a/src/msg/msg_environment.cpp +++ b/src/msg/msg_environment.cpp @@ -68,10 +68,11 @@ xbt_dict_t MSG_environment_as_get_routing_sons(msg_as_t as) { const char *MSG_environment_as_get_property_value(msg_as_t as, const char *name) { - xbt_dict_t dict = (xbt_dict_t) xbt_lib_get_or_null(as_router_lib, MSG_environment_as_get_name(as), ROUTING_PROP_ASR_LEVEL); + xbt_dict_t dict = static_cast (xbt_lib_get_or_null(as_router_lib, MSG_environment_as_get_name(as), + ROUTING_PROP_ASR_LEVEL)); if (dict==nullptr) return nullptr; - return (char*) xbt_dict_get_or_null(dict, name); + return static_cast(xbt_dict_get_or_null(dict, name)); } xbt_dynar_t MSG_environment_as_get_hosts(msg_as_t as) { diff --git a/src/msg/msg_global.cpp b/src/msg/msg_global.cpp index f056efdf31..9a953007f0 100644 --- a/src/msg/msg_global.cpp +++ b/src/msg/msg_global.cpp @@ -19,7 +19,7 @@ XBT_LOG_NEW_CATEGORY(msg, "All MSG categories"); XBT_LOG_NEW_DEFAULT_SUBCATEGORY(msg_kernel, msg, "Logging specific to MSG (kernel)"); MSG_Global_t msg_global = nullptr; -static void MSG_exit(void); +static void MSG_exit(); /********************************* MSG **************************************/ @@ -77,13 +77,14 @@ void MSG_init_nocheck(int *argc, char **argv) { XBT_DEBUG("ADD MSG LEVELS"); MSG_STORAGE_LEVEL = xbt_lib_add_level(storage_lib, (void_f_pvoid_t) __MSG_storage_destroy); MSG_FILE_LEVEL = xbt_lib_add_level(file_lib, (void_f_pvoid_t) __MSG_file_destroy); - if(xbt_cfg_get_boolean("clean-atexit")) atexit(MSG_exit); + if(xbt_cfg_get_boolean("clean-atexit")) + atexit(MSG_exit); } /** \ingroup msg_simulation * \brief Launch the MSG simulation */ -msg_error_t MSG_main(void) +msg_error_t MSG_main() { /* Clean IO before the run */ fflush(stdout); @@ -128,7 +129,7 @@ int MSG_process_killall(int reset_PIDs) } -static void MSG_exit(void) { +static void MSG_exit() { if (msg_global==nullptr) return; @@ -141,7 +142,7 @@ static void MSG_exit(void) { /** \ingroup msg_simulation * \brief A clock (in second). */ -double MSG_get_clock(void) +double MSG_get_clock() { return SIMIX_get_clock(); } diff --git a/src/msg/msg_gos.cpp b/src/msg/msg_gos.cpp index eac963dfcc..b4cffe92d9 100644 --- a/src/msg/msg_gos.cpp +++ b/src/msg/msg_gos.cpp @@ -11,8 +11,7 @@ #include "xbt/log.h" #include "xbt/sysdep.h" -XBT_LOG_NEW_DEFAULT_SUBCATEGORY(msg_gos, msg, - "Logging specific to MSG (gos)"); +XBT_LOG_NEW_DEFAULT_SUBCATEGORY(msg_gos, msg, "Logging specific to MSG (gos)"); /** \ingroup msg_task_usage * \brief Executes a task and waits for its termination. @@ -45,18 +44,17 @@ msg_error_t MSG_task_execute(msg_task_t task) msg_error_t MSG_parallel_task_execute(msg_task_t task) { simdata_task_t simdata = task->simdata; - simdata_process_t p_simdata = (simdata_process_t) SIMIX_process_self_get_data(); + simdata_process_t p_simdata = static_cast(SIMIX_process_self_get_data()); e_smx_state_t comp_state; msg_error_t status = MSG_OK; TRACE_msg_task_execute_start(task); - xbt_assert((!simdata->compute) && !task->simdata->isused, - "This task is executed somewhere else. Go fix your code!"); + xbt_assert((!simdata->compute) && !task->simdata->isused, "This task is executed somewhere else. Go fix your code!"); XBT_DEBUG("Computing on %s", MSG_process_get_name(MSG_process_self())); - if (simdata->flops_amount == 0 && !simdata->host_nb) { + if (simdata->flops_amount <= 0.0 && !simdata->host_nb) { TRACE_msg_task_execute_end(task); return MSG_OK; } @@ -72,8 +70,8 @@ msg_error_t MSG_parallel_task_execute(msg_task_t task) XBT_DEBUG("Parallel execution action created: %p", simdata->compute); } else { unsigned long affinity_mask = - (unsigned long)(uintptr_t) xbt_dict_get_or_null_ext(simdata->affinity_mask_db, (char *) p_simdata->m_host, - sizeof(msg_host_t)); + static_cast((uintptr_t) xbt_dict_get_or_null_ext(simdata->affinity_mask_db, (char *) p_simdata->m_host, + sizeof(msg_host_t))); XBT_DEBUG("execute %s@%s with affinity(0x%04lx)", MSG_task_get_name(task), MSG_host_get_name(p_simdata->m_host), affinity_mask); @@ -122,7 +120,6 @@ msg_error_t MSG_parallel_task_execute(msg_task_t task) msg_error_t MSG_process_sleep(double nb_sec) { msg_error_t status = MSG_OK; - /*msg_process_t proc = MSG_process_self();*/ TRACE_msg_process_sleep_in(MSG_process_self()); @@ -317,7 +314,7 @@ static inline msg_comm_t MSG_task_isend_internal(msg_task_t task, const char *al /* Prepare the task to send */ t_simdata = task->simdata; t_simdata->sender = myself; - t_simdata->source = ((simdata_process_t) SIMIX_process_self_get_data())->m_host; + t_simdata->source = (static_cast(SIMIX_process_self_get_data()))->m_host; t_simdata->setUsed(); t_simdata->comm = nullptr; msg_global->sent_msg++; @@ -724,7 +721,7 @@ void MSG_comm_copy_data_from_SIMIX(smx_synchro_t synchro, void* buff, size_t buf // notify the user callback if any if (msg_global->task_copy_callback) { - msg_task_t task = (msg_task_t) buff; + msg_task_t task = static_cast(buff); msg_global->task_copy_callback(task, comm->src_proc, comm->dst_proc); } } @@ -784,7 +781,7 @@ msg_error_t MSG_task_send_with_timeout(msg_task_t task, const char *alias, doubl msg_error_t ret = MSG_OK; simdata_task_t t_simdata = nullptr; msg_process_t process = MSG_process_self(); - simdata_process_t p_simdata = (simdata_process_t) SIMIX_process_self_get_data(); + simdata_process_t p_simdata = static_cast(SIMIX_process_self_get_data()); simgrid::s4u::MailboxPtr mailbox = simgrid::s4u::Mailbox::byName(alias); int call_end = TRACE_msg_task_put_start(task); //must be after CHECK_HOST() @@ -792,7 +789,7 @@ msg_error_t MSG_task_send_with_timeout(msg_task_t task, const char *alias, doubl /* Prepare the task to send */ t_simdata = task->simdata; t_simdata->sender = process; - t_simdata->source = ((simdata_process_t) SIMIX_process_self_get_data())->m_host; + t_simdata->source = (static_cast(SIMIX_process_self_get_data())) ->m_host; t_simdata->setUsed(); @@ -932,7 +929,7 @@ const char *MSG_task_get_category (msg_task_t task) */ const char *MSG_as_router_get_property_value(const char* asr, const char *name) { - return (char*) xbt_dict_get_or_null(MSG_as_router_get_properties(asr), name); + return static_cast(xbt_dict_get_or_null(MSG_as_router_get_properties(asr), name)); } /** diff --git a/src/msg/msg_host.cpp b/src/msg/msg_host.cpp index e50f818115..a6aae4a402 100644 --- a/src/msg/msg_host.cpp +++ b/src/msg/msg_host.cpp @@ -79,7 +79,7 @@ void *MSG_host_get_data(msg_host_t host) { * * \brief Return the location on which the current process is executed. */ -msg_host_t MSG_host_self(void) +msg_host_t MSG_host_self() { return MSG_process_get_host(nullptr); } @@ -128,7 +128,7 @@ void __MSG_host_priv_free(msg_host_priv_t priv) /** \ingroup m_host_management * \brief Return the current number MSG hosts. */ -int MSG_get_host_number(void) +int MSG_get_host_number() { return xbt_dict_length(host_list); } @@ -138,7 +138,7 @@ int MSG_get_host_number(void) * \remark The host order in the returned array is generally different from the host creation/declaration order in the * XML platform (we use a hash table internally) */ -xbt_dynar_t MSG_hosts_as_dynar(void) { +xbt_dynar_t MSG_hosts_as_dynar() { return sg_hosts_as_dynar(); } @@ -190,7 +190,7 @@ xbt_swag_t MSG_host_get_process_list(msg_host_t host) */ const char *MSG_host_get_property_value(msg_host_t host, const char *name) { - return (const char*) xbt_dict_get_or_null(MSG_host_get_properties(host), name); + return static_cast(xbt_dict_get_or_null(MSG_host_get_properties(host), name)); } /** \ingroup m_host_management @@ -333,7 +333,7 @@ xbt_dict_t MSG_host_get_storage_content(msg_host_t host) xbt_dict_t storage_list = host->mountedStoragesAsDict(); xbt_dict_foreach(storage_list,cursor,mount_name,storage_name){ - storage = (msg_storage_t)xbt_lib_get_elm_or_null(storage_lib,storage_name); + storage = static_cast(xbt_lib_get_elm_or_null(storage_lib,storage_name)); xbt_dict_t content = simcall_storage_get_content(storage); xbt_dict_set(contents,mount_name, content,nullptr); } diff --git a/src/simdag/sd_global.cpp b/src/simdag/sd_global.cpp index 66f3c612d7..f6fb1dcd66 100644 --- a/src/simdag/sd_global.cpp +++ b/src/simdag/sd_global.cpp @@ -139,8 +139,8 @@ xbt_dynar_t SD_simulate(double how_long) { /* let's see which tasks are done */ unsigned int iter; xbt_dynar_foreach(all_existing_models, iter, model) { - surf_action_t action; - while ((action = surf_model_extract_done_action_set(model))) { + surf_action_t action = surf_model_extract_done_action_set(model); + while (action != nullptr) { SD_task_t task = static_cast(action->getData()); XBT_VERB("Task '%s' done", SD_task_get_name(task)); SD_task_set_state(task, SD_DONE); @@ -185,14 +185,17 @@ xbt_dynar_t SD_simulate(double how_long) { SD_task_run(output); } task->outputs->clear(); + action = surf_model_extract_done_action_set(model); } /* let's see which tasks have just failed */ - while ((action = surf_model_extract_failed_action_set(model))) { + action = surf_model_extract_failed_action_set(model); + while (action != nullptr) { SD_task_t task = static_cast(action->getData()); XBT_VERB("Task '%s' failed", SD_task_get_name(task)); SD_task_set_state(task, SD_FAILED); xbt_dynar_push(sd_global->return_set, &task); + action = surf_model_extract_failed_action_set(model); } } } -- 2.20.1