On (12/07/16 13:57), Fabiano FidĂȘncio wrote:
Comments are in-line.
The test directory was not removed (tp_test_utils-test_utils)
because it contain the snippet for krb5_libdefaults.
---
src/tests/cmocka/test_utils.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/src/tests/cmocka/test_utils.c b/src/tests/cmocka/test_utils.c
index fd20990ce7ac632b3b62bf84a20cc75a5ec0e968..b08b19708bb59a076a79805fa37a15924152b8e2
100644
--- a/src/tests/cmocka/test_utils.c
+++ b/src/tests/cmocka/test_utils.c
@@ -1252,6 +1252,7 @@ void test_sss_write_krb5_conf_snippet(void **state)
char *cwd;
char *path;
char *file;
+ char *file_krb5_libdefaults;
ret = sss_write_krb5_conf_snippet(NULL, false);
assert_int_equal(ret, EINVAL);
@@ -1274,6 +1275,10 @@ void test_sss_write_krb5_conf_snippet(void **state)
ret = asprintf(&file, "%s/%s/localauth_plugin", cwd, TESTS_PATH);
assert_true(ret > 0);
I know it's not part of your change and I know it's just in the test
suite, but by doing the assert here you're leaking the path.
+ ret = asprintf(&file_krb5_libdefaults,
+ "%s/%s/krb5_libdefaults", cwd, TESTS_PATH);
+ assert_true(ret > 0);
+
Leaking path and file here.
ret = sss_write_krb5_conf_snippet(path, true);
assert_int_equal(ret, EOK);
@@ -1286,7 +1291,11 @@ void test_sss_write_krb5_conf_snippet(void **state)
assert_int_equal(ret, EOK);
#endif
+ ret = unlink(file_krb5_libdefaults);
+ assert_int_equal(ret, EOK);
Leaking the path, file and file_krb6_libdefaults here..
We do not leak memory if test passes
and we have a requirement that unit test have to pass with each commit.
We could write a setup and teardown to get rid of potential leaks
in case of failure. But IMHO it does not worth it.
LS