On Tue, Jan 28, 2014 at 04:50:44AM +0545, Pallavi Jha wrote:
Hi,
I have tried to make all the required changes as mentioned in the above
mail. The patch for the same is attached along with this mail. While
writing test "test_sss_ncache_prepopulate" I came across a bug, as
mentioned below. Let me know if I am going wrong anywhere:
*In negcache.c*
line 676 ret = sss_parse_name_for_domains(tmpctx, domain_list,
rctx->default_domain,
filter_list[i],
&domainname, &name);
line 676 calls sss_parse_name() which in return executes line no 395 in
src/util/usertools.c which is ret = sss_parse_name(tmp_ctx, dom->names,
orig, &dmatch, &nmatch);
Now look at 2nd argument of sss_parse_namae it is of type char * but if you
check sss_parse_name defn at line no 304:
int sss_parse_name(TALLOC_CTX *memctx,
struct sss_names_ctx *snctx,
const char *orig, char **domain, char **name)
so line no 308 : pcre *re = snctx->re; is the reason for
crash(segmentation fault).
Please review the patch.
Hi,
you just need to initialize the snctx, that can be done with a similar
call in the prepopulate test:
ret = sss_names_init(ts, tc->confdb, TEST_DOM_NAME, &dom->names);
assert_int_equal(ret, EOK);
See some minor comments about the patch inline.
+#define SHORTSPAN 2
I know I suggested the sleep of 2 seconds, but when I ran the test now
it looked like too much, the test was paused for too long. Maybe just 1s
would be better, I tried running the test in a loop and didn't find any
bugs the short span would cause
[snip]
+static void test_sss_ncache_prepopulate(void **state)
+{
+ int ret;
+ struct test_state *ts;
+ struct tevent_context *ev;
+ struct sss_nc_ctx *ncache;
+ struct sss_test_ctx *tc;
+ struct sss_domain_info *dom;
+
+ ts = talloc_get_type_abort(*state, struct test_state);
+
+ ev = tevent_context_init(ts);
You should check if ev is not NULL.
+
+ dom = talloc(ts, struct sss_domain_info);
Same here for dom.
+ dom->name = NAME;
It's a very good idea to define the NAME, but with my (admittedly
strict) CFLAGS, I'm getting a warning because the domain name is "char
*" but the string constant in NAME is "const char *".
You can get around this warning by using a special 'discard_const' macro
like this:
dom->name = discard_const(NAME);
+
+ ts->nctx = mock_nctx(ts);
+ assert_non_null(ts->nctx);
+
+ struct sss_test_conf_param params[] = {
+ { "filter_users", "testuser1" },
+ { "filter_groups", "testgroup1" },
+ { NULL, NULL },
+ };
I prefer if variable definitions are kept at the function beginning and
not interleave with code.
+
+ tc = create_dom_test_ctx(ts, TESTS_PATH, TEST_CONF_DB,
+ TEST_SYSDB_FILE, TEST_DOM_NAME,
+ TEST_ID_PROVIDER, params);
+ assert_non_null(tc);
+
+ ncache = ts->ctx;
+ ts->rctx = mock_rctx(ts, ev, dom, ts->nctx);
+ assert_non_null(ts->rctx);
+ assert_non_null(tc->confdb);
Why do you check confdb here separately? I think create_dom_test_ctx can
be trusted to create it on its own.
+
+ ret = sss_ncache_prepopulate(ncache, tc->confdb, ts->rctx);
+ assert_int_equal(ret, EOK);
I assume the continuation of this test would be to check that testuser1
and testgroup1 will be present in the cache even after the short sleep?
+}
The test is looking good, nice work!