Hi, > > - ret = start_command(&hook); > > - if (ret) { > > - warning("Could not spawn %s", argv[0]); > > - return ret; > > - } > > - ret = finish_command(&hook); > > - if (ret == -ERR_RUN_COMMAND_WAITPID_SIGNAL) > > - warning("%s exited due to uncaught signal", argv[0]); > > What are the side effects of replacing this with "ret = > run_command(&hook);"? This has to be discussed and defended in the commit > message. This is a very good point. The consequences are that two warnings are missing, but these are warnings that are useful enough to be included for all those hooks, imho. Thanks! > Lots of improvements possible (I agree; _after_ this patch): [...] > - use ALLOC_GROW instead of having a fixed size argv, Agreed. > - use an strbuf for the index file Is that useful in some way? Currently it would only adds code to generate strbufs at the caller side. And in the case the caller has a strbuf for the index file, it can simply use the .buf member. > - checking executability of argv[0] before filling argv, Agreed. Patch series v2 will follow. Thanks, Stephan -- Stephan Beyer <s-beyer@xxxxxxx>, PGP 0x6EDDD207FCC5040F -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html