Re: [Tarantool-patches] [PATCH 2/2] fiber: fiber_join -- don't crash on misuse

[Cyrill Gorcunov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

On Mon, Apr 26, 2021 at 10:58:16PM +0200, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
> 
> > diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
> > index baf78a130..dd7498dd7 100644
> > --- a/src/lib/core/fiber.c
> > +++ b/src/lib/core/fiber.c
> > @@ -615,7 +615,10 @@ fiber_join(struct fiber *fiber)
> >  int
> >  fiber_join_timeout(struct fiber *fiber, double timeout)
> >  {
> > -	assert(fiber->flags & FIBER_IS_JOINABLE);
> > +	if (!(fiber->flags & FIBER_IS_JOINABLE)) {
> 
> 1. https://github.com/tarantool/tarantool/wiki/Code-review-procedure#code-style
> 
> 	In C we don't apply ! operator to non-boolean values. It means, to
> 	check if an integer is not 0, you use != 0. To check if a pointer is
> 	not NULL, you use != NULL. The same for ==;

I know. I left it this way on a purpose -- the whole file follows more
sane use of negation operator. I thought better to keep the old form,
otherwise we will have a mixture of old a new style in one file.

> >  	struct fiber *fiber = fiber_new_xc("join", noop_f);
> > +	/* gh-6046: crash on attempt to join non joinable */
> > +	fiber_set_joinable(fiber, false);
> > +	fiber_join(fiber);
> 
> 2. Would be good to test that it returns -1, and that the diag is not
> empty.

Will update, thanks!