On Wed, Oct 13, 2021 at 2:54 PM Christian Göttsche <cgzones@xxxxxxxxxxxxxx> wrote: > > Free all memory in test cases, reported by LeakSanitizer. > > Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx> > --- > libsemanage/tests/test_bool.c | 33 +++++++++++++++++++++++++++++- > libsemanage/tests/test_fcontext.c | 31 +++++++++++++++++++++++++++- > libsemanage/tests/test_ibendport.c | 13 ++++++++++++ > libsemanage/tests/test_iface.c | 24 ++++++++++++++++++++++ > libsemanage/tests/test_node.c | 29 ++++++++++++++++++++++++++ > libsemanage/tests/test_other.c | 6 ++++++ > libsemanage/tests/test_port.c | 24 ++++++++++++++++++++++ > libsemanage/tests/test_user.c | 17 +++++++++++++++ > libsemanage/tests/utilities.c | 5 ++++- > libsemanage/tests/utilities.h | 2 ++ > 10 files changed, 181 insertions(+), 3 deletions(-) The diff was very large, so I posted 3 comments on GitHub: https://github.com/SELinuxProject/selinux/pull/321/commits/322243ee7d34f1b3e23c04c75a6b5c4f597092f7#r730306247 and https://github.com/SELinuxProject/selinux/pull/321/commits/322243ee7d34f1b3e23c04c75a6b5c4f597092f7#r730306949 and https://github.com/SELinuxProject/selinux/pull/321/commits/322243ee7d34f1b3e23c04c75a6b5c4f597092f7#r730307266 , about the same pattern which causes (in my humble opinion) issues. The pattern is in: CU_ASSERT(semanage_ibendport_query_local(sh, key, &ibendport_local) >= 0); CU_ASSERT_PTR_NOT_NULL_FATAL(ibendport_local); CU_ASSERT(semanage_ibendport_del_local(sh, key) >= 0); CU_ASSERT(semanage_ibendport_query_local(sh, key, &ibendport_local) < 0); /* cleanup */ semanage_ibendport_key_free(key); semanage_ibendport_free(ibendport_local); The last ..._free occurs after a second call to semanage_ibendport_query_local. It seems more appropriate to free the memory right after CU_ASSERT_PTR_NOT_NULL_FATAL(ibendport_local), before the second semanage_ibendport_query_local. So if an error happens, there is no memory issue too. This pattern is similar when testing boolean, fiface, node, port, user and context objects. What do you think? Thanks, Nicolas
