libdlog: initialise through attribute constructor

The previous implementation suffered from the double-checked locking
anti-pattern. It is possible for writes to be reordered either by the
compiler OR the hardware (for example armv7l does this), potentially
setting the initialisation flag before having actually initialized
the rest of the library, meaning a parallel thread could see the flag
and try to use uninitialised variables.

Using the constructor attribute solves this neatly since it is done
before any parallel threads can do anything; even runtime invocation
through `dlopen()` is safe since in that case other threads won't be
able to access the pointer to `dlog_print` before `dlopen()` returns,
by which time the constructor must have finished.

Change-Id: Ic55a6d6858dc2112efd67338894a0c44b876fee8
Signed-off-by: Michal Bloch <m.bloch@samsung.com>

diff --git a/src/libdlog/log.c b/src/libdlog/log.c
index 041d04d..37a47c7 100644 (file)
--- a/src/libdlog/log.c
+++ b/src/libdlog/log.c
@@ -33,8 +33,6 @@
 #define DEFAULT_CONFIG_DEBUGMODE 0
 #define DEFAULT_CONFIG_LIMITER_APPLY_TO_ALL_BUFFERS 0
 
-static int __write_to_log_null(log_id_t, log_priority, const char *, const char *);
-
 /**
  * @brief Points to a function which writes a log message
  * @details The function pointed to depends on the backend used
@@ -45,7 +43,7 @@ static int __write_to_log_null(log_id_t, log_priority, const char *, const char
  * @return Returns the number of bytes written on success and a negative error value on error.
  * @see __dlog_init_backend
  */
-int (*write_to_log)(log_id_t log_id, log_priority prio, const char *tag, const char *msg) = __write_to_log_null;
+int (*write_to_log)(log_id_t log_id, log_priority prio, const char *tag, const char *msg) = NULL;
 pthread_mutex_t log_init_lock = PTHREAD_MUTEX_INITIALIZER;
 extern void __dlog_init_pipe(const struct log_config *conf);
 extern void __dlog_init_android(const struct log_config *conf);
@@ -61,20 +59,6 @@ static int debugmode;
 static int fatal_assert;
 static int limiter_apply_to_all_buffers;
 
-/**
- * @brief Null handler
- * @details Ignores a log
- * @param[in] log_id ID of the buffer to log to. Belongs to (LOG_ID_INVALID, LOG_ID_MAX) non-inclusive
- * @param[in] prio Priority of the message.
- * @param[in] tag The message tag, identifies the sender.
- * @param[in] msg The contents of the message.
- * @return DLOG_ERROR_NOT_PERMITTED
- */
-static int __write_to_log_null(log_id_t log_id, log_priority prio, const char *tag, const char *msg)
-{
-       return DLOG_ERROR_NOT_PERMITTED;
-}
-
 static void __configure_limiter(struct log_config *config)
 {
        assert(config);
@@ -153,7 +137,11 @@ void __update_plog(const struct log_config *conf)
  * @brief Configure the library
  * @details Reads relevant config values
  */
-static void __configure(void)
+#ifdef UNIT_TEST
+void __configure(void)
+#else
+static void __attribute__((constructor)) __configure(void)
+#endif
 {
        struct log_config config;
 
@@ -175,26 +163,6 @@ out:
 }
 
 /**
- * @brief DLog init
- * @details Initializes the library
- */
-static bool is_initialized = false;
-static inline void __dlog_init(void)
-{
-       if (is_initialized)
-               return;
-
-       pthread_mutex_lock(&log_init_lock);
-
-       if (!is_initialized) {
-               __configure();
-               is_initialized = true;
-       }
-
-       pthread_mutex_unlock(&log_init_lock);
-}
-
-/**
  * @brief Fatal assertion
  * @details Conditionally crash the sucka who sent the log
  * @param[in] prio Priority of the log
@@ -269,7 +237,8 @@ static int __write_to_log(log_id_t log_id, int prio, const char *tag, const char
        if (ret < 0)
                return ret;
 
-       __dlog_init();
+       if (!write_to_log)
+               return DLOG_ERROR_NOT_PERMITTED;
 
        /* if limiter_apply_to_all_buffers config variable is set to 1,
         * check_should_log value does not matter and the entry is always
@@ -349,8 +318,6 @@ void __dlog_fini(void)
 {
        __log_limiter_destroy();
        __dynamic_config_destroy();
-       write_to_log = __write_to_log_null;
-       is_initialized = false;
 }
 #endif
 

diff --git a/src/tests/libdlog_base.c b/src/tests/libdlog_base.c
index cfebb03..4cbea4a 100644 (file)
--- a/src/tests/libdlog_base.c
+++ b/src/tests/libdlog_base.c
@@ -10,6 +10,7 @@
 
 extern int (*write_to_log)(log_id_t, log_priority, const char *tag, const char *msg);
 void __dlog_fini(void);
+void __configure(void);
 
 void __log_limiter_update(struct log_config *config) { }
 void __log_limiter_destroy(void) { }
@@ -127,16 +128,15 @@ void tct_tests()
 
 int main()
 {
-       __dlog_fini();
-       __dlog_fini();
-
        fail_conf_read = true;
+       __configure();
        assert(dlog_print(DLOG_ERROR, "tag", "msg") == DLOG_ERROR_NOT_PERMITTED);
        assert(write_to_log != wtl_android);
        assert(write_to_log != wtl_pipe);
        fail_conf_read = false;
        __dlog_fini();
 
+       __configure();
        assert(dlog_print(DLOG_ERROR, "tag", "msg") == DLOG_ERROR_NOT_PERMITTED);
        assert(write_to_log != wtl_android);
        assert(write_to_log != wtl_pipe);
@@ -146,13 +146,16 @@ int main()
        __dlog_fini();
 
        log_config_set(&CONFIG, "backend", "Hej, zamawiam Bentoye. Czekam do 11:15.");
+       __configure();
        assert(dlog_print(DLOG_ERROR, "tag", "msg") == DLOG_ERROR_NOT_PERMITTED);
        assert(write_to_log != wtl_pipe);
        assert(write_to_log != wtl_android);
        __dlog_fini();
 
+
        log_config_set(&CONFIG, "backend", "pipe");
        log_config_set(&CONFIG, "limiter", "0");
+       __configure();
        assert(dlog_print(DLOG_ERROR, "tag", "msg") == 0xDECC16);
        assert(!dynamic_config_called);
        assert(write_to_log == wtl_pipe);
@@ -164,6 +167,7 @@ int main()
        log_config_set(&CONFIG, "debugmode", "0");
        use_dynamic_conf = true;
        limiter_ret = 1;
+       __configure();
        assert(dlog_print(DLOG_ERROR, "tag", "msg") == 45835705);
        assert(__dlog_print(LOG_ID_MAIN, DLOG_ERROR, "tag", "msg") == 45835705);
        assert(write_to_log != wtl_pipe); // ceci n'est pas une pipe
@@ -186,13 +190,16 @@ int main()
        __dlog_fini();
 
        log_config_set(&CONFIG, "enable_system", "0");
+       __configure();
        assert(__dlog_print(LOG_ID_SYSTEM, DLOG_ERROR, "tag", "msg") == DLOG_ERROR_NONE);
        __dlog_fini();
 
        fail_snprintf = true;
+       __configure();
        assert(__dlog_print(LOG_ID_SYSTEM, DLOG_ERROR, "tag", "msg") == 45835705);
        fail_snprintf = false;
        __dlog_fini();
 
+       __configure();
        tct_tests();
 }