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

Reply via email to