Hi Duy,
On Wed, 2 May 2018, Duy Nguyen wrote:
> On Wed, May 2, 2018 at 11:38 AM, Johannes Schindelin
> <johannes.schindelin@xxxxxx> wrote:
> > When we call BUG(), we signal via SIGABRT that something bad happened,
> > dumping cores if so configured. In some setups these coredumps are
> > redirected to some central place such as /proc/sys/kernel/core_pattern,
> > which is a good thing.
> >
> > However, when we try to verify in our test suite that bugs are caught in
> > certain code paths, we do *not* want to clutter such a central place
> > with unnecessary coredumps.
> >
> > So let's special-case the test helpers (which we use to verify such code
> > paths) so that the BUG() calls will *not* call abort() but exit with a
> > special-purpose exit code instead.
> >
> > Helped-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> > ---
> > t/helper/test-tool.c | 2 ++
> > usage.c | 5 +++++
> > 2 files changed, 7 insertions(+)
> >
> > diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
> > index 87066ced62a..5176f9f20ae 100644
> > --- a/t/helper/test-tool.c
> > +++ b/t/helper/test-tool.c
> > @@ -47,7 +47,9 @@ static struct test_cmd cmds[] = {
> > int cmd_main(int argc, const char **argv)
> > {
> > int i;
> > + extern int BUG_exit_code;
> >
> > + BUG_exit_code = 99;
>
> It may be even better to let individual tests in t1406 control this,
> pretty much like your original patch, except that we tell usage.c to
> not send SIGABRT and just return a special fault code. That way we
> don't accidentally suppress BUG()'s sigabrt behavior in tests that do
> not anticipate it (even in t1406).
I thought long and hard about this, even slept over it. And I came to the
conclusion that I do not really know whether we want such a special
treatment (you may even want to go crazier and limit *which* BUG() call
you intend to catch, so that others are still reported).
And I came to an important realization: whether or not to handle the bugs
in the way you described is actually *outside* the purpose of this patch
series. This patch series is really only about converting die(BUG:...)
calls to BUG() calls. And it simply leaves the concern you raised for
another patch series.
> But this patch is ok for me too if you don't want another reroll.
I don't ;-) (for the reasons mentioned above: I don't disagree with you, I
just think it should be addressed in another patch series than this here
patch series).
Ciao,
Dscho
