Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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...