From 227dc3051568014a2cfb83c8641b108f2083bb55 Mon Sep 17 00:00:00 2001 From: Michal Bloch Date: Tue, 27 Nov 2018 19:20:07 +0100 Subject: [PATCH] Use the actual bool type instead of a fake enum Improves type correctness and fixes a case of two functions being identical - `log_config_get_boolean` used to just call `log_config_get_int` with the same parameters and did not even touch the return value. Includes tests for `log_config_get_boolean`. Change-Id: I185dfac32d40bcab3ddbf95d3b4536c2d275098e Signed-off-by: Michal Bloch --- include/logconfig.h | 4 +++- include/ptrs_list.h | 10 ++++++---- src/logger/log_storage.c | 2 +- src/logger/logger.c | 29 ++++++++++++++--------------- src/shared/logconfig.c | 9 +++++---- src/shared/ptrs_list.c | 12 +++++------- src/tests/config.c | 14 ++++++++++++++ 7 files changed, 48 insertions(+), 32 deletions(-) diff --git a/include/logconfig.h b/include/logconfig.h index e3f08a5..2f39481 100644 --- a/include/logconfig.h +++ b/include/logconfig.h @@ -18,6 +18,8 @@ #ifndef _LOGCONFIG_H_ #define _LOGCONFIG_H_ +#include + #define MAX_CONF_KEY_LEN 32 #define MAX_CONF_VAL_LEN 256 #define MAX_CONF_ENTRY_LEN (MAX_CONF_KEY_LEN + MAX_CONF_VAL_LEN + 2) // +2 for the delimiter and newline @@ -49,7 +51,7 @@ extern "C" { void log_config_set(struct log_config *config, const char *key, const char *value); const char *log_config_get(const struct log_config *config, const char *key); int log_config_get_int(const struct log_config *config, const char *key, int default_val); -int log_config_get_boolean(const struct log_config *config, const char *key, int default_val); +bool log_config_get_boolean(const struct log_config *config, const char *key, bool default_val); int log_config_read(struct log_config *config); int log_config_read_file(struct log_config *config, const char *filename); void log_config_free(struct log_config *config); diff --git a/include/ptrs_list.h b/include/ptrs_list.h index 4b51acf..aebd1ee 100644 --- a/include/ptrs_list.h +++ b/include/ptrs_list.h @@ -17,6 +17,8 @@ #ifndef PTRS_LIST_INCLUDED #define PTRS_LIST_INCLUDED +#include + struct list_elem; /** @@ -38,9 +40,9 @@ typedef void *elem_value; * @brief Condition callback for list element * @param value list element value * @param user_data pointer to user data to be forwarded to callback - * @return nonzero if condition if fulfilled, zero if not + * @return true if condition if fulfilled, false if not */ -typedef int (*cond_cb)(elem_value value, void *user_data); +typedef bool (*cond_cb)(elem_value value, void *user_data); /** * @brief Apply callback to list element @@ -58,9 +60,9 @@ extern "C" { * @details Push front a new element * @param[in,out] head pointer to the list head * @param[in] value user data pointer - * @return nonzero on success, zero on error (bad alloc) + * @return true on success, false on allocation failure */ -int list_add(list_head *head, elem_value value); +bool list_add(list_head *head, elem_value value); /** * @brief Remove list elements equal to given parameter diff --git a/src/logger/log_storage.c b/src/logger/log_storage.c index 9cb4a34..2d0c84b 100644 --- a/src/logger/log_storage.c +++ b/src/logger/log_storage.c @@ -243,7 +243,7 @@ static void readers_move_all_to_another_entry(list_head *list_from, log_storage_ list_clear(list_from); } -static int log_storage_reader_check_final(void *reader_, void *lse_) +static bool log_storage_reader_check_final(void *reader_, void *lse_) { assert(reader_); assert(lse_); diff --git a/src/logger/logger.c b/src/logger/logger.c index 811435d..ddf8507 100644 --- a/src/logger/logger.c +++ b/src/logger/logger.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -60,8 +61,6 @@ enum { DLOG_EXIT_ERR_RUNTIME }; -enum { FALSE = 0, TRUE }; - /* Size in bytes of the pipe given to libdlog clients. * The default size (1 page, usually 4 kiB) is fairly small * which leads programs that log a lot to easily clog the @@ -710,9 +709,9 @@ static int buffer_create(struct log_buffer **log_buffer, log_id_t buf_id, struct * @details Use this function when deleting all readers in the list * @param[in] ptr pointer to the reader * @param[in] user_data pointer to the logger to remove readers waiting for an event - * @return always nonzero (the reader is to be removed) + * @return always true (the reader is to be removed) */ -static int cond_reader_free(void *ptr, void *user_data) +static bool cond_reader_free(void *ptr, void *user_data) { struct reader *reader = (struct reader *)ptr; struct logger *logger = (struct logger *)user_data; @@ -720,7 +719,7 @@ static int cond_reader_free(void *ptr, void *user_data) if (reader->fd_entity.fd >= 0) remove_fd_entity(logger, &reader->fd_entity); reader_free(reader); - return TRUE; + return true; } /** @@ -1618,9 +1617,9 @@ static int service_writer_kmsg(struct logger* server, struct writer* wr, struct * @details Use this function when servicing all readers in the list * @param[in] ptr pointer to the reader * @param[in] user_data pointer to the logger to add readers waiting for an event - * @return nonzero if the reader is to be removed + * @return true if the reader is to be removed */ -static int cond_service_reader(void *ptr, void *user_data) +static bool cond_service_reader(void *ptr, void *user_data) { assert(ptr); assert(user_data); @@ -1628,7 +1627,7 @@ static int cond_service_reader(void *ptr, void *user_data) struct logger *logger = (struct logger *)user_data; if (!logger->exiting && reader_should_buffer(reader, &logger->buf_params, logger->now)) - return FALSE; + return false; assert(reader->service_reader); int r = reader->service_reader(reader); @@ -1637,7 +1636,7 @@ static int cond_service_reader(void *ptr, void *user_data) if (reader->fd_entity.fd >= 0) remove_fd_entity(logger, &reader->fd_entity); reader_free(reader); - return TRUE; + return true; } /* `service_reader()` returns -1 if everything was flushed, or 0 if @@ -1656,7 +1655,7 @@ static int cond_service_reader(void *ptr, void *user_data) * to such buffer would leak until a log finally arrived (which could * be never). This is why waiting is also done on EPOLLHUP. */ modify_fd_entity(logger, &reader->fd_entity, (r == 0) ? EPOLLOUT : EPOLLHUP); - return FALSE; + return false; } /** @@ -1828,10 +1827,10 @@ static int logger_create(struct logger_config_data *data, struct logger *l) return 0; } -static int cond_writer_free(void *ptr, void *user_data) +static bool cond_writer_free(void *ptr, void *user_data) { writer_free((struct writer *)ptr, user_data); - return TRUE; + return true; } /** @@ -2157,7 +2156,7 @@ int prepare_config_data(struct logger_config_data *data, int argc, char **argv) } data->is_buffer_enabled[LOG_ID_KMSG] = 1; data->is_buffer_enabled[LOG_ID_SYSLOG] = - dev_log_sock_get() >= 0 || log_config_get_boolean(&conf, "syslog_force", FALSE); + dev_log_sock_get() >= 0 || log_config_get_boolean(&conf, "syslog_force", false); for (log_id_t buf_id = 0; buf_id < LOG_ID_MAX; ++buf_id) { if (data->is_buffer_enabled[buf_id]) { @@ -2175,10 +2174,10 @@ end: return ret; } -static int cond_string_free(void *ptr, void *user_data) +static bool cond_string_free(void *ptr, void *user_data) { free(ptr); - return TRUE; + return true; } static void free_config_data(struct logger_config_data *data) diff --git a/src/shared/logconfig.c b/src/shared/logconfig.c index b415d91..edad451 100644 --- a/src/shared/logconfig.c +++ b/src/shared/logconfig.c @@ -10,6 +10,7 @@ #include #include #include +#include /** * @addtogroup SHARED_FUNCTIONS @@ -94,16 +95,16 @@ int log_config_get_int(const struct log_config *c, const char *key, int default_ /** * @brief Get a config value converted to boolean - * @details Returns TRUE if config string is non-zero digital number + * @details Returns true if config string is a non-zero number * @param[in] c The config to work with * @param[in] key The key of the value entry to get * @param[in] default_val Default boolean value to return when proper value not given in config - * @return The value of the entry if such exists, else default value + * @return If the entry exists and is a number, its value cast to boolean; else default value * @see log_config_get_int */ -int log_config_get_boolean(const struct log_config *c, const char *key, int default_val) +bool log_config_get_boolean(const struct log_config *c, const char *key, bool default_val) { - return log_config_get_int(c, key, default_val); + return !!log_config_get_int(c, key, default_val); } /** diff --git a/src/shared/ptrs_list.c b/src/shared/ptrs_list.c index f5ea0da..21485ce 100644 --- a/src/shared/ptrs_list.c +++ b/src/shared/ptrs_list.c @@ -22,8 +22,6 @@ #include #include -enum { FALSE = 0, TRUE }; - struct list_elem { struct list_elem *next; elem_value value; @@ -57,9 +55,9 @@ static struct list_elem *list_remove_at(struct list_elem *elem) * with some primitive values like integers * @param[in] value list value to compare * @param[in] user_data pointer to value to compare with - * @return nonzero if user data points on the same value + * @return true if user data points on the same value */ -static int cond_value_equal(elem_value value, void *user_data) +static bool cond_value_equal(elem_value value, void *user_data) { return (value == *((elem_value *)user_data)); } @@ -76,13 +74,13 @@ static void apply_increment_counter(elem_value value, void *user_data) (*counter)++; } -int list_add(list_head *head, elem_value value) +bool list_add(list_head *head, elem_value value) { assert(head); //valid pointer struct list_elem *elem = (struct list_elem *)calloc(1, sizeof(struct list_elem)); if (elem == NULL) - return FALSE; + return false; *elem = (struct list_elem) { .next = *head, @@ -90,7 +88,7 @@ int list_add(list_head *head, elem_value value) }; (*head) = elem; - return TRUE; + return true; } void list_remove(list_head *head, elem_value value) diff --git a/src/tests/config.c b/src/tests/config.c index 884720b..85900ec 100644 --- a/src/tests/config.c +++ b/src/tests/config.c @@ -26,9 +26,11 @@ int main() get = log_config_get(&config, "valid_extra_conf"); assert(get); assert(!strncmp(get, "123", 4)); + assert(log_config_get_int(&config, "valid_extra_conf", 567) == 123); get = log_config_get(&config, "invalid_extra_conf"); assert(!get); + assert(log_config_get_int(&config, "invalid_extra_conf", 678) == 678); /* test existing key modification */ log_config_set(&config, "foo", "quux"); @@ -43,5 +45,17 @@ int main() assert(get); assert(!strncmp(get, "ok", 3)); + /* test boolean conversion */ + log_config_set(&config, "b_true" , "1" ); + log_config_set(&config, "b_false", "0" ); + log_config_set(&config, "b_none" , "foo"); + + assert( log_config_get_boolean(&config, "b_true" , true )); + assert( log_config_get_boolean(&config, "b_true" , false)); + assert(!log_config_get_boolean(&config, "b_false", true )); + assert(!log_config_get_boolean(&config, "b_false", false)); + assert( log_config_get_boolean(&config, "b_none" , true )); + assert(!log_config_get_boolean(&config, "b_none" , false)); + return 0; } -- 2.7.4