On Mon, Dec 10, 2018 at 03:36:54PM +0900, Junio C Hamano wrote:
> Nguyễn Thái Ngọc Duy <[email protected]> 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< --
