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

Reply via email to