From 8141a3eb1445b66f6b00f87cf57c56dd94da79f7 Mon Sep 17 00:00:00 2001 From: Michael Engel Date: Fri, 9 Aug 2024 15:15:40 +0200 Subject: [PATCH 1/4] Refactored ListUnitsRequest and ListUnitFilesRequest Relates to: https://github.com/eclipse-bluechi/bluechi/issues/889 The structs and functions used for ListUnits and ListUnitFiles are identical for the most part. In order to reduce code duplication, the implementation for both have been combined into a single struct with unified functions. This should also allow for additional, similar features in the future. Signed-off-by: Michael Engel --- src/controller/controller.c | 319 +++++++++++++----------------------- src/controller/node.h | 2 - 2 files changed, 113 insertions(+), 208 deletions(-) diff --git a/src/controller/controller.c b/src/controller/controller.c index c4aee4a124..b80b6a734e 100644 --- a/src/controller/controller.c +++ b/src/controller/controller.c @@ -562,11 +562,18 @@ static int controller_setup_heartbeat_timer(Controller *controller) { } /************************************************************************ - ************** org.eclipse.bluechi.Controller.ListUnits ***** + ***************** AgentFleetRequest ************************************ ************************************************************************/ -typedef struct ListUnitsRequest { +typedef struct AgentFleetRequest AgentFleetRequest; + +typedef int (*agent_fleet_request_encode_reply_t)(AgentFleetRequest *req, sd_bus_message *reply); +typedef AgentRequest *(*agent_fleet_request_create_t)( + Node *node, agent_request_response_t cb, void *userdata, free_func_t free_userdata); + +typedef struct AgentFleetRequest { sd_bus_message *request_message; + agent_fleet_request_encode_reply_t encode; int n_done; int n_sub_req; @@ -575,9 +582,9 @@ typedef struct ListUnitsRequest { sd_bus_message *m; AgentRequest *agent_req; } sub_req[0]; -} ListUnitsRequest; +} AgentFleetRequest; -static void list_unit_request_free(ListUnitsRequest *req) { +static void agent_fleet_request_free(AgentFleetRequest *req) { sd_bus_message_unref(req->request_message); for (int i = 0; i < req->n_sub_req; i++) { @@ -589,90 +596,18 @@ static void list_unit_request_free(ListUnitsRequest *req) { free(req); } -static void list_unit_request_freep(ListUnitsRequest **reqp) { +static void agent_fleet_request_freep(AgentFleetRequest **reqp) { if (reqp && *reqp) { - list_unit_request_free(*reqp); + agent_fleet_request_free(*reqp); *reqp = NULL; } } -#define _cleanup_list_unit_request_ _cleanup_(list_unit_request_freep) - - -static int controller_method_list_units_encode_reply(ListUnitsRequest *req, sd_bus_message *reply) { - int r = sd_bus_message_open_container(reply, SD_BUS_TYPE_ARRAY, NODE_AND_UNIT_INFO_STRUCT_TYPESTRING); - if (r < 0) { - return r; - } - - for (int i = 0; i < req->n_sub_req; i++) { - const char *node_name = req->sub_req[i].node->name; - sd_bus_message *m = req->sub_req[i].m; - if (m == NULL) { - continue; - } - - const sd_bus_error *err = sd_bus_message_get_error(m); - if (err != NULL) { - bc_log_errorf("Failed to list units for node '%s': %s", node_name, err->message); - return -sd_bus_message_get_errno(m); - } - - r = sd_bus_message_enter_container(m, SD_BUS_TYPE_ARRAY, UNIT_INFO_STRUCT_TYPESTRING); - if (r < 0) { - return r; - } - - while (sd_bus_message_at_end(m, false) == 0) { - r = sd_bus_message_open_container( - reply, SD_BUS_TYPE_STRUCT, NODE_AND_UNIT_INFO_TYPESTRING); - if (r < 0) { - return r; - } - - r = sd_bus_message_append(reply, "s", node_name); - if (r < 0) { - return r; - } - - r = sd_bus_message_enter_container(m, SD_BUS_TYPE_STRUCT, UNIT_INFO_TYPESTRING); - if (r < 0) { - return r; - } - - r = sd_bus_message_copy(reply, m, true); - if (r < 0) { - return r; - } - - r = sd_bus_message_close_container(reply); - if (r < 0) { - return r; - } - r = sd_bus_message_exit_container(m); - if (r < 0) { - return r; - } - } +#define _cleanup_agent_fleet_request_ _cleanup_(agent_fleet_request_freep) - r = sd_bus_message_exit_container(m); - if (r < 0) { - return r; - } - } - - r = sd_bus_message_close_container(reply); - if (r < 0) { - return r; - } - - return 0; -} - - -static void controller_method_list_units_done(ListUnitsRequest *req) { +static void agent_fleet_request_done(AgentFleetRequest *req) { /* All sub_req-requests are done, collect results and free when done */ - UNUSED _cleanup_list_unit_request_ ListUnitsRequest *free_me = req; + UNUSED _cleanup_agent_fleet_request_ AgentFleetRequest *free_me = req; _cleanup_sd_bus_message_ sd_bus_message *reply = NULL; int r = sd_bus_message_new_method_return(req->request_message, &reply); @@ -685,12 +620,12 @@ static void controller_method_list_units_done(ListUnitsRequest *req) { return; } - r = controller_method_list_units_encode_reply(req, reply); + r = req->encode(req, reply); if (r < 0) { sd_bus_reply_method_errorf( req->request_message, SD_BUS_ERROR_FAILED, - "List units request to at least one node failed: %s", + "Request to at least one node failed: %s", strerror(-r)); return; } @@ -702,15 +637,15 @@ static void controller_method_list_units_done(ListUnitsRequest *req) { } } -static void controller_method_list_units_maybe_done(ListUnitsRequest *req) { +static void agent_fleet_request_maybe_done(AgentFleetRequest *req) { if (req->n_done == req->n_sub_req) { - controller_method_list_units_done(req); + agent_fleet_request_done(req); } } -static int controller_list_units_callback( +static int agent_fleet_request_callback( AgentRequest *agent_req, sd_bus_message *m, UNUSED sd_bus_error *ret_error) { - ListUnitsRequest *req = agent_req->userdata; + AgentFleetRequest *req = agent_req->userdata; int i = 0; for (i = 0; i < req->n_sub_req; i++) { @@ -724,27 +659,30 @@ static int controller_list_units_callback( req->sub_req[i].m = sd_bus_message_ref(m); req->n_done++; - controller_method_list_units_maybe_done(req); + agent_fleet_request_maybe_done(req); return 0; } - -static int controller_method_list_units(sd_bus_message *m, void *userdata, UNUSED sd_bus_error *ret_error) { - Controller *controller = userdata; - ListUnitsRequest *req = NULL; - Node *node = NULL; +static int agent_fleet_request_start( + sd_bus_message *request_message, + Controller *controller, + agent_fleet_request_create_t create_request, + agent_fleet_request_encode_reply_t encode) { + AgentFleetRequest *req = NULL; req = malloc0_array(sizeof(*req), sizeof(req->sub_req[0]), controller->number_of_nodes); if (req == NULL) { - return sd_bus_reply_method_errorf(m, SD_BUS_ERROR_NO_MEMORY, "Out of memory"); + return sd_bus_reply_method_errorf(request_message, SD_BUS_ERROR_NO_MEMORY, "Out of memory"); } - req->request_message = sd_bus_message_ref(m); + req->request_message = sd_bus_message_ref(request_message); + req->encode = encode; + Node *node = NULL; int i = 0; LIST_FOREACH(nodes, node, controller->nodes) { - _cleanup_agent_request_ AgentRequest *agent_req = node_request_list_units( - node, controller_list_units_callback, req, NULL); + _cleanup_agent_request_ AgentRequest *agent_req = create_request( + node, agent_fleet_request_callback, req, NULL); if (agent_req) { req->sub_req[i].agent_req = steal_pointer(&agent_req); req->sub_req[i].node = node_ref(node); @@ -760,50 +698,97 @@ static int controller_method_list_units(sd_bus_message *m, void *userdata, UNUSE #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wanalyzer-malloc-leak" - controller_method_list_units_maybe_done(req); + agent_fleet_request_maybe_done(req); return 1; } #pragma GCC diagnostic pop /************************************************************************ - ***** org.eclipse.bluechi.Controller.ListUnitFiles ************** + ************** org.eclipse.bluechi.Controller.ListUnits ***** ************************************************************************/ -typedef struct ListUnitFilesRequest { - sd_bus_message *request_message; +static int controller_method_list_units_encode_reply(AgentFleetRequest *req, sd_bus_message *reply) { + int r = sd_bus_message_open_container(reply, SD_BUS_TYPE_ARRAY, NODE_AND_UNIT_INFO_STRUCT_TYPESTRING); + if (r < 0) { + return r; + } - int n_done; - int n_sub_req; - struct { - Node *node; - sd_bus_message *m; - AgentRequest *agent_req; - } sub_req[0]; -} ListUnitFilesRequest; + for (int i = 0; i < req->n_sub_req; i++) { + const char *node_name = req->sub_req[i].node->name; + sd_bus_message *m = req->sub_req[i].m; + if (m == NULL) { + continue; + } -static void list_unit_files_request_free(ListUnitFilesRequest *req) { - sd_bus_message_unref(req->request_message); + const sd_bus_error *err = sd_bus_message_get_error(m); + if (err != NULL) { + bc_log_errorf("Failed to list units for node '%s': %s", node_name, err->message); + return -sd_bus_message_get_errno(m); + } - for (int i = 0; i < req->n_sub_req; i++) { - node_unrefp(&req->sub_req[i].node); - sd_bus_message_unrefp(&req->sub_req[i].m); - agent_request_unrefp(&req->sub_req[i].agent_req); + r = sd_bus_message_enter_container(m, SD_BUS_TYPE_ARRAY, UNIT_INFO_STRUCT_TYPESTRING); + if (r < 0) { + return r; + } + + while (sd_bus_message_at_end(m, false) == 0) { + r = sd_bus_message_open_container( + reply, SD_BUS_TYPE_STRUCT, NODE_AND_UNIT_INFO_TYPESTRING); + if (r < 0) { + return r; + } + + r = sd_bus_message_append(reply, "s", node_name); + if (r < 0) { + return r; + } + + r = sd_bus_message_enter_container(m, SD_BUS_TYPE_STRUCT, UNIT_INFO_TYPESTRING); + if (r < 0) { + return r; + } + + r = sd_bus_message_copy(reply, m, true); + if (r < 0) { + return r; + } + + r = sd_bus_message_close_container(reply); + if (r < 0) { + return r; + } + r = sd_bus_message_exit_container(m); + if (r < 0) { + return r; + } + } + + r = sd_bus_message_exit_container(m); + if (r < 0) { + return r; + } } - free(req); + r = sd_bus_message_close_container(reply); + if (r < 0) { + return r; + } + + return 0; } -static void list_unit_files_request_freep(ListUnitFilesRequest **reqp) { - if (reqp && *reqp) { - list_unit_files_request_free(*reqp); - *reqp = NULL; - } +static int controller_method_list_units(sd_bus_message *m, void *userdata, UNUSED sd_bus_error *ret_error) { + Controller *controller = userdata; + return agent_fleet_request_start( + m, controller, node_request_list_units, controller_method_list_units_encode_reply); } -#define _cleanup_list_unit_files_request_ _cleanup_(list_unit_files_request_freep) +/************************************************************************ + ***** org.eclipse.bluechi.Controller.ListUnitFiles ************** + ************************************************************************/ -static int controller_method_list_unit_files_encode_reply(ListUnitFilesRequest *req, sd_bus_message *reply) { +static int controller_method_list_unit_files_encode_reply(AgentFleetRequest *req, sd_bus_message *reply) { int r = sd_bus_message_open_container( reply, SD_BUS_TYPE_ARRAY, NODE_AND_UNIT_FILE_INFO_STRUCT_TYPESTRING); if (r < 0) { @@ -875,92 +860,14 @@ static int controller_method_list_unit_files_encode_reply(ListUnitFilesRequest * return 0; } -static void controller_method_list_unit_files_done(ListUnitFilesRequest *req) { - UNUSED _cleanup_list_unit_files_request_ ListUnitFilesRequest *free_me = req; - - _cleanup_sd_bus_message_ sd_bus_message *reply = NULL; - int r = sd_bus_message_new_method_return(req->request_message, &reply); - if (r < 0) { - sd_bus_reply_method_errorf( - req->request_message, - SD_BUS_ERROR_FAILED, - "Failed to create a reply message: %s", - strerror(-r)); - return; - } - r = controller_method_list_unit_files_encode_reply(req, reply); - if (r < 0) { - sd_bus_reply_method_errorf( - req->request_message, - SD_BUS_ERROR_FAILED, - "List unit files request to at least one node failed: %s", - strerror(-r)); - return; - } - r = sd_bus_message_send(reply); - if (r < 0) { - bc_log_errorf("Failed to send reply message: %s", strerror(-r)); - return; - } -} - -static void controller_method_list_unit_files_maybe_done(ListUnitFilesRequest *req) { - if (req->n_done == req->n_sub_req) { - controller_method_list_unit_files_done(req); - } -} - -static int controller_list_unit_files_callback( - AgentRequest *agent_req, sd_bus_message *m, UNUSED sd_bus_error *ret_error) { - ListUnitFilesRequest *req = agent_req->userdata; - int i = 0; - - for (i = 0; i < req->n_sub_req; i++) { - if (req->sub_req[i].agent_req == agent_req) { - break; - } - } - - assert(i != req->n_sub_req); - - req->sub_req[i].m = sd_bus_message_ref(m); - req->n_done++; - - controller_method_list_unit_files_maybe_done(req); - - return 0; -} - static int controller_method_list_unit_files(sd_bus_message *m, void *userdata, UNUSED sd_bus_error *ret_error) { Controller *controller = userdata; - ListUnitFilesRequest *req = NULL; - Node *node = NULL; - - req = malloc0_array(sizeof(*req), sizeof(req->sub_req[0]), controller->number_of_nodes); - if (req == NULL) { - return sd_bus_reply_method_errorf(m, SD_BUS_ERROR_NO_MEMORY, "Out of memory"); - } - req->request_message = sd_bus_message_ref(m); - - int i = 0; - LIST_FOREACH(nodes, node, controller->nodes) { - _cleanup_agent_request_ AgentRequest *agent_req = node_request_list_unit_files( - node, controller_list_unit_files_callback, req, NULL); - if (agent_req) { - req->sub_req[i].agent_req = steal_pointer(&agent_req); - req->sub_req[i].node = node_ref(node); - req->n_sub_req++; - i++; - } - } -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wanalyzer-malloc-leak" - - controller_method_list_unit_files_maybe_done(req); - - return 1; + return agent_fleet_request_start( + m, + controller, + node_request_list_unit_files, + controller_method_list_unit_files_encode_reply); } -#pragma GCC diagnostic pop /************************************************************************ ***** org.eclipse.bluechi.Controller.ListNodes ************** diff --git a/src/controller/node.h b/src/controller/node.h index b73320489b..64e4a6ef26 100644 --- a/src/controller/node.h +++ b/src/controller/node.h @@ -32,9 +32,7 @@ struct AgentRequest { AgentRequest *agent_request_ref(AgentRequest *req); void agent_request_unref(AgentRequest *req); - int agent_request_start(AgentRequest *req); - int agent_request_cancel(AgentRequest *r); struct Node { From 853905cd9fa29a1b49a9bfaa6936d641b88c67ed Mon Sep 17 00:00:00 2001 From: Michael Engel Date: Fri, 9 Aug 2024 16:01:09 +0200 Subject: [PATCH 2/4] Updated python bindings version Signed-off-by: Michael Engel --- src/bindings/python/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bindings/python/setup.py b/src/bindings/python/setup.py index e65878a937..f228aabea2 100644 --- a/src/bindings/python/setup.py +++ b/src/bindings/python/setup.py @@ -12,7 +12,7 @@ def readme(): setup( name="bluechi", - version="0.8.0", + version="0.9.0", description="Python bindings for BlueChi's D-Bus API", long_description=readme(), long_description_content_type="text/markdown", From a0afdc7a1ee3e9989873aab6074f992446982563 Mon Sep 17 00:00:00 2001 From: Michael Engel Date: Fri, 9 Aug 2024 16:30:17 +0200 Subject: [PATCH 3/4] Applied code formatting due to updated build-base image Signed-off-by: Michael Engel --- src/agent/main.c | 20 +++--- src/client/main.c | 62 ++++++++--------- src/controller/main.c | 10 +-- .../cli/command/command_add_option_test.c | 6 +- .../cli/command/command_flag_exists_test.c | 6 +- .../cli/command/command_get_option_test.c | 6 +- .../test/cli/command/get_option_type_test.c | 6 +- .../cli/command/methods_get_method_test.c | 6 +- .../cfg_load_complete_configuration_test.c | 36 +++++----- .../test/common/network/get_address_test.c | 14 ++-- src/libbluechi/test/log/bc_log_init_test.c | 66 +++++++++---------- src/proxy/main.c | 8 +-- 12 files changed, 123 insertions(+), 123 deletions(-) diff --git a/src/agent/main.c b/src/agent/main.c index 1e2bb50008..5485eba421 100644 --- a/src/agent/main.c +++ b/src/agent/main.c @@ -15,16 +15,16 @@ #include "agent.h" const struct option options[] = { - {ARG_HOST, required_argument, 0, ARG_HOST_SHORT }, - { ARG_PORT, required_argument, 0, ARG_PORT_SHORT }, - { ARG_ADDRESS, required_argument, 0, ARG_ADDRESS_SHORT }, - { ARG_NAME, required_argument, 0, ARG_NAME_SHORT }, - { ARG_HEARTBEAT_INTERVAL, required_argument, 0, ARG_HEARTBEAT_INTERVAL_SHORT}, - { ARG_CONFIG, required_argument, 0, ARG_CONFIG_SHORT }, - { ARG_USER, no_argument, 0, ARG_USER_SHORT }, - { ARG_HELP, no_argument, 0, ARG_HELP_SHORT }, - { ARG_VERSION, no_argument, 0, ARG_VERSION_SHORT }, - { NULL, 0, 0, '\0' } + { ARG_HOST, required_argument, 0, ARG_HOST_SHORT }, + { ARG_PORT, required_argument, 0, ARG_PORT_SHORT }, + { ARG_ADDRESS, required_argument, 0, ARG_ADDRESS_SHORT }, + { ARG_NAME, required_argument, 0, ARG_NAME_SHORT }, + { ARG_HEARTBEAT_INTERVAL, required_argument, 0, ARG_HEARTBEAT_INTERVAL_SHORT }, + { ARG_CONFIG, required_argument, 0, ARG_CONFIG_SHORT }, + { ARG_USER, no_argument, 0, ARG_USER_SHORT }, + { ARG_HELP, no_argument, 0, ARG_HELP_SHORT }, + { ARG_VERSION, no_argument, 0, ARG_VERSION_SHORT }, + { NULL, 0, 0, '\0' } }; #define GETOPT_OPTSTRING \ diff --git a/src/client/main.c b/src/client/main.c index 5a86edcfc7..07afa7fa8a 100644 --- a/src/client/main.c +++ b/src/client/main.c @@ -34,44 +34,44 @@ int method_version(UNUSED Command *command, UNUSED void *userdata) { } const Method methods[] = { - {"help", 0, 0, OPT_NONE, method_help, usage_bluechi}, - { "list-unit-files", 0, 1, OPT_FILTER, method_list_unit_files, usage_bluechi}, - { "list-units", 0, 1, OPT_FILTER, method_list_units, usage_bluechi}, - { "start", 2, 2, OPT_NONE, method_start, usage_bluechi}, - { "stop", 2, 2, OPT_NONE, method_stop, usage_bluechi}, - { "freeze", 2, 2, OPT_NONE, method_freeze, usage_bluechi}, - { "thaw", 2, 2, OPT_NONE, method_thaw, usage_bluechi}, - { "restart", 2, 2, OPT_NONE, method_restart, usage_bluechi}, - { "reload", 2, 2, OPT_NONE, method_reload, usage_bluechi}, - { "monitor", 0, 2, OPT_NONE, method_monitor, usage_bluechi}, - { "metrics", 1, 1, OPT_NONE, method_metrics, usage_bluechi}, - { "enable", 2, ARG_ANY, OPT_FORCE | OPT_RUNTIME | OPT_NO_RELOAD, method_enable, usage_bluechi}, - { "disable", 2, ARG_ANY, OPT_NO_RELOAD, method_disable, usage_bluechi}, - { "daemon-reload", 1, 1, OPT_NONE, method_daemon_reload, usage_bluechi}, - { "status", 0, ARG_ANY, OPT_WATCH, method_status, usage_bluechi}, - { "set-loglevel", 1, 2, OPT_NONE, method_set_loglevel, usage_bluechi}, - { "version", 0, 0, OPT_NONE, method_version, usage_bluechi}, - { NULL, 0, 0, 0, NULL, NULL } + { "help", 0, 0, OPT_NONE, method_help, usage_bluechi }, + { "list-unit-files", 0, 1, OPT_FILTER, method_list_unit_files, usage_bluechi }, + { "list-units", 0, 1, OPT_FILTER, method_list_units, usage_bluechi }, + { "start", 2, 2, OPT_NONE, method_start, usage_bluechi }, + { "stop", 2, 2, OPT_NONE, method_stop, usage_bluechi }, + { "freeze", 2, 2, OPT_NONE, method_freeze, usage_bluechi }, + { "thaw", 2, 2, OPT_NONE, method_thaw, usage_bluechi }, + { "restart", 2, 2, OPT_NONE, method_restart, usage_bluechi }, + { "reload", 2, 2, OPT_NONE, method_reload, usage_bluechi }, + { "monitor", 0, 2, OPT_NONE, method_monitor, usage_bluechi }, + { "metrics", 1, 1, OPT_NONE, method_metrics, usage_bluechi }, + { "enable", 2, ARG_ANY, OPT_FORCE | OPT_RUNTIME | OPT_NO_RELOAD, method_enable, usage_bluechi }, + { "disable", 2, ARG_ANY, OPT_NO_RELOAD, method_disable, usage_bluechi }, + { "daemon-reload", 1, 1, OPT_NONE, method_daemon_reload, usage_bluechi }, + { "status", 0, ARG_ANY, OPT_WATCH, method_status, usage_bluechi }, + { "set-loglevel", 1, 2, OPT_NONE, method_set_loglevel, usage_bluechi }, + { "version", 0, 0, OPT_NONE, method_version, usage_bluechi }, + { NULL, 0, 0, 0, NULL, NULL } }; const OptionType option_types[] = { - {ARG_FILTER_SHORT, ARG_FILTER, OPT_FILTER }, - { ARG_FORCE_SHORT, ARG_FORCE, OPT_FORCE }, - { ARG_RUNTIME_SHORT, ARG_RUNTIME, OPT_RUNTIME }, - { ARG_NO_RELOAD_SHORT, ARG_NO_RELOAD, OPT_NO_RELOAD}, - { ARG_WATCH_SHORT, ARG_WATCH, OPT_WATCH }, - { 0, NULL, 0 } + { ARG_FILTER_SHORT, ARG_FILTER, OPT_FILTER }, + { ARG_FORCE_SHORT, ARG_FORCE, OPT_FORCE }, + { ARG_RUNTIME_SHORT, ARG_RUNTIME, OPT_RUNTIME }, + { ARG_NO_RELOAD_SHORT, ARG_NO_RELOAD, OPT_NO_RELOAD }, + { ARG_WATCH_SHORT, ARG_WATCH, OPT_WATCH }, + { 0, NULL, 0 } }; #define GETOPT_OPTSTRING ARG_HELP_SHORT_S ARG_FORCE_SHORT_S ARG_WATCH_SHORT_S const struct option getopt_options[] = { - {ARG_HELP, no_argument, 0, ARG_HELP_SHORT }, - { ARG_FILTER, required_argument, 0, ARG_FILTER_SHORT }, - { ARG_FORCE, no_argument, 0, ARG_FORCE_SHORT }, - { ARG_RUNTIME, no_argument, 0, ARG_RUNTIME_SHORT }, - { ARG_NO_RELOAD, no_argument, 0, ARG_NO_RELOAD_SHORT}, - { ARG_WATCH, no_argument, 0, ARG_WATCH_SHORT }, - { NULL, 0, 0, '\0' } + { ARG_HELP, no_argument, 0, ARG_HELP_SHORT }, + { ARG_FILTER, required_argument, 0, ARG_FILTER_SHORT }, + { ARG_FORCE, no_argument, 0, ARG_FORCE_SHORT }, + { ARG_RUNTIME, no_argument, 0, ARG_RUNTIME_SHORT }, + { ARG_NO_RELOAD, no_argument, 0, ARG_NO_RELOAD_SHORT }, + { ARG_WATCH, no_argument, 0, ARG_WATCH_SHORT }, + { NULL, 0, 0, '\0' } }; static void usage() { diff --git a/src/controller/main.c b/src/controller/main.c index ce413c49f3..6dbcc9d978 100644 --- a/src/controller/main.c +++ b/src/controller/main.c @@ -13,11 +13,11 @@ #include "controller.h" const struct option options[] = { - {ARG_PORT, required_argument, 0, ARG_PORT_SHORT }, - { ARG_CONFIG, required_argument, 0, ARG_CONFIG_SHORT }, - { ARG_HELP, no_argument, 0, ARG_HELP_SHORT }, - { ARG_VERSION, no_argument, 0, ARG_VERSION_SHORT}, - { NULL, 0, 0, '\0' } + { ARG_PORT, required_argument, 0, ARG_PORT_SHORT }, + { ARG_CONFIG, required_argument, 0, ARG_CONFIG_SHORT }, + { ARG_HELP, no_argument, 0, ARG_HELP_SHORT }, + { ARG_VERSION, no_argument, 0, ARG_VERSION_SHORT }, + { NULL, 0, 0, '\0' } }; #define GETOPT_OPTSTRING ARG_PORT_SHORT_S ARG_HELP_SHORT_S ARG_CONFIG_SHORT_S ARG_VERSION_SHORT_S diff --git a/src/libbluechi/test/cli/command/command_add_option_test.c b/src/libbluechi/test/cli/command/command_add_option_test.c index 903888e685..29deb47797 100644 --- a/src/libbluechi/test/cli/command/command_add_option_test.c +++ b/src/libbluechi/test/cli/command/command_add_option_test.c @@ -11,9 +11,9 @@ static const OptionType option_types[] = { - {'f', "force", 1 }, - { 'r', "reload", 11}, - { 0, NULL, 0 }, + { 'f', "force", 1 }, + { 'r', "reload", 11 }, + { 0, NULL, 0 }, }; int main() { diff --git a/src/libbluechi/test/cli/command/command_flag_exists_test.c b/src/libbluechi/test/cli/command/command_flag_exists_test.c index c6c1e36bc1..fb8d5cd67e 100644 --- a/src/libbluechi/test/cli/command/command_flag_exists_test.c +++ b/src/libbluechi/test/cli/command/command_flag_exists_test.c @@ -11,9 +11,9 @@ static const OptionType option_types[] = { - {'f', "force", 1 }, - { 'r', "reload", 11}, - { 0, NULL, 0 }, + { 'f', "force", 1 }, + { 'r', "reload", 11 }, + { 0, NULL, 0 }, }; int main() { diff --git a/src/libbluechi/test/cli/command/command_get_option_test.c b/src/libbluechi/test/cli/command/command_get_option_test.c index 405bda0981..6899e85a71 100644 --- a/src/libbluechi/test/cli/command/command_get_option_test.c +++ b/src/libbluechi/test/cli/command/command_get_option_test.c @@ -11,9 +11,9 @@ static const OptionType option_types[] = { - {'f', "force", 1 }, - { 'r', "reload", 11}, - { 0, NULL, 0 }, + { 'f', "force", 1 }, + { 'r', "reload", 11 }, + { 0, NULL, 0 }, }; int main() { diff --git a/src/libbluechi/test/cli/command/get_option_type_test.c b/src/libbluechi/test/cli/command/get_option_type_test.c index 161501dace..723307ccac 100644 --- a/src/libbluechi/test/cli/command/get_option_type_test.c +++ b/src/libbluechi/test/cli/command/get_option_type_test.c @@ -11,9 +11,9 @@ static const OptionType option_types[] = { - {0, "zero", 1 }, - { 1, "one", 11}, - { 0, NULL, 0 }, + { 0, "zero", 1 }, + { 1, "one", 11 }, + { 0, NULL, 0 }, }; bool check_option_type(const OptionType *got, OptionType *expected) { diff --git a/src/libbluechi/test/cli/command/methods_get_method_test.c b/src/libbluechi/test/cli/command/methods_get_method_test.c index b3723a2ed0..0339657029 100644 --- a/src/libbluechi/test/cli/command/methods_get_method_test.c +++ b/src/libbluechi/test/cli/command/methods_get_method_test.c @@ -22,9 +22,9 @@ static void usage_dummy() { } static const Method methods[] = { - {"dummy-one", 0, 0, 0, method_dummy_one, usage_dummy}, - { "dummy-two", 0, 0, 0, method_dummy_two, usage_dummy}, - { NULL, 0, 0, 0, NULL, NULL } + { "dummy-one", 0, 0, 0, method_dummy_one, usage_dummy }, + { "dummy-two", 0, 0, 0, method_dummy_two, usage_dummy }, + { NULL, 0, 0, 0, NULL, NULL } }; int main() { diff --git a/src/libbluechi/test/common/cfg/cfg_load_complete_configuration_test.c b/src/libbluechi/test/common/cfg/cfg_load_complete_configuration_test.c index f9e9ec7ec7..d1b93e6b6b 100644 --- a/src/libbluechi/test/common/cfg/cfg_load_complete_configuration_test.c +++ b/src/libbluechi/test/common/cfg/cfg_load_complete_configuration_test.c @@ -39,38 +39,38 @@ typedef struct cfg_test_param { } cfg_test_param; struct cfg_test_param test_data[] = { - /* basic single config file tests */ - {"no config files", + /* basic single config file tests */ + { "no config files", { NULL, NULL, { NULL, NULL }, NULL }, - 0, { { NULL, NULL }, { NULL, NULL }, { NULL, NULL } } }, + 0, { { NULL, NULL }, { NULL, NULL }, { NULL, NULL } } }, { "only default config file", { "NodeName = laptop\n", NULL, { NULL, NULL }, NULL }, - 0, { { "NodeName", "laptop" }, { NULL, NULL }, { NULL, NULL } } }, + 0, { { "NodeName", "laptop" }, { NULL, NULL }, { NULL, NULL } } }, { "only custom config file", { NULL, "NodeName = laptop\n", { NULL, NULL }, NULL }, - 0, { { "NodeName", "laptop" }, { NULL, NULL }, { NULL, NULL } } }, + 0, { { "NodeName", "laptop" }, { NULL, NULL }, { NULL, NULL } } }, { "only file in confd", { NULL, NULL, { "NodeName = laptop\n", NULL }, NULL }, - 0, { { "NodeName", "laptop" }, { NULL, NULL }, { NULL, NULL } } }, + 0, { { "NodeName", "laptop" }, { NULL, NULL }, { NULL, NULL } } }, { "only file via CLI", { NULL, NULL, { NULL, NULL }, "NodeName = laptop\n" }, - 0, { { "NodeName", "laptop" }, { NULL, NULL }, { NULL, NULL } } }, + 0, { { "NodeName", "laptop" }, { NULL, NULL }, { NULL, NULL } } }, - /* testing of loading order and overwrite */ + /* testing of loading order and overwrite */ { "with custom config file", { "NodeName = laptop\n", "NodeName = pi\n", { NULL, NULL }, NULL }, - 0, { { "NodeName", "pi" }, { NULL, NULL }, { NULL, NULL } } }, + 0, { { "NodeName", "pi" }, { NULL, NULL }, { NULL, NULL } } }, { "with config file in confd", { "NodeName = laptop\n", "NodeName = pi\n", { "NodeName = vm\n", NULL }, NULL }, - 0, { { "NodeName", "vm" }, { NULL, NULL }, { NULL, NULL } } }, + 0, { { "NodeName", "vm" }, { NULL, NULL }, { NULL, NULL } } }, { "with config CLI option", { "NodeName = laptop\n", "NodeName = pi\n", { "NodeName = vm\n", NULL }, "NodeName = container\n" }, - 0, { { "NodeName", "container" }, { NULL, NULL }, { NULL, NULL } }}, + 0, { { "NodeName", "container" }, { NULL, NULL }, { NULL, NULL } } }, { "with multiple config files in confd", { "NodeName = laptop\n", "NodeName = pi\n", { "NodeName = vm\n", "NodeName = container\n" }, NULL }, - 0, { { "NodeName", "container" }, { NULL, NULL }, { NULL, NULL } }}, + 0, { { "NodeName", "container" }, { NULL, NULL }, { NULL, NULL } } }, - /* testing of loading order, overwrite and merges */ + /* testing of loading order, overwrite and merges */ { "with multiple, duplicate keys", { "NodeName = laptop\nLogLevel=INFO\n", "NodeName = pi\n", @@ -79,21 +79,21 @@ struct cfg_test_param test_data[] = { 0, { { "NodeName", "pi" }, { "LogLevel", "DEBUG" }, { "LogTarget", "journald" }, - { "ControllerPort", "1337" } } }, + { "ControllerPort", "1337" } } }, - /* testing invalid entries */ + /* testing invalid entries */ { "with invalid value in default config file", { "NodeName 0 laptop\n", "NodeName = pi\n", { "LogLevel=DEBUG\n", "LogTarget=stderr\n" }, NULL }, -EINVAL, - { { NULL, NULL }, { NULL, NULL }, { NULL, NULL } } }, + { { NULL, NULL }, { NULL, NULL }, { NULL, NULL } } }, { "with invalid value in custom config file", { "NodeName = laptop\n", "NodeName 0 pi\n", { "LogLevel=DEBUG\n", "LogTarget=stderr\n" }, NULL }, -EINVAL, - { { "NodeName", "laptop" }, { NULL, NULL }, { NULL, NULL } } }, + { { "NodeName", "laptop" }, { NULL, NULL }, { NULL, NULL } } }, { "with invalid value in one of the confd files", { "NodeName = laptop\n", "NodeName = pi\n", { "LogLevel=DEBUG\n", "LogTarget0stderr\n" }, NULL }, -EINVAL, - { { "NodeName", "pi" }, { "LogLevel", "DEBUG" }, { NULL, NULL } } }, + { { "NodeName", "pi" }, { "LogLevel", "DEBUG" }, { NULL, NULL } } }, }; void create_file(const char *file_path, const char *cfg_file_content) { diff --git a/src/libbluechi/test/common/network/get_address_test.c b/src/libbluechi/test/common/network/get_address_test.c index 28c599f28b..28165006fc 100644 --- a/src/libbluechi/test/common/network/get_address_test.c +++ b/src/libbluechi/test/common/network/get_address_test.c @@ -21,13 +21,13 @@ typedef struct get_address_test_params { long unsigned int test_data_index = 0; struct get_address_test_params test_data[] = { - {NULL, true, NULL, -EINVAL}, - { ".", true, NULL, -1 }, - { "redhat", true, NULL, -1 }, - { "?10.10.10.3", true, NULL, -1 }, - { "192.168.1.", true, NULL, -1 }, - { "8.8.8.8", true, NULL, 0 }, - { "localhost.localdomain", true, "127.0.0.1", 0 }, + { NULL, true, NULL, -EINVAL }, + { ".", true, NULL, -1 }, + { "redhat", true, NULL, -1 }, + { "?10.10.10.3", true, NULL, -1 }, + { "192.168.1.", true, NULL, -1 }, + { "8.8.8.8", true, NULL, 0 }, + { "localhost.localdomain", true, "127.0.0.1", 0 }, }; int getaddrinfo_mock( diff --git a/src/libbluechi/test/log/bc_log_init_test.c b/src/libbluechi/test/log/bc_log_init_test.c index 62de8cbfd8..aab6cb504d 100644 --- a/src/libbluechi/test/log/bc_log_init_test.c +++ b/src/libbluechi/test/log/bc_log_init_test.c @@ -23,39 +23,39 @@ typedef struct log_test_param { struct log_test_param test_data[] = { - // test valid log targets - {"journald", "INFO", "false", bc_log_to_journald_with_location, LOG_LEVEL_INFO, false}, - { "stderr", "INFO", "false", bc_log_to_stderr_with_location, LOG_LEVEL_INFO, false}, - { "stderr-full", "INFO", "false", bc_log_to_stderr_full_with_location, LOG_LEVEL_INFO, false}, - // test valid log levels - { "journald", "DEBUG", "false", bc_log_to_journald_with_location, LOG_LEVEL_DEBUG, false}, - { "journald", "INFO", "false", bc_log_to_journald_with_location, LOG_LEVEL_INFO, false}, - { "journald", "ERROR", "false", bc_log_to_journald_with_location, LOG_LEVEL_ERROR, false}, - { "journald", "WARN", "false", bc_log_to_journald_with_location, LOG_LEVEL_WARN, false}, - // test valid is quiet - { "journald", "INFO", "true", bc_log_to_journald_with_location, LOG_LEVEL_INFO, true }, - - // test invalid log targets: should result in a default log function - { NULL, "INFO", "false", bc_log_to_journald_with_location, LOG_LEVEL_INFO, false}, - { "", "INFO", "false", bc_log_to_journald_with_location, LOG_LEVEL_INFO, false}, - { "stderr-ful", "INFO", "false", bc_log_to_journald_with_location, LOG_LEVEL_INFO, false}, - - // test invalid log levels: should result in a default log function - { "journald", NULL, "false", bc_log_to_journald_with_location, LOG_LEVEL_INFO, false}, - { "journald", "", "false", bc_log_to_journald_with_location, LOG_LEVEL_INFO, false}, - { "journald", "info", "false", bc_log_to_journald_with_location, LOG_LEVEL_INFO, false}, - { "journald", "just wrong", "false", bc_log_to_journald_with_location, LOG_LEVEL_INFO, false}, - - // test is quiet: different string values interpreted as true - { "journald", "INFO", "true", bc_log_to_journald_with_location, LOG_LEVEL_INFO, true }, - { "journald", "INFO", "t", bc_log_to_journald_with_location, LOG_LEVEL_INFO, true }, - { "journald", "INFO", "yes", bc_log_to_journald_with_location, LOG_LEVEL_INFO, true }, - { "journald", "INFO", "y", bc_log_to_journald_with_location, LOG_LEVEL_INFO, true }, - { "journald", "INFO", "on", bc_log_to_journald_with_location, LOG_LEVEL_INFO, true }, - // test is quiet: invalid values should result in false - { "journald", "INFO", "f", bc_log_to_journald_with_location, LOG_LEVEL_INFO, false}, - { "journald", "INFO", "false", bc_log_to_journald_with_location, LOG_LEVEL_INFO, false}, - { "journald", "INFO", "invalid", bc_log_to_journald_with_location, LOG_LEVEL_INFO, false}, + // test valid log targets + { "journald", "INFO", "false", bc_log_to_journald_with_location, LOG_LEVEL_INFO, false }, + { "stderr", "INFO", "false", bc_log_to_stderr_with_location, LOG_LEVEL_INFO, false }, + { "stderr-full", "INFO", "false", bc_log_to_stderr_full_with_location, LOG_LEVEL_INFO, false }, + // test valid log levels + { "journald", "DEBUG", "false", bc_log_to_journald_with_location, LOG_LEVEL_DEBUG, false }, + { "journald", "INFO", "false", bc_log_to_journald_with_location, LOG_LEVEL_INFO, false }, + { "journald", "ERROR", "false", bc_log_to_journald_with_location, LOG_LEVEL_ERROR, false }, + { "journald", "WARN", "false", bc_log_to_journald_with_location, LOG_LEVEL_WARN, false }, + // test valid is quiet + { "journald", "INFO", "true", bc_log_to_journald_with_location, LOG_LEVEL_INFO, true }, + + // test invalid log targets: should result in a default log function + { NULL, "INFO", "false", bc_log_to_journald_with_location, LOG_LEVEL_INFO, false }, + { "", "INFO", "false", bc_log_to_journald_with_location, LOG_LEVEL_INFO, false }, + { "stderr-ful", "INFO", "false", bc_log_to_journald_with_location, LOG_LEVEL_INFO, false }, + + // test invalid log levels: should result in a default log function + { "journald", NULL, "false", bc_log_to_journald_with_location, LOG_LEVEL_INFO, false }, + { "journald", "", "false", bc_log_to_journald_with_location, LOG_LEVEL_INFO, false }, + { "journald", "info", "false", bc_log_to_journald_with_location, LOG_LEVEL_INFO, false }, + { "journald", "just wrong", "false", bc_log_to_journald_with_location, LOG_LEVEL_INFO, false }, + + // test is quiet: different string values interpreted as true + { "journald", "INFO", "true", bc_log_to_journald_with_location, LOG_LEVEL_INFO, true }, + { "journald", "INFO", "t", bc_log_to_journald_with_location, LOG_LEVEL_INFO, true }, + { "journald", "INFO", "yes", bc_log_to_journald_with_location, LOG_LEVEL_INFO, true }, + { "journald", "INFO", "y", bc_log_to_journald_with_location, LOG_LEVEL_INFO, true }, + { "journald", "INFO", "on", bc_log_to_journald_with_location, LOG_LEVEL_INFO, true }, + // test is quiet: invalid values should result in false + { "journald", "INFO", "f", bc_log_to_journald_with_location, LOG_LEVEL_INFO, false }, + { "journald", "INFO", "false", bc_log_to_journald_with_location, LOG_LEVEL_INFO, false }, + { "journald", "INFO", "invalid", bc_log_to_journald_with_location, LOG_LEVEL_INFO, false }, }; diff --git a/src/proxy/main.c b/src/proxy/main.c index f154d78dd2..b9d893b595 100644 --- a/src/proxy/main.c +++ b/src/proxy/main.c @@ -15,10 +15,10 @@ #include "libbluechi/log/log.h" const struct option options[] = { - {ARG_USER, no_argument, 0, ARG_USER_SHORT }, - { ARG_HELP, no_argument, 0, ARG_HELP_SHORT }, - { ARG_VERSION, no_argument, 0, ARG_VERSION_SHORT}, - { NULL, 0, 0, '\0' } + { ARG_USER, no_argument, 0, ARG_USER_SHORT }, + { ARG_HELP, no_argument, 0, ARG_HELP_SHORT }, + { ARG_VERSION, no_argument, 0, ARG_VERSION_SHORT }, + { NULL, 0, 0, '\0' } }; #define GETOPT_OPTSTRING ARG_HELP_SHORT_S ARG_USER_SHORT_S ARG_VERSION_SHORT_S From 6e23e2d63290008732eb45f4e87c0f4bdeec7c9d Mon Sep 17 00:00:00 2001 From: Michael Engel Date: Fri, 9 Aug 2024 16:48:38 +0200 Subject: [PATCH 4/4] Fixed code linting issues due to updated build-base image In the new build-base image, a newer clang version with an updated set of checks is detecting issues. The issues - bugprone-multi-level-implicit-pointerconversion - cppcoreguidelines-macro-to-enum have been added to the ignore list while others have been fixed. Signed-off-by: Michael Engel --- .clang-tidy | 4 +++- src/agent/agent.c | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 301b31cd59..c076058c73 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -25,7 +25,9 @@ Checks: > -modernize-macro-to-enum, -llvm-else-after-return, -readability-else-after-return, - -misc-include-cleaner + -misc-include-cleaner, + -bugprone-multi-level-implicit-pointer-conversion, + -cppcoreguidelines-macro-to-enum CheckOptions: - key: readability-function-cognitive-complexity.Threshold diff --git a/src/agent/agent.c b/src/agent/agent.c index c2ad36c747..0b94a99ec4 100644 --- a/src/agent/agent.c +++ b/src/agent/agent.c @@ -1384,7 +1384,7 @@ static int agent_method_subscribe(sd_bus_message *m, void *userdata, UNUSED sd_b } agent->wildcard_subscription_active = true; - AgentUnitInfo info = { NULL, (char *) unit, true, true, -1, NULL }; + AgentUnitInfo info = { NULL, (char *) unit, true, true, _UNIT_ACTIVE_STATE_INVALID, NULL }; agent_emit_unit_new(agent, &info, "virtual"); return sd_bus_reply_method_return(m, "");