Re: [PATCH v5] selftests/mm: add process_madvise() tests

On Mon, Jul 14, 2025 at 08:25:33PM +0800, wang lian wrote:

+TEST_F(process_madvise, remote_collapse) +{

• self->child_pid = fork();
• ASSERT_NE(self->child_pid, -1);

• ret = read(pipe_info[0], &info, sizeof(info));
• if (ret <= 0) {

• waitpid(self->child_pid, NULL, 0);
  

• SKIP(return, "Failed to read child info from pipe.\n");
  

• }
• ASSERT_EQ(ret, sizeof(info));

+cleanup:

• /* Cleanup */
• kill(self->child_pid, SIGKILL);
• waitpid(self->child_pid, NULL, 0);
• if (pidfd >= 0)

• close(pidfd);
  

The cleanup here won't get run if we skip or assert, skipping will return immediately (you could replace the return with a 'goto cleanup') and the asserts will exit the test immediately. This will mean we leak the child. This isn't an issue for things that are memory mapped or tracked with file descriptors, the harness will for a new child for each test so anything that's cleaned up with the process will be handled, but that doesn't apply to child processes.

I think doing the child setup in a fixture should DTRT but I haven't gone through in full detail to verify that this is the case.