Subject: Re: [PATCH v2 7/7] t/helper/simple-ipc: convert test-simple-ipc to use start_bg_command

From: Junio C Hamano <[email-masked]>
Date: Thu, 23 Sep 2021 11:37:01 -0700

Ævar Arnfjörð Bjarmason <[email-masked]> writes:

> I played with the diff below on top of this, I can't remember if it was
> noted already, but the way you declare function ptrs and use them isn't
> the usual style:
>
> -- >8 --
> diff --git a/run-command.c b/run-command.c
> index 76bbef9d96d..5c831545201 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -1903,7 +1903,7 @@ void prepare_other_repo_env(struct strvec *env_array, const char *new_git_dir)
>  }
>  
>  enum start_bg_result start_bg_command(struct child_process *cmd,
> -				      start_bg_wait_cb *wait_cb,
> +				      start_bg_wait_cb wait_cb,
> -typedef int(start_bg_wait_cb)(const struct child_process *cmd, void *cb_data);
> +typedef int (*start_bg_wait_cb)(const struct child_process *cmd, void *cb_data);

I have no comment on the "default" thing, but I agree that the
preimage does look _unusual_ in our codebase.  You cannot even
declare a "variable" of that type with the typedef, i.e.

	start_bg_wait_cb an_instance_of_that;

If there is a good reason behind choosing the unusual "the type of
the function is...", that is OK, but otherwise...