On Thu, 29 Aug 2019 at 22:08, René Scharfe <l.s.r@xxxxxx> wrote:
>
> Am 29.08.19 um 21:40 schrieb Martin Ågren:
> > On Thu, 29 Aug 2019 at 21:15, René Scharfe <l.s.r@xxxxxx> wrote:
> >> 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)`.
>
> Ah, I didn't even notice that.
>
> > If we actually did call `die()`, I suppose the compiler
> > should/could figure out by itself that this function, too, won't ever
> > return.
>
> The compiler can figure it out with exit(), too; system headers (at
> least for glibc, but it's probably common) assign it the noreturn
> attribute. But there is no way to transmit that information to callers
> across compilation units if not for the header file, right?
Of course, you're right.
> > I wonder whether the real bug here is that the implementation calls
(Re-reading this, "the real bug" might be a bit of a harsh statement. I
didn't mean to imply that this patch does not fix an actual problem.)
> > `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?
>
> This inconsistency has been present since e56181060e ("help: add
> help_unknown_ref()", 2013-05-04). Using die() is going to be difficult
> due to the multi-line suggestions printed by the function.
Yeah, that's true. We could manually prefix each line with "fatal: " or
"error: ", then die with something like "see above", which is not very
cool, or die("%s", error), which is a bit repetitive. There are a few
decisions to be made for fixing this discrepancy.
Anyway, I don't think this is something that needs to hold up your
patch.
Martin
