From e9b6f1f8db8d7ad1842b43126b9352d1c1d43fc7 Mon Sep 17 00:00:00 2001 From: Mateusz Majewski Date: Wed, 19 Aug 2020 09:55:06 +0200 Subject: [PATCH] Improve __log_limiter_pass_log interface Change-Id: Ie47e84c25a5ed615e52a69e51bde2018d987f06d --- include/loglimiter.h | 13 ++++++++++++- src/libdlog/log.c | 29 +++++++++++++++++++---------- src/shared/loglimiter.c | 34 ++++++++++++++++++---------------- src/tests/libdlog_base_neg.c | 4 ++-- src/tests/libdlog_base_pos.c | 8 ++++---- src/tests/libdlog_base_wrap.c | 9 +++++---- src/tests/libdlog_prio_filter_pos.c | 7 ++++--- src/tests/limiter_neg.c | 14 +++++++------- src/tests/limiter_pos.c | 24 ++++++++++++------------ src/tests/pid_limiter.c | 4 ++-- 10 files changed, 85 insertions(+), 61 deletions(-) diff --git a/include/loglimiter.h b/include/loglimiter.h index 7f58582..c3ae5c0 100644 --- 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 --- 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 --- 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 --- 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 --- 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 --- a/src/tests/libdlog_base_wrap.c +++ b/src/tests/libdlog_base_wrap.c @@ -24,8 +24,9 @@ #include #include #include +#include -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 --- a/src/tests/libdlog_prio_filter_pos.c +++ b/src/tests/libdlog_prio_filter_pos.c @@ -22,6 +22,7 @@ #include #include #include +#include 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 --- 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 --- 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 --- 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; -- 2.7.4