Use the actual bool type instead of a fake enum 44/193944/2
authorMichal Bloch <m.bloch@samsung.com>
Tue, 27 Nov 2018 18:20:07 +0000 (19:20 +0100)
committerHyotaek Shim <hyotaek.shim@samsung.com>
Fri, 30 Nov 2018 00:30:25 +0000 (00:30 +0000)
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 <m.bloch@samsung.com>
include/logconfig.h
include/ptrs_list.h
src/logger/log_storage.c
src/logger/logger.c
src/shared/logconfig.c
src/shared/ptrs_list.c
src/tests/config.c

index e3f08a5..2f39481 100644 (file)
@@ -18,6 +18,8 @@
 #ifndef _LOGCONFIG_H_
 #define _LOGCONFIG_H_
 
+#include <stdbool.h>
+
 #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);
index 4b51acf..aebd1ee 100644 (file)
@@ -17,6 +17,8 @@
 #ifndef PTRS_LIST_INCLUDED
 #define PTRS_LIST_INCLUDED
 
+#include <stdbool.h>
+
 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
index 9cb4a34..2d0c84b 100644 (file)
@@ -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_);
index 811435d..ddf8507 100644 (file)
@@ -25,6 +25,7 @@
 #include <ctype.h>
 #include <fcntl.h>
 #include <signal.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <pwd.h>
@@ -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)
index b415d91..edad451 100644 (file)
@@ -10,6 +10,7 @@
 #include <fcntl.h>
 #include <logcommon.h>
 #include <limits.h>
+#include <stdbool.h>
 
 /**
  * @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);
 }
 
 /**
index f5ea0da..21485ce 100644 (file)
@@ -22,8 +22,6 @@
 #include <stdlib.h>
 #include <ptrs_list.h>
 
-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)
index 884720b..85900ec 100644 (file)
@@ -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;
 }