Re: [SSSD] [PATCH] TESTS: Removing part of responder_cache_req-tests
On 08/21/2015 04:55 PM, Petr Cech wrote: On 08/21/2015 02:35 PM, Michal Židek wrote: Hi, some of the tests you deleted are valid and should not be deleted. Only those tests that rely on time(NULL) being the same as the time of request creation are invalid. All those that test old entries or nonexistent entries are OK. See comments inline. I agree. Those tests have another logic. So I returned them back. Petr ACK. CI results: http://sssd-ci.duckdns.org/logs/job/23/75/summary.html Michal ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] [HBAC]: Better libhbac debuging
On 07/24/2015 06:20 PM, Petr Cech wrote: From 2fcf13ef59f00b460afa77b27ef6cc2789b06393 Mon Sep 17 00:00:00 2001 From: Petr Cechpc...@redhat.com Date: Fri, 24 Jul 2015 10:56:49 -0400 Subject: [PATCH] [HBAC]: Better libhbac debuging s/debuging/debugging Added support for logging via external log function. Log provides information about rules evaluating (HBAC_DBG_INFO level) and additionally can describe rules (HBAC_DBG_TRACE level). Resolves: https://fedorahosted.org/sssd/ticket/2703 --- src/providers/ipa/hbac_evaluator.c | 146 + src/providers/ipa/ipa_access.c | 45 src/providers/ipa/ipa_hbac.exports | 3 +- src/providers/ipa/ipa_hbac.h | 23 ++ 4 files changed, 216 insertions(+), 1 deletion(-) diff --git a/src/providers/ipa/hbac_evaluator.c b/src/providers/ipa/hbac_evaluator.c index f40f9e0a7f16f5e012079c637b89c8e49ec5d15b..66d3512937702b5955f333c0c837807ee9e13deb 100644 --- a/src/providers/ipa/hbac_evaluator.c +++ b/src/providers/ipa/hbac_evaluator.c @@ -24,6 +24,8 @@ */ #include stdlib.h +#include stdio.h +#include stdarg.h #include string.h #include errno.h #include providers/ipa/ipa_hbac.h @@ -38,6 +40,41 @@ typedef int errno_t; #define EOK 0 #endif +/* HBAC logging system */ + +/* static pointer to external logging function */ +static void (*hbac_debug_fn)(const char *file, int line, enum hbac_debug_level, + const char *format, ...) = NULL; Do you think that introducing a new type using typedef for this type of callback would be more readable? + +/* setup function for external logging function */ +void hbac_enable_debug(void (*external_debug_fn)(const char *file, int line, + enum hbac_debug_level, const char *format, ...)) +{ +hbac_debug_fn = external_debug_fn; +} + +/* debug macro */ +#define HBAC_DEBUG(level, format, ...) do { \ +if (hbac_debug_fn != NULL) { \ +hbac_debug_fn(__FILE__, __LINE__, level, format, ##__VA_ARGS__); \ +} \ +} while (0) IMO macro should be defined after includes and before function definitions, but I haven't check if we are 100 % consistent about this in SSSD. + +/* auxiliary function for hbac_request_element logging */ +static void hbac_request_element_debug_print(struct hbac_request_element *el, +const char *label); bad indentation + +/* auxiliary function for hbac_eval_req logging */ +static void hbac_req_debug_print(struct hbac_eval_req *req); + +/* auxiliary function for hbac_rule_element logging */ +static void hbac_rule_element_debug_print(struct hbac_rule_element *el, + const char *label); + +/* auxiliary function for hbac_rule logging */ +static void hbac_rule_debug_print(struct hbac_rule *rule); + + /* Placeholder structure for future HBAC time-based * evaluation rules */ @@ -110,6 +147,9 @@ enum hbac_eval_result hbac_evaluate(struct hbac_rule **rules, struct hbac_eval_req *hbac_req, struct hbac_info **info) { +HBAC_DEBUG(HBAC_DBG_INFO, [ hbac_evaluate()); +hbac_req_debug_print(hbac_req); + We generally do not add any code before variable definitions, I understand that logging is kinda special, but I would prefer to add it after the definitions, do you agree? enum hbac_error_code ret; enum hbac_eval_result result = HBAC_EVAL_DENY; enum hbac_eval_result_int intermediate_result; @@ -117,6 +157,7 @@ enum hbac_eval_result hbac_evaluate(struct hbac_rule **rules, if (info) { *info = malloc(sizeof(struct hbac_info)); if (!*info) { +HBAC_DEBUG(HBAC_DBG_ERROR, Out of memory.); return HBAC_EVAL_OOM; } (*info)-code = HBAC_ERROR_UNKNOWN; @@ -125,20 +166,25 @@ enum hbac_eval_result hbac_evaluate(struct hbac_rule **rules, uint32_t i; for (i = 0; rules[i]; i++) { +hbac_rule_debug_print(rules[i]); intermediate_result = hbac_evaluate_rule(rules[i], hbac_req, ret); if (intermediate_result == HBAC_EVAL_UNMATCHED) { /* This rule did not match at all. Skip it */ +HBAC_DEBUG(HBAC_DBG_INFO, DISALLOWED by rule [%s]., + rules[i]-name); continue; } else if (intermediate_result == HBAC_EVAL_MATCHED) { /* This request matched an ALLOW rule * Set the result to ALLOW but continue checking * the other rules in case a DENY rule trumps it. */ +HBAC_DEBUG(HBAC_DBG_INFO, ALLOWED by rule [%s]., rules[i]-name); result = HBAC_EVAL_ALLOW; if (info) { (*info)-code = HBAC_SUCCESS; (*info)-rule_name = strdup(rules[i]-name); if (!(*info)-rule_name) { +HBAC_DEBUG(HBAC_DBG_ERROR, Out of memory.);
Re: [SSSD] Embedding Lua into SSSD
On 08/21/2015 07:01 PM, Nikolai Kondrashov wrote: Hi everyone, I might be in a strange and careless mood today, but here is something I wanted to suggest since the time I saw the amount of logic that goes into SSSD and is implemented in C. What if we implement some of the complicated logic inside SSSD in Lua [1]? Lua is a language specifically designed to be embedded. It grew into popularity after it was embedded into World Of Warcraft. It was also embedded and is still being embedded into a bunch of other games. However, games is far from the only application for Lua. Lua is a simple and lightweight language, yet with lots of flexibility - grasping the essence of the whole reference manual [2] takes one evening and it's an entertaining read. The VM is small and fast. In fact some people use it on microcontrollers [3]. I personally embedded it into two C-based projects, one of which was already quite big, and another was a rewrite of a smallish, but complicated C project. In both cases, where before things were tedious and hard, they have become easy and fun. The first project was on an embedded platform. I also had some other, but smaller stuff done with it. Generally, Lua gets put into the middle with outside interfaces and performance-critical parts implemented in C. With a very simple interface to and from C one can put boundaries wherever necessary. Yes, the interface is simpler than Python's. The process is this: you implement some C libraries (shared/static) which are loadable from Lua (Lua-C interface) and then create a VM or several, load some Lua code and call into it (C-Lua interface). Some of the benefits are: Clearer logic (higher level language) Much less memory leaks and very little memory management (mostly in Lua-C interface). Faster development cycle (no compilation, edit and retry on a live daemon) Potentially easy plugin interface (yay for admin scripts!) Some of the drawbacks are: More difficult to do low level stuff (so we just don't do that) Lower performance than C (still good enough for games, and we can have the critical parts in C easily) Needs learning and development investment. Regarding the development investment, due to low cost of C/Lua interface implementation, it is practical to replace the relevant C code piece-by-piece, provided pieces are not too small (say, 2KLOC and up). Lastly, don't take this too seriously. I realise that this is disruptive and hard. However if you ever feel like SSSD has grown too big and complex, ping me, or just try Lua yourself :) Nick I may be too conservative here, but I believe SSSD should be written in language with static type-checking at least as strong as C. Michal ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] DATA_PROVIDER: BE_REQ as string in log message
[snip] On 08/21/2015 05:10 PM, Pavel Reichl wrote: ACK Just adding link to passed ci (with exception of test test_ldap_id_cleanup which is false positive (and already fixed and acked on list)): http://sssd-ci.duckdns.org/logs/job/23/73/summary.html ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
[SSSD] [PATCH] LDAP: end on ENOMEM
Hello, please see trivial patch fixing coverity warning. Thanks! From 69c20ca306827a558d6be015d0d060f9f46423f8 Mon Sep 17 00:00:00 2001 From: Pavel Reichl prei...@redhat.com Date: Mon, 24 Aug 2015 07:05:00 -0400 Subject: [PATCH] LDAP: end on ENOMEM --- src/providers/ldap/ldap_id_cleanup.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/providers/ldap/ldap_id_cleanup.c b/src/providers/ldap/ldap_id_cleanup.c index 0a2b23297f5feaee30b75b31a9b2fd9a0a2a6f23..0cbcf04791abe701d335523302e5c1f1afacc716 100644 --- a/src/providers/ldap/ldap_id_cleanup.c +++ b/src/providers/ldap/ldap_id_cleanup.c @@ -466,6 +466,7 @@ get_group_dn_with_filter_sanitized_name(TALLOC_CTX *mem_ctx, tmp_ctx = talloc_new(NULL); if (tmp_ctx == NULL) { ret = ENOMEM; +goto done; } /* sanitize group name */ -- 2.4.3 ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] Embedding Lua into SSSD
On 08/21/2015 07:01 PM, Nikolai Kondrashov wrote: Hi everyone, I might be in a strange and careless mood today, but here is something I wanted to suggest since the time I saw the amount of logic that goes into SSSD and is implemented in C. What if we implement some of the complicated logic inside SSSD in Lua [1]? Lua is a language specifically designed to be embedded. It grew into popularity after it was embedded into World Of Warcraft. It was also embedded and is still being embedded into a bunch of other games. However, games is far from the only application for Lua. Lua is a simple and lightweight language, yet with lots of flexibility - grasping the essence of the whole reference manual [2] takes one evening and it's an entertaining read. The VM is small and fast. In fact some people use it on microcontrollers [3]. I personally embedded it into two C-based projects, one of which was already quite big, and another was a rewrite of a smallish, but complicated C project. In both cases, where before things were tedious and hard, they have become easy and fun. The first project was on an embedded platform. I also had some other, but smaller stuff done with it. Generally, Lua gets put into the middle with outside interfaces and performance-critical parts implemented in C. With a very simple interface to and from C one can put boundaries wherever necessary. Yes, the interface is simpler than Python's. The process is this: you implement some C libraries (shared/static) which are loadable from Lua (Lua-C interface) and then create a VM or several, load some Lua code and call into it (C-Lua interface). Some of the benefits are: Clearer logic (higher level language) Much less memory leaks and very little memory management (mostly in Lua-C interface). Faster development cycle (no compilation, edit and retry on a live daemon) Potentially easy plugin interface (yay for admin scripts!) Some of the drawbacks are: More difficult to do low level stuff (so we just don't do that) Lower performance than C (still good enough for games, and we can have the critical parts in C easily) Needs learning and development investment. Regarding the development investment, due to low cost of C/Lua interface implementation, it is practical to replace the relevant C code piece-by-piece, provided pieces are not too small (say, 2KLOC and up). Lastly, don't take this too seriously. I realise that this is disruptive and hard. However if you ever feel like SSSD has grown too big and complex, ping me, or just try Lua yourself :) Nick [1] http://www.lua.org/ [2] http://www.lua.org/manual/5.3/manual.html [3] http://www.eluaproject.net/ Hi, can you tell us what features of Lua do you like and might help simplify SSSD? I don't know the language, just fast scrolled over the manual and seen few examples. It seems to be similar to javascript. ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] LDAP: end on ENOMEM
On 08/24/2015 01:11 PM, Pavel Reichl wrote: Hello, please see trivial patch fixing coverity warning. Thanks! 0001-LDAP-end-on-ENOMEM.patch From 69c20ca306827a558d6be015d0d060f9f46423f8 Mon Sep 17 00:00:00 2001 From: Pavel Reichlprei...@redhat.com Date: Mon, 24 Aug 2015 07:05:00 -0400 Subject: [PATCH] LDAP: end on ENOMEM --- src/providers/ldap/ldap_id_cleanup.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/providers/ldap/ldap_id_cleanup.c b/src/providers/ldap/ldap_id_cleanup.c index 0a2b23297f5feaee30b75b31a9b2fd9a0a2a6f23..0cbcf04791abe701d335523302e5c1f1afacc716 100644 --- a/src/providers/ldap/ldap_id_cleanup.c +++ b/src/providers/ldap/ldap_id_cleanup.c @@ -466,6 +466,7 @@ get_group_dn_with_filter_sanitized_name(TALLOC_CTX *mem_ctx, tmp_ctx = talloc_new(NULL); if (tmp_ctx == NULL) { ret = ENOMEM; +goto done; We usually use return ENOMEM in this case. But this works as well and it may be even clearer. Ack. } /* sanitize group name */ -- 2.4.3 ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel