On Fri, Feb 19, 2016 at 02:16:15PM +0100, Pavel Březina wrote: > Fixes: > https://fedorahosted.org/sssd/ticket/2869 > https://fedorahosted.org/sssd/ticket/2848
> From 681a8ff812f5af8adaed92bf1fd0248be7c2bda0 Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]> > Date: Fri, 12 Feb 2016 11:58:32 +0100 > Subject: [PATCH 01/12] cache_req: bring together search parameters ACK I think this would play nicer with the sysdb refactoring, too. > From 0b5f044357a4f5d0b8d8b35c4ab2c2d0a7495bcf Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]> > Date: Fri, 12 Feb 2016 12:12:12 +0100 > Subject: [PATCH 02/12] cache_req: fix typo in debug message ACK > From 457757b2f7fd4c2dbd061aa779dc0f576eba0975 Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]> > Date: Fri, 12 Feb 2016 12:20:04 +0100 > Subject: [PATCH 03/12] cache_req: break cache_req_input_create into more > functions ACK to the changes, but I wonder if it's correct to say "Bug" if the input ID is 0, do you think an upper layer should check for that instead and reject the zero ID with a more graceful message? Also please remember we'll support the root user one day.. > From 87972b1c7b7d74b052953e425cb532801bd51526 Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]> > Date: Fri, 12 Feb 2016 12:26:16 +0100 > Subject: [PATCH 04/12] cache_req: rename debug_fqn to debugobj ACK > From 429023b7fe38b6a46d5ecaf4c345591e173eb87d Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]> > Date: Fri, 12 Feb 2016 13:12:34 +0100 > Subject: [PATCH 05/12] cache_req: improve debugging ACK, I like this. > From c38cae6eef560028c54ad135b4f3a4bead30263f Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]> > Date: Tue, 16 Feb 2016 13:14:00 +0100 > Subject: [PATCH 06/12] cache_req tests: remove unused users and groups ACK > From f61d0192b8254247802167ea385b52f65d4e175d Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]> > Date: Thu, 18 Feb 2016 14:25:18 +0100 > Subject: [PATCH 07/12] sysdb: reset ldb errors > > After ldb connect ldb context contains the following error: > "NULL Base DN invalid for a base search" > > This comes from internal ldb function ldb_set_default_dns() which > runs base search on NULL dn to discover records similar to what > rootDSE provides. However, tdb backend considers this an error > and sets the message above. > > This may break memory leak checks in tests when we do push/pop on > test_ctx which is a indirect parent of ldb_context. The error message > is allocated when push is called but it is freed by other ldb queries > and therefore not preset during the push phase and thus the leak check > fails. I know this fixed an error for you, but I wonder if it wouldn't be better to work around the issue or use this only in the test itself. The reason being that the function is only part of ldb_modules.h so it feels a bit hacky to use it outside an ldb module.. > From 7b86bf36da117f86c4390d3b56e2f6065754d16b Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]> > Date: Wed, 17 Feb 2016 10:07:18 +0100 > Subject: [PATCH 08/12] cache_req tests: use leak check in test fixtures > > To ensure no memory is leak on long living context such as rctx. > > Resolves: > https://fedorahosted.org/sssd/ticket/2869 ACK, but perhaps we'll have to amend this patch if we decide to drop the previous patch. > From 2375eb0a03c4d5fb094670fcd0a2e178f9a503c4 Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]> > Date: Fri, 19 Feb 2016 13:04:04 +0100 > Subject: [PATCH 09/12] cache_req tests: improve user and group creation > > --- > src/tests/cmocka/test_responder_cache_req.c | 257 > +++++++++++++--------------- There are some compilation warnings here: CC src/tests/cmocka/responder_cache_req_tests-test_responder_cache_req.o /home/remote/jhrozek/devel/sssd/src/tests/cmocka/test_responder_cache_req.c: In function ‘__wrap_sss_dp_get_account_send’: /home/remote/jhrozek/devel/sssd/src/tests/cmocka/test_responder_cache_req.c:324:13: warning: unused variable ‘ret’ [-Wunused-variable] errno_t ret; ^ In file included from /home/remote/jhrozek/devel/sssd/src/tests/cmocka/test_responder_cache_req.c:21:0: /home/remote/jhrozek/devel/sssd/src/tests/cmocka/test_responder_cache_req.c: In function ‘test_user_by_name_multiple_domains_parse’: /home/remote/jhrozek/devel/sssd/src/tests/cmocka/test_responder_cache_req.c:539:17: warning: passing argument 1 of ‘_talloc_free’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] talloc_free(fqn); ^ /usr/include/talloc.h:229:5: note: expected ‘void *’ but argument is of type ‘const char *’ int _talloc_free(void *ptr, const char *location); ^ /home/remote/jhrozek/devel/sssd/src/tests/cmocka/test_responder_cache_req.c: In function ‘test_group_by_name_multiple_domains_parse’: /home/remote/jhrozek/devel/sssd/src/tests/cmocka/test_responder_cache_req.c:1039:17: warning: passing argument 1 of ‘_talloc_free’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] talloc_free(fqn); ^ /usr/include/talloc.h:229:5: note: expected ‘void *’ but argument is of type ‘const char *’ int _talloc_free(void *ptr, const char *location); ^ CCLD responder_cache_req-tests But the general intent looks good > From bd5957798c785ea0668fe3b74638e07dd0cfbe03 Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]> > Date: Mon, 15 Feb 2016 11:57:02 +0100 > Subject: [PATCH 10/12] utils: return const char ** from dup_string_list I think the reason we had char* and not const char * is that you don't have to discard_const if you talloc_free() the result directly (as opposed to freeing the parent). > From b90e9bf4ec7b7377cbfd8e12801d5c186f3c292d Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]> > Date: Fri, 12 Feb 2016 14:28:33 +0100 > Subject: [PATCH 11/12] cache_req: add SID lookups > > Resolves: > https://fedorahosted.org/sssd/ticket/2848 Just one question.. > input = cache_req_input_create(mem_ctx, rctx, CACHE_REQ_USER_BY_NAME, > - name, 0, NULL); > + name, 0, NULL, NULL, NULL); Seeing the number of parametrs increase, what do you think about merging them to an union and passing a pointer to a struct as an input instead? Each CACHE_REQ_* maps to exactly one input type after all.. > From 4d4b87e40b25d1528aac2222d6a7444a32eae515 Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]> > Date: Fri, 19 Feb 2016 14:09:26 +0100 > Subject: [PATCH 12/12] cache_req test: add lookup by sid LGTM, thanks for including the tests. Coverity returned clean. _______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/admin/lists/[email protected]
