Re: [PATCH v2 7/7] t/helper/simple-ipc: convert test-simple-ipc to use start_bg_command
From: Junio C Hamano <hidden>
Date: 2021-09-23 18:37:07
Ævar Arnfjörð Bjarmason [off-list ref] writes:
quoted hunk
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...