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 _______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/admin/lists/[email protected]
