Re: [PATCH v4 12/12] t0052: add simple-ipc tests and t/helper/test-simple-ipc tool
- From
Jeff King <peff@peff.net>
- Date
- Mar 2, 2021, 09:44 UTC
- Message-ID
- <YD4JAvK0epzm9b2y@coredump.intra.peff.net>
- In-Reply-To
- <09568a6500dde4a592a994b661a7beec23af32b4.1613598529.git.gitgitgadget@gmail.com>
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):
Show 29 quoted lines
> +#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:
Show 36 quoted lines
> +#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;
> +}
> +#endifWe 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