On Mon, Dec 10, 2018 at 03:36:54PM +0900, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> writes: > > > The compiler reports this because show_gitcomp() never actually > > returns a value: > > > > "parse-options.c", line 520: warning: Function has no return > > statement : show_gitcomp > > > > The function calls exit() and will never return. Update and mark it > > NORETURN. > > Yuck. This should do for now, but I am not impressed by the choice > to hook show_gitcomp() call into parse_options_step(), which forces > such an abnormal exit deeper in the callchain [*1*]. For readers > (not compilers), it would help to have a comment at the caller that > says that show_gitcomp() never returns and exits. > > side note #1. Perhaps parse_options_start() would have been > a better place, instead of parse_options_step() that is > repeatedly called in a loop and itself has a loop. ANd > worse yet, the check is done inside the loop even though the > call is to be made only when the --git-completion-helper > option is given as the sole request. The reason it's in parse_options_step() is because -h is also handled in there. Although -h does not bury exit() deep in the call chain. So how about this as a replacement? -- 8< -- diff --git a/parse-options.c b/parse-options.c index 3b874a83a0..6932eaff61 100644 --- a/parse-options.c +++ b/parse-options.c @@ -516,7 +516,7 @@ static int show_gitcomp(struct parse_opt_ctx_t *ctx, show_negated_gitcomp(original_opts, -1); show_negated_gitcomp(original_opts, nr_noopts); fputc('\n', stdout); - exit(0); + return PARSE_OPT_COMPLETE; } static int usage_with_options_internal(struct parse_opt_ctx_t *, @@ -638,6 +638,8 @@ int parse_options(int argc, const char **argv, const char *prefix, case PARSE_OPT_HELP: case PARSE_OPT_ERROR: exit(129); + case PARSE_OPT_COMPLETE: + exit(0); case PARSE_OPT_NON_OPTION: case PARSE_OPT_DONE: break; diff --git a/parse-options.h b/parse-options.h index 6c4fe2016d..a650a7d220 100644 --- a/parse-options.h +++ b/parse-options.h @@ -208,6 +208,7 @@ extern int opterror(const struct option *opt, const char *reason, int flags); /*----- incremental advanced APIs -----*/ enum { + PARSE_OPT_COMPLETE = -2, PARSE_OPT_HELP = -1, PARSE_OPT_DONE, PARSE_OPT_NON_OPTION, -- 8< --