Re: [PATCH] help: make help_unknown_ref() NORETURN
- From
- Martin Ågren <martin.agren@gmail.com>
- Date
- Aug 30, 2019, 03:50 UTC
- Message-ID
- <CAN0heSo6YdCn2cHUTiyhuZ3Z_Rk6YFg=yhWCVHhaoAC3_0=0Xg@mail.gmail.com>
- In-Reply-To
- <bde881b6-7c23-6f4e-5cb0-a793a5e4f5d7@web.de>
On Thu, 29 Aug 2019 at 22:08, René Scharfe <l.s.r@web.de> wrote:
Show 28 quoted lines
> > Am 29.08.19 um 21:40 schrieb Martin Ågren: > > On Thu, 29 Aug 2019 at 21:15, René Scharfe <l.s.r@web.de> 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.)
Show 7 quoted lines
> > `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