Improve __log_limiter_pass_log interface

Change-Id: Ie47e84c25a5ed615e52a69e51bde2018d987f06d

diff --git a/include/loglimiter.h b/include/loglimiter.h
index 7f58582..c3ae5c0 100644 (file)
--- a/include/loglimiter.h
+++ b/include/loglimiter.h
@@ -52,7 +52,18 @@ struct pid_limit {
 
 void __log_limiter_destroy(void);
 
-int __log_limiter_pass_log(const char* tag, int prio);
+struct pass_log_result {
+       enum {
+               DECISION_ALLOWED,
+               DECISION_DENIED,
+               DECISION_TAG_LIMIT_EXCEEDED_MESSAGE,
+       } decision;
+
+       int logs_per_period;
+       int period_s;
+};
+
+struct pass_log_result __log_limiter_pass_log(const char* tag, int prio);
 
 int __log_limiter_create(const struct log_config *config);
 

diff --git a/src/libdlog/log.c b/src/libdlog/log.c
index 11afb3e..a989744 100644 (file)
--- a/src/libdlog/log.c
+++ b/src/libdlog/log.c
@@ -302,22 +302,31 @@ static int dlog_check_limiter(log_id_t log_id, int prio, const char *tag)
                __dynamic_config_update();
 
        if (limiter) {
-               int should_log = 0;
+               struct pass_log_result should_log = { .decision = DECISION_DENIED };
                if (!pthread_rwlock_rdlock(&log_limiter_lock)) {
                        should_log = __log_limiter_pass_log(tag, prio);
                        pthread_rwlock_unlock(&log_limiter_lock);
                }
 
-               if (!should_log) {
-                       return DLOG_ERROR_NOT_PERMITTED;
-               } else if (should_log < 0) {
-                       struct timespec tp;
-                       int result = clock_gettime(CLOCK_MONOTONIC, &tp);
-                       if (result < 0)
+               switch (should_log.decision) {
+                       case DECISION_DENIED:
                                return DLOG_ERROR_NOT_PERMITTED;
-                       write_to_log(log_id, prio, tag,
-                                       "Your log has been blocked due to limit of log lines per minute.", &tp);
-                       return DLOG_ERROR_NOT_PERMITTED;
+
+                       case DECISION_TAG_LIMIT_EXCEEDED_MESSAGE: {
+                               struct timespec tp;
+                               int result = clock_gettime(CLOCK_MONOTONIC, &tp);
+                               if (result < 0)
+                                       return DLOG_ERROR_NOT_PERMITTED;
+                               char buf[100] = {};
+                               snprintf(buf, sizeof(buf),
+                                               "Your log has been blocked due to per-tag limit of %d logs per %d seconds.",
+                                               should_log.logs_per_period, should_log.period_s);
+                               write_to_log(log_id, prio, tag, buf, &tp);
+                               return DLOG_ERROR_NOT_PERMITTED;
+                       }
+
+                       case DECISION_ALLOWED:
+                               break;
                }
        }
 

diff --git a/src/shared/loglimiter.c b/src/shared/loglimiter.c
index af9e095..c573ffa 100644 (file)
--- a/src/shared/loglimiter.c
+++ b/src/shared/loglimiter.c
@@ -407,22 +407,20 @@ struct limiter_limits __log_limiter_get_limits(const char *tag, int prio)
 /* Function implement logic needed to decide,
  * whenever message is written to log or not.
  *
- * Possible return values are:
- *     0  - to indicate that message is deny to write into log.
- *     (-1) - to indicate that limit of the messages is reached.
- *     1  - to indicate that message is allowed to write into log.
+ * On first time being blocked, it will return *_LIMIT_EXCEEDED_MESSAGE in the decision field,
+ * which allows the callee to write a relevant message to logs.
  */
-int __log_limiter_pass_log(const char* tag, int prio)
+struct pass_log_result __log_limiter_pass_log(const char* tag, int prio)
 {
        if (block_by_pid())
-               return 0;
+               return (struct pass_log_result) { .decision = DECISION_DENIED, };
 
        if (!rules_hashmap)
-               return 1;
+               return (struct pass_log_result) { .decision = DECISION_ALLOWED, };
 
        /* allow empty-tagged messages and make it easy to catch an application that does that */
        if (!strlen(tag))
-               return 1;
+               return (struct pass_log_result) { .decision = DECISION_ALLOWED, };
 
        const char prio_c = util_prio_to_char(prio);
        struct rule *r =
@@ -432,34 +430,38 @@ int __log_limiter_pass_log(const char* tag, int prio)
                hashmap_search(rules_hashmap, "*", '*');
 
        if (!r)
-               return 1;
+               return (struct pass_log_result) { .decision = DECISION_ALLOWED, };
        if (!r->limit)
-               return 0;
+               return (struct pass_log_result) { .decision = DECISION_DENIED, };
        if (__LOG_LIMITER_LIMIT_MAX < r->limit)
-               return 1;
+               return (struct pass_log_result) { .decision = DECISION_ALLOWED, };
 
        /* Decide, if it should go through or stop */
        time_t now = time(NULL);
 
        if (0 > now)
-               return 1;
+               return (struct pass_log_result) { .decision = DECISION_ALLOWED, };
 
        if (now - r->start <= refresh_rate_s) {
                if (r->hit >= 0) {
                        if (r->hit < r->limit) {
                                r->hit++;
-                               return 1;
+                               return (struct pass_log_result) { .decision = DECISION_ALLOWED, };
                        }
                        r->hit = INT_MIN+1;
-                       return (-1);
+                       return (struct pass_log_result) {
+                               .decision = DECISION_TAG_LIMIT_EXCEEDED_MESSAGE,
+                               .logs_per_period = r->limit,
+                               .period_s = refresh_rate_s,
+                       };
                } else {
                        r->hit++;
-                       return 0;
+                       return (struct pass_log_result) { .decision = DECISION_DENIED, };
                }
        } else {
                r->start = now;
                r->hit = 0;
-               return 1;
+               return (struct pass_log_result) { .decision = DECISION_ALLOWED, };
        }
 }
 

diff --git a/src/tests/libdlog_base_neg.c b/src/tests/libdlog_base_neg.c
index 9e765c9..40b7a93 100644 (file)
--- a/src/tests/libdlog_base_neg.c
+++ b/src/tests/libdlog_base_neg.c
@@ -46,7 +46,7 @@ int main()
        log_config_set(&CONFIG, "limiter", "1");
        log_config_set(&CONFIG, "debugmode", "0");
        use_dynamic_conf = true;
-       limiter_ret = 1;
+       limiter_ret = (struct pass_log_result) { .decision = DECISION_ALLOWED };
        __configure();
        assert(write_to_log != wtl_pipe); // ceci n'est pas une pipe
        assert(write_to_log == wtl_android);
@@ -58,7 +58,7 @@ int main()
        assert(__dlog_sec_print((log_id_t) -1, DLOG_ERROR, "tag", "msg") == DLOG_ERROR_INVALID_PARAMETER);
        assert(__dlog_sec_print(LOG_ID_MAIN, DLOG_ERROR, NULL , "msg") == DLOG_ERROR_INVALID_PARAMETER);
 
-       limiter_ret = 1;
+       limiter_ret = (struct pass_log_result) { .decision = DECISION_ALLOWED };
        __dlog_fini();
 
        fail_snprintf = true;

diff --git a/src/tests/libdlog_base_pos.c b/src/tests/libdlog_base_pos.c
index 9d398aa..a045a56 100644 (file)
--- a/src/tests/libdlog_base_pos.c
+++ b/src/tests/libdlog_base_pos.c
@@ -31,7 +31,7 @@ int main()
        log_config_set(&CONFIG, "limiter", "1");
        log_config_set(&CONFIG, "debugmode", "0");
        use_dynamic_conf = true;
-       limiter_ret = 1;
+       limiter_ret = (struct pass_log_result) { .decision = DECISION_ALLOWED };
        __configure();
        assert(dlog_print(DLOG_ERROR, "tag", "msg") == 45835705);
        assert(__dlog_print(LOG_ID_MAIN, DLOG_ERROR, "tag", "msg") == 45835705);
@@ -40,10 +40,10 @@ int main()
        assert(limiter_created);
        assert(dynamic_config_called);
 
-       limiter_ret = 0;
+       limiter_ret = (struct pass_log_result) { .decision = DECISION_DENIED };
        assert(dlog_print(DLOG_ERROR, "tag", "msg") == 45835705);
        assert(__dlog_print(LOG_ID_MAIN, DLOG_ERROR, "tag", "msg") == DLOG_ERROR_NONE);
-       limiter_ret = -1;
+       limiter_ret = (struct pass_log_result) { .decision = DECISION_TAG_LIMIT_EXCEEDED_MESSAGE };
        assert(dlog_print(DLOG_ERROR, "tag", "msg") == 45835705);
        assert(__dlog_print(LOG_ID_MAIN, DLOG_ERROR, "tag", "msg") == DLOG_ERROR_NONE);
 
@@ -56,7 +56,7 @@ int main()
        assert(__dlog_sec_print(LOG_ID_MAIN, DLOG_DEBUG, "tag", "msg") == 0);
        assert(__dlog_sec_print(LOG_ID_MAIN, DLOG_DEBUG, "tag", "msg") == 0);
 
-       limiter_ret = 1;
+       limiter_ret = (struct pass_log_result) { .decision = DECISION_ALLOWED };
        __dlog_fini();
        log_config_set(&CONFIG, "enable_system", "0");
        __configure();

diff --git a/src/tests/libdlog_base_wrap.c b/src/tests/libdlog_base_wrap.c
index f4ba0a9..dac335d 100644 (file)
--- a/src/tests/libdlog_base_wrap.c
+++ b/src/tests/libdlog_base_wrap.c
@@ -24,8 +24,9 @@
 #include <dlog.h>
 #include <libdlog.h>
 #include <logconfig.h>
+#include <loglimiter.h>
 
-void __log_limiter_update(struct log_config *config) { }
+void __log_limiter_update(const struct log_config *config) { }
 void __log_limiter_destroy(void) { }
 void __dynamic_config_destroy() { }
 
@@ -60,11 +61,11 @@ int __wrap_log_config_read(struct log_config *config)
        return 0;
 }
 
-int limiter_ret;
-int __log_limiter_pass_log(const char* tag, int prio) { return limiter_ret; }
+struct pass_log_result limiter_ret;
+struct pass_log_result __log_limiter_pass_log(const char* tag, int prio) { return limiter_ret; }
 
 static bool limiter_created;
-int __log_limiter_create(struct log_config *config)
+int __log_limiter_create(const struct log_config *config)
 {
        limiter_created = true;
        return 1;

diff --git a/src/tests/libdlog_prio_filter_pos.c b/src/tests/libdlog_prio_filter_pos.c
index ad03858..f70ea6a 100644 (file)
--- a/src/tests/libdlog_prio_filter_pos.c
+++ b/src/tests/libdlog_prio_filter_pos.c
@@ -22,6 +22,7 @@
 #include <dlog.h>
 #include <libdlog.h>
 #include <logconfig.h>
+#include <loglimiter.h>
 
 int wtl(log_id_t buf_id, log_priority pri, const char *tag, const char *msg, struct timespec *tp_mono) { return 0xABCD; }
 void __dlog_init_pipe(const struct log_config *conf) { write_to_log = wtl; }
@@ -35,11 +36,11 @@ int __wrap_log_config_read(struct log_config *config)
 }
 
 // lobotomize various mechanisms I don't want to deal with
-void __log_limiter_update(struct log_config *config) { }
+void __log_limiter_update(const struct log_config *config) { }
 void __log_limiter_destroy(void) { }
 void __dynamic_config_destroy() { }
-int __log_limiter_pass_log(const char* tag, int prio) { return 1; }
-int __log_limiter_create(struct log_config *config) { return 1; }
+struct pass_log_result __log_limiter_pass_log(const char* tag, int prio) { return (struct pass_log_result) { .decision = DECISION_ALLOWED }; }
+int __log_limiter_create(const struct log_config *config) { return 1; }
 bool __dynamic_config_create(struct log_config *config) { return false; }
 void __dynamic_config_update() { }
 void __dlog_init_android(const struct log_config *conf) { }

diff --git a/src/tests/limiter_neg.c b/src/tests/limiter_neg.c
index 4ae4aec..41afa66 100644 (file)
--- a/src/tests/limiter_neg.c
+++ b/src/tests/limiter_neg.c
@@ -37,17 +37,17 @@ int main()
        fail_malloc = 1;
        __log_limiter_update(&conf);
        for (int i = 0; i < 100; ++i) {
-               assert(1 == __log_limiter_pass_log("FOO", 'F'));
-               assert(1 == __log_limiter_pass_log("BAR", 'F'));
-               assert(1 == __log_limiter_pass_log("BAR", 'E'));
+               assert(__log_limiter_pass_log("FOO", 'F').decision == DECISION_ALLOWED);
+               assert(__log_limiter_pass_log("BAR", 'F').decision == DECISION_ALLOWED);
+               assert(__log_limiter_pass_log("BAR", 'E').decision == DECISION_ALLOWED);
        }
 
        fail_malloc = 2;
        __log_limiter_update(&conf);
        for (int i = 0; i < 100; ++i) {
-               assert(1 == __log_limiter_pass_log("FOO", 'F'));
-               assert(1 == __log_limiter_pass_log("BAR", 'F'));
-               assert(1 == __log_limiter_pass_log("BAR", 'E'));
+               assert(__log_limiter_pass_log("FOO", 'F').decision == DECISION_ALLOWED);
+               assert(__log_limiter_pass_log("BAR", 'F').decision == DECISION_ALLOWED);
+               assert(__log_limiter_pass_log("BAR", 'E').decision == DECISION_ALLOWED);
        }
 
        fail_malloc = 0;
@@ -55,7 +55,7 @@ int main()
        assert(__log_limiter_create(&conf));
 
        fail_time = true;
-       assert(1 == __log_limiter_pass_log("FOO", 'E'));
+       assert(__log_limiter_pass_log("FOO", 'E').decision == DECISION_ALLOWED);
        fail_time = false;
 
        int rule_count = get_rulecount();

diff --git a/src/tests/limiter_pos.c b/src/tests/limiter_pos.c
index fda96df..26ed02c 100644 (file)
--- a/src/tests/limiter_pos.c
+++ b/src/tests/limiter_pos.c
@@ -29,26 +29,26 @@ int main()
        __log_limiter_update(&conf);
 
        for (int i = 0; i < 100; ++i) {
-               assert(0 == __log_limiter_pass_log("FOO", 'F'));
-               assert(1 == __log_limiter_pass_log("BAR", 'F'));
-               assert(0 == __log_limiter_pass_log("BAR", 'E'));
+               assert(__log_limiter_pass_log("FOO", 'F').decision == DECISION_DENIED);
+               assert(__log_limiter_pass_log("BAR", 'F').decision == DECISION_ALLOWED);
+               assert(__log_limiter_pass_log("BAR", 'E').decision == DECISION_DENIED);
        }
 
        __log_limiter_destroy();
        assert(__log_limiter_create(&conf));
 
        for (int i = 0; i < 7; ++i)
-               assert(1 == __log_limiter_pass_log("FOO", 'E'));
-       assert(-1 == __log_limiter_pass_log("FOO", 'E'));
+               assert(__log_limiter_pass_log("FOO", 'E').decision == DECISION_ALLOWED);
+       assert(__log_limiter_pass_log("FOO", 'E').decision == DECISION_TAG_LIMIT_EXCEEDED_MESSAGE);
        for (int i = 0; i < 2; ++i)
-               assert(0 == __log_limiter_pass_log("FOO", 'E'));
+               assert(__log_limiter_pass_log("FOO", 'E').decision == DECISION_DENIED);
 
        fail_time = false;
-       assert(0 == __log_limiter_pass_log("FOO", 'E'));
+       assert(__log_limiter_pass_log("FOO", 'E').decision == DECISION_DENIED);
        future_time = true;
-       assert(1 == __log_limiter_pass_log("FOO", 'E'));
+       assert(__log_limiter_pass_log("FOO", 'E').decision == DECISION_ALLOWED);
        future_time = false;
-       assert(1 == __log_limiter_pass_log("FOO", 'E'));
+       assert(__log_limiter_pass_log("FOO", 'E').decision == DECISION_ALLOWED);
 
        int rulecount = get_rulecount();
        assert(rulecount == 4);
@@ -131,7 +131,7 @@ int main()
        __log_limiter_destroy();
        __log_limiter_initialize(&r1);
 
-       assert(1 == __log_limiter_pass_log("FOO", 'E'));
+       assert(__log_limiter_pass_log("FOO", 'E').decision == DECISION_ALLOWED);
 
        /* Searching a hashmap kinda doesn't work if you
         * fake hashes (esp. identical ones) so this just
@@ -180,8 +180,8 @@ int main()
        };
        __log_limiter_destroy();
        __log_limiter_initialize(&block_all);
-       assert(__log_limiter_pass_log("tag", 'W') == 0);
-       assert(__log_limiter_pass_log("", 'W') == 1);
+       assert(__log_limiter_pass_log("tag", 'W').decision == DECISION_DENIED);
+       assert(__log_limiter_pass_log("", 'W').decision == DECISION_ALLOWED);
 
        log_config_free(&conf);
        __log_limiter_destroy();

diff --git a/src/tests/pid_limiter.c b/src/tests/pid_limiter.c
index 69d457e..8d5a52f 100644 (file)
--- a/src/tests/pid_limiter.c
+++ b/src/tests/pid_limiter.c
@@ -35,8 +35,8 @@ int main()
        assert(__log_limiter_create(&conf));
        __log_limiter_update(&conf);
 
-#define PASS  assert(1 == __log_limiter_pass_log("FOO", 'W'))
-#define BLOCK assert(0 == __log_limiter_pass_log("FOO", 'W'))
+#define PASS  assert(__log_limiter_pass_log("FOO", 'W').decision == DECISION_ALLOWED)
+#define BLOCK assert(__log_limiter_pass_log("FOO", 'W').decision == DECISION_DENIED)
 
        pid_ret = 77;
        for (int i =   0; i <   7; ++i) PASS;