Re: [PATCH v4 12/12] t0052: add simple-ipc tests and t/helper/test-simple-ipc tool

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Feb 17, 2021 at 09:48:48PM +0000, Jeff Hostetler via GitGitGadget wrote:

> Create t/helper/test-simple-ipc test tool to exercise the "simple-ipc"
> functions.

BTW, one oddity I noticed in this (because of my -Wunused-parameters
branch):

> +#ifndef GIT_WINDOWS_NATIVE
> +/*
> + * This is adapted from `daemonize()`.  Use `fork()` to directly create and
> + * run the daemon in a child process.
> + */
> +static int spawn_server(const char *path,
> +			const struct ipc_server_opts *opts,
> +			pid_t *pid)
> +{
> +	*pid = fork();
> +
> +	switch (*pid) {
> +	case 0:
> +		if (setsid() == -1)
> +			error_errno(_("setsid failed"));
> +		close(0);
> +		close(1);
> +		close(2);
> +		sanitize_stdfds();
> +
> +		return ipc_server_run(path, opts, test_app_cb, (void*)&my_app_data);
> +
> +	case -1:
> +		return error_errno(_("could not spawn daemon in the background"));
> +
> +	default:
> +		return 0;
> +	}
> +}

In the non-Windows version, we spawn a server using the "path" parameter
we got from the caller.

But in the Windows version:

> +#else
> +/*
> + * Conceptually like `daemonize()` but different because Windows does not
> + * have `fork(2)`.  Spawn a normal Windows child process but without the
> + * limitations of `start_command()` and `finish_command()`.
> + */
> +static int spawn_server(const char *path,
> +			const struct ipc_server_opts *opts,
> +			pid_t *pid)
> +{
> +	char test_tool_exe[MAX_PATH];
> +	struct strvec args = STRVEC_INIT;
> +	int in, out;
> +
> +	GetModuleFileNameA(NULL, test_tool_exe, MAX_PATH);
> +
> +	in = open("/dev/null", O_RDONLY);
> +	out = open("/dev/null", O_WRONLY);
> +
> +	strvec_push(&args, test_tool_exe);
> +	strvec_push(&args, "simple-ipc");
> +	strvec_push(&args, "run-daemon");
> +	strvec_pushf(&args, "--threads=%d", opts->nr_threads);
> +
> +	*pid = mingw_spawnvpe(args.v[0], args.v, NULL, NULL, in, out, out);
> +	close(in);
> +	close(out);
> +
> +	strvec_clear(&args);
> +
> +	if (*pid < 0)
> +		return error(_("could not spawn daemon in the background"));
> +
> +	return 0;
> +}
> +#endif

We ignore the "path" parameter entirely. Should we be passing it along
as an option to the child process? I think it doesn't really matter at
this point because both the parent and child processes will use the
hard-coded string "ipc-test", but it seems like something the test
script might want to be able to specify.

-Peff



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux