Re: [Tarantool-patches] [PATCH luajit 15/19] Fix LJ_MAX_JSLOTS assertion in rec_check_slots().

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Maxim!
Thanks for the review!
Fixed your comments inline.

On 15.08.23, Maxim Kokryashkin wrote:
> Hi, Sergey!
> Thanks for the patch!
> LGTM, except for a few comments below.
> On Wed, Aug 09, 2023 at 06:36:04PM +0300, Sergey Kaplun via Tarantool-patches wrote:
> > From: Mike Pall <mike>
> > 
> > Thanks to Yichun Zhang.
> > 
> > (cherry-picked from commit 1c89933f129dde76944336c6bfd05297b8d67730)
> > 
> > This patch is predecessor for the commit
> Typo: s/is predecessor for the/is the predecessor to/

Fixed.

> > 944d32afd6ddd9dbac1cddf64bf81333efeb9e30 ("Add missing LJ_MAX_JSLOTS
> > check.") It tries to fix the issue, when `J->baseslot == LJ_MAX_JSLOTS`,
> > that leading to the assertion failure. Since the predecessor patch,
> Typo: s/leading/leads/

Fixed, thanks!

> > there are no places, that can lead to the condition failure, since we
> > always check that new baseslot + framesize (+ vargframe) >=
> > `LJ_MAX_JSLOTS`. As far as minimum framesize is 1 (see <src/lj_parse.c>
> Typo: s/as minimum/as the minimum/

Fixed.

> > for details), we can't obtain this assertion failure. This patch is
> > added for the consistency with the upstream.
> Typo: s/the consistency/consistency/

Fixed.

> > 
> > Since the predecessor patch fixes the issue, there is no new test case
> > to add.
> > 
> > Sergey Kaplun:
> > * added the description for the problem
> > 
> > Part of tarantool/tarantool#8825
> > ---
> >  src/lj_record.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/lj_record.c b/src/lj_record.c
> > index 02d9db9e..6030f77c 100644
> > --- a/src/lj_record.c
> > +++ b/src/lj_record.c

<snipped>

> > -- 
> > 2.41.0
> > 

-- 
Best regards,
Sergey Kaplun