[PATCH v5 3/3] Add tests for the -l/--syscall-limit option

Dmitry V. Levin ldv at strace.io
Sat Mar 25 19:58:02 UTC 2023


On Mon, Mar 20, 2023 at 11:10:58AM +0530, Sahil Siddiq wrote:
[...]
> +static int
> +syscall_limit__status(void)
> +{
> +	int pid = getpid();
> +	long ret = chdir("invalid.dir");
> +
> +	if (ret == -1) {
> +		printf("%-5u chdir(\"invalid.dir\") = %s\n", pid, sprintrc(ret));
> +		fflush(stdout);
> +	}
> +
> +	pid_t child = fork();
> +	if (child < 0)
> +		perror_msg_and_fail("fork");
> +	else if (child == 0) {
> +		int pid = getpid();
> +		long ret = chdir("invalid.dir");
> +
> +		if (ret == -1) {
> +			printf("%-5u chdir(\"invalid.dir\") = %s\n", pid, sprintrc(ret));
> +			fflush(stdout);
> +		}
> +
> +		char buf[256];
> +
> +		if (getcwd(buf, 1) == NULL)
> +			printf("%-5u getcwd(%p, 1) = %s\n", pid, buf, sprintrc(-1));
> +		fflush(stdout);

I suggest using something more reliable instead of getcwd,
e.g. fchdir(-1) or rmdir("invalid.dir").

> +		struct stat statbuf;
> +		stat("invalid.file", &statbuf);
> +
> +		return 0;
> +	}
> +	int status;
> +	while ((waitpid(child, &status, 0)) != child) {
> +		if (errno == EINTR)
> +			continue;
> +		perror_msg_and_fail("waitpid: %d", child);
> +	}
> +
> +	chdir("invalid.dir");

Compiler is unhappy about this chdir() invocation because the return value
is not used.

I suggest the following approach:

static const char invalid_path[] = "invalid.dir";

static void
chdir_invalid(void)
{
	if (chdir(invalid_path) == 0)
		error_msg_and_fail("chdir: %s", invalid_path);
}

> +	FILE *f = fopen("dummy.file", "w");
> +	fclose(f);
> +
> +	return WEXITSTATUS(status);
> +}

There is a problem with this approach: it's racy, because the expected
output is partially printed asynchronously.  For the same reason, the exit
status is ignored.  I suggest writing the exit status into a file, and in
the test script wait for this file, check the exit status, and then
compare the expected and the actual output.

> +int
> +main(void)
> +{
> +	return syscall_limit__status();
> +}

You could rename syscall_limit__status() to main() as well.


-- 
ldv


More information about the Strace-devel mailing list