On Thu, 29 Aug 2019 at 21:15, René Scharfe <l....@web.de> wrote:
>
> Announce that calling help_unknown_ref() exits the program.
>
> Signed-off-by: René Scharfe <l....@web.de>
> ---
> Patch generated with --function-context for easier review.
>
> help.c | 3 ++-
> help.h | 2 +-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/help.c b/help.c
> index 5261d83ecf..9ff2be6b18 100644
> --- a/help.c
> +++ b/help.c
> @@ -774,22 +774,23 @@ static struct string_list guess_refs(const char *ref)
> return similar_refs;
> }
>
> -void help_unknown_ref(const char *ref, const char *cmd, const char *error)
> +NORETURN void help_unknown_ref(const char *ref, const char *cmd,
> + const char *error)
> {
> int i;
> struct string_list suggested_refs = guess_refs(ref);
>
> fprintf_ln(stderr, _("%s: %s - %s"), cmd, ref, error);
>
> if (suggested_refs.nr > 0) {
> fprintf_ln(stderr,
> Q_("\nDid you mean this?",
> "\nDid you mean one of these?",
> suggested_refs.nr));
> for (i = 0; i < suggested_refs.nr; i++)
> fprintf(stderr, "\t%s\n",
> suggested_refs.items[i].string);
> }
>
> string_list_clear(&suggested_refs, 0);
> exit(1);
> }
Indeed, this function always ends up exit()-ing.
> diff --git a/help.h b/help.h
> index b8780fbd0f..7a455beeb7 100644
> --- a/help.h
> +++ b/help.h
> @@ -42,8 +42,8 @@ void list_commands(unsigned int colopts, struct cmdnames
> *main_cmds, struct cmdn
> /*
> * call this to die(), when it is suspected that the user mistyped a
> * ref to the command, to give suggested "correct" refs.
> */
> -void help_unknown_ref(const char *ref, const char *cmd, const char *error);
> +NORETURN void help_unknown_ref(const char *ref, const char *cmd, const char
> *error);
Funny how this claims we'll call `die()`, when we'll actually call
`exit(1)`. If we actually did call `die()`, I suppose the compiler
should/could figure out by itself that this function, too, won't ever
return.
I wonder whether the real bug here is that the implementation calls
`exit(1)`, not `die()`. That is, the exit code is wrong (1 != 128) and
we're missing out on the flexibility offered by `set_die_routine()`. If
not that, then I'd say the documentation is buggy. Hm?
In any case, your patch seems correct. Just wondering what should be
done on top of it...
Martin
