Skip to content

Commit

Permalink
improved logging for loading configuration files
Browse files Browse the repository at this point in the history 
Signed-off-by: Michael Engel <mengel@redhat.com>
  • Loading branch information
@engelmi
engelmi committed Mar 31, 2023
1 parent af53c70 commit 1a0de8a
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 41 deletions.
4 changes: 3 additions & 1 deletion src/agent/agent.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,6 @@ bool agent_parse_config(Agent *agent, const char *configfile) {
CFG_ETC_HIRTE_AGENT_CONF,
CFG_ETC_AGENT_CONF_DIR);
if (result != 0) {
fprintf(stderr, "Error loading configuration: '%s'.", strerror(-result));
cfg_dispose(agent->config);
return false;
}
Expand Down Expand Up @@ -544,6 +543,9 @@ bool agent_parse_config(Agent *agent, const char *configfile) {
}
}

_cleanup_free_ const char *dumped_cfg = cfg_dump(agent->config);
hirte_log_debug_with_data("Final configuration used", "\n%s", dumped_cfg);

return true;
}

Expand Down
74 changes: 49 additions & 25 deletions src/libhirte/common/cfg.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,12 @@ static uint64_t option_hash(const void *item, uint64_t seed0, uint64_t seed1) {
return hash;
}

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wunused-parameter"
/*
* Handle used to store option parsed from a file into a configuration.
*
* Returns 1 on success, 0 on error as per ini library requirements.
*/
static int parsing_handler(void *user, const char *section, const char *name, const char *value) { // NOLINT
static int parsing_handler(void *user, const char *section, const char *name, const char *value) {
if (user == NULL) {
return 0;
}
Expand All @@ -63,24 +61,20 @@ static int parsing_handler(void *user, const char *section, const char *name, co
}
return 1;
}
#pragma GCC diagnostic pop

int cfg_initialize(struct config **config) {
struct config *new_cfg = malloc(sizeof(struct config));
if (new_cfg == NULL) {
// error during configuration structure initialization
return -ENOMEM;
}
new_cfg->cfg_store = hashmap_new(
sizeof(struct config_option), 0, 0, 0, option_hash, option_compare, option_destroy, NULL);
if (new_cfg->cfg_store == NULL) {
// error during hashmap initialization
free(new_cfg);
return -ENOMEM;
}
char *section_copy = strdup(CFG_SECT_GLOBAL);
if (section_copy == NULL) {
// error during default section initialization
hashmap_free(new_cfg->cfg_store);
free(new_cfg);
return -ENOMEM;
Expand All @@ -106,23 +100,31 @@ int cfg_load_complete_configuration(

if (default_config_file != NULL) {
result = cfg_load_from_file(config, default_config_file);
if (result != 0) {
return result;
if (result < 0) {
fprintf(stderr,
"Error loading default configuration file '%s': '%s'.\n",
default_config_file,
strerror(-result));
}
}

if (custom_config_file != NULL) {
result = cfg_load_from_file(config, custom_config_file);
// if custom config file doesn't exist, continue
if (result != 0 && result != -ENOENT) {
return result;
if (result < 0) {
fprintf(stderr,
"Error loading custom configuration file '%s': '%s'.\n",
custom_config_file,
strerror(-result));
}
}

if (custom_config_directory != NULL) {
result = cfg_load_from_dir(config, custom_config_directory);
if (result != 0) {
return result;
if (result < 0) {
fprintf(stderr,
"Error loading configuration from conf.d dir '%s': '%s'.\n",
custom_config_directory,
strerror(-result));
}
}

Expand All @@ -135,8 +137,8 @@ int cfg_load_from_file(struct config *config, const char *config_file) {
if (config_file == NULL) {
return -EINVAL;
}
if (access(config_file, F_OK) != 0) {
return -ENOENT;
if (access(config_file, R_OK) != 0) {
return -errno;
}
return ini_parse(config_file, parsing_handler, config);
}
Expand All @@ -160,7 +162,10 @@ int cfg_load_from_dir(struct config *config, const char *custom_config_directory

for (int i = 0; i < number_of_files; i++) {
char *file_name = all_files_in_dir[i]->d_name;
cfg_load_from_file(config, file_name);
int r = cfg_load_from_file(config, file_name);
if (r < 0) {
fprintf(stderr, "Error loading confd file '%s': '%s'.\n", file_name, strerror(-r));
}
free(all_files_in_dir[i]);
}
free(all_files_in_dir);
Expand All @@ -176,9 +181,12 @@ int cfg_load_from_env(struct config *config) {
for (int i = 0; i < length; i++) {
char *value = getenv(env_vars[i]);
if (value != NULL && strlen(value) > 0) {
// environment variable is defined and has a non-empty value, so store it to the configuration
int result = cfg_set_value(config, option_names[i], value);
if (result != 0) {
if (result < 0) {
fprintf(stderr,
"Error setting env value for '%s': '%s'.\n",
option_names[i],
strerror(-result));
return result;
}
}
Expand Down Expand Up @@ -229,7 +237,6 @@ int cfg_s_set_value(

char *value_copy = NULL;
if (option_value != NULL && strlen(option_value) > 0) {
// storing non NULL value, so copy is needed
value_copy = strdup(option_value);
if (value_copy == NULL) {
free(section_copy);
Expand All @@ -242,15 +249,13 @@ int cfg_s_set_value(
&(struct config_option){
.section = section_copy, .name = name_copy, .value = value_copy });
if (hashmap_oom(config->cfg_store)) {
// OOM during hashmap set operation
free(section_copy);
free(name_copy);
free(value_copy);
return -ENOMEM;
}

if (replaced != NULL) {
// option has been replaced with the new one, we need to deallocate the old one
free(replaced->section);
free(replaced->name);
free(replaced->value);
Expand Down Expand Up @@ -288,6 +293,25 @@ bool cfg_s_get_bool_value(struct config *config, const char *section_name, const
return result;
}

void config_iterate(struct config *config, bool (*iter)(const void *item, void *udata), void *udata) {
hashmap_scan(config->cfg_store, iter, udata);
}
const char *cfg_dump(struct config *config) {
if (config == NULL) {
return NULL;
}

int r = 0;
char *cfg_info = "";
void *item = NULL;
size_t i = 0;
while (hashmap_iter(config->cfg_store, &i, &item)) {
struct config_option *opt = item;
char *tmp = cfg_info;
r = asprintf(&cfg_info, "%s%s=%s\n", cfg_info, opt->name, opt->value);
if (!streq(tmp, "")) {
free(tmp);
}
if (r < 0) {
return NULL;
}
}
return cfg_info;
}
14 changes: 1 addition & 13 deletions src/libhirte/common/cfg.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,16 +175,4 @@ bool cfg_get_bool_value(struct config *config, const char *option_name);
*/
bool cfg_s_get_bool_value(struct config *config, const char *section, const char *option_name);

/*
* Iterate over configuration and for each item execute the iter function.
* For example using following iterator method would print the whole configuration:
*
* bool config_option_iterator(const void *item, void *udata) {
* const struct config_option *option = item;
* printf("%s = %s\n", option->name, option->value);
* return true;
* }
*
* WARNING: configuration items are accessed directly, DO NOT modify them during iteration!!!
*/
void cfg_iterate(struct config *config, bool (*iter)(const void *item, void *udata), void *udata);
const char *cfg_dump(struct config *config);
18 changes: 18 additions & 0 deletions src/libhirte/test/common/cfg_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <unistd.h>

#include "libhirte/common/cfg.h"
#include "libhirte/common/common.h"

void _config_set_and_get(
struct config *config,
Expand Down Expand Up @@ -325,6 +326,21 @@ void test_env_override_config() {
dispose_env();
}

void test_config_dump() {
struct config *config = NULL;
cfg_initialize(&config);

_config_set_and_get(config, "key1", "value1", "value1", false);
_config_set_and_get(config, "key2", "value4", "value4", false);

const char *expected_cfg_dump = "key1=value1\nkey2=value4\n";

_cleanup_free_ const char *cfg = cfg_dump(config);
assert(streq(cfg, expected_cfg_dump));

cfg_dispose(config);
}

int main() {
test_config_set_get();
test_config_set_get_bool();
Expand All @@ -335,5 +351,7 @@ int main() {
test_default_section();
test_parse_config_from_env();
test_env_override_config();
test_config_dump();

return EXIT_SUCCESS;
}
6 changes: 4 additions & 2 deletions src/manager/manager.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,6 @@ bool manager_parse_config(Manager *manager, const char *configfile) {
result = cfg_load_complete_configuration(
manager->config, CFG_HIRTE_DEFAULT_CONFIG, CFG_ETC_HIRTE_CONF, CFG_ETC_HIRTE_CONF_DIR);
if (result != 0) {
fprintf(stderr, "Error loading configuration '%s'.", strerror(result));
cfg_dispose(manager->config);
return false;
}
Expand Down Expand Up @@ -331,6 +330,9 @@ bool manager_parse_config(Manager *manager, const char *configfile) {
}
}

_cleanup_free_ const char *dumped_cfg = cfg_dump(manager->config);
hirte_log_debug_with_data("Final configuration used", "\n%s", dumped_cfg);

/* TODO: Handle per-node-name option section */

return true;
Expand Down Expand Up @@ -837,7 +839,7 @@ bool manager_start(Manager *manager) {
int r = sd_bus_request_name(
manager->api_bus, manager->api_bus_service_name, SD_BUS_NAME_REPLACE_EXISTING);
if (r < 0) {
hirte_log_errorf("Failed to acquire service name on user dbus: %s", strerror(-r));
hirte_log_errorf("Failed to acquire service name on api dbus: %s", strerror(-r));
return false;
}

Expand Down

0 comments on commit 1a0de8a

Please sign in to comment.