On Tue, Jul 19, 2022 at 10:27:36AM -0700, Yosry Ahmed wrote:
Nit: keep the cleanup_* naming for labels to make it obvious and to be consistent with the rest of the file (e.g. cleanup_free, cleanup_memcg, cleanup_file/cleanup_all). See test_memcg_subtree_control().
I would honestly have one label to cleanup the memcg. Calling cg_destroy() on a non-existent memcg should be fine. rmdir() will just fail silently. All other tests do this and it's easier to read when we have fewer return paths. My advice would be cleanup_file and cleanup_memcg labels.
One error label handling is very bug prone. You always end up freeing things which have not been initialized/allocated. Or dereferencing pointers which are NULL. Or, since most kernel functions clean up after themselves, you end up double freeing things.
regards, dan carpenter