From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [tarantool-patches] Re: [PATCH v1 1/1] Implement mp_stack_top for mp_stack class References: <316cea905dca2ac8db8b1adb9d0040a9c338dc5c.1554219130.git.kshcherbatov@tarantool.org> <20190403121202.w66dbmhr7yrr5ysd@esperanza> <4377f916-eb85-44c4-2124-cb4b14d5a6d4@tarantool.org> <20190403154003.kdpt3vgbqtch3sjg@esperanza> From: Kirill Shcherbatov Message-ID: <62477fef-d2fc-9559-30f4-36d03a9b70db@tarantool.org> Date: Wed, 3 Apr 2019 20:49:23 +0300 MIME-Version: 1.0 In-Reply-To: <20190403154003.kdpt3vgbqtch3sjg@esperanza> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit To: tarantool-patches@freelists.org, Vladimir Davydov List-ID: > Now it's not that bad, don't you agree? We even managed to get rid of > the empty array/map check. Yep. Looks good. On the branch. ==================================================== Introduced a new mp_stack_top method for mp_stack class to return the pointer to a top frame of the stack. Class methods have been redesigned so that this new mp_stack_top helper fits into the API naturally. This is required in scope of multikey indexes to keep a pointer to a multikey frame and extract currently processed item index of this frame later. Needed for https://github.com/tarantool/tarantool/issues/1257 --- msgpuck.c | 16 ++++++------ msgpuck.h | 76 ++++++++++++++++++++----------------------------------- 2 files changed, 36 insertions(+), 56 deletions(-) diff --git a/msgpuck.c b/msgpuck.c index 05e472e..d0ffb83 100644 --- a/msgpuck.c +++ b/msgpuck.c @@ -306,16 +306,16 @@ next: \ return -1; \ } \ while (!mp_stack_is_empty(&stack)) { \ - enum mp_type type = mp_stack_type(&stack); \ - int curr = mp_stack_advance(&stack); \ - if (curr == 0 || mp_stack_count(&stack) == 0) \ - PRINTF(type == MP_ARRAY ? "[" : "{"); \ - if (curr == -1) { \ - PRINTF(type == MP_ARRAY ? "]" : "}"); \ + struct mp_frame *frame = mp_stack_top(&stack); \ + if (frame->idx < 0) \ + PRINTF(frame->type == MP_ARRAY ? "[" : "{"); \ + if (!mp_frame_advance(frame)) { \ + PRINTF(frame->type == MP_ARRAY ? "]" : "}"); \ mp_stack_pop(&stack); \ continue; \ - } else if (curr != 0) { \ - PRINTF(type == MP_MAP && curr % 2 == 1 ? ": " : ", "); \ + } else if (frame->idx != 0) { \ + PRINTF(frame->type == MP_MAP && frame->idx % 2 == 1 ? \ + ": " : ", "); \ } \ goto next; \ } \ diff --git a/msgpuck.h b/msgpuck.h index eab1339..64b10e6 100644 --- a/msgpuck.h +++ b/msgpuck.h @@ -1222,7 +1222,7 @@ struct mp_frame { * Index of currently processing item. Must be less than * mp_frame::count member. */ - int curr; + int idx; }; /** @@ -1278,6 +1278,14 @@ mp_stack_is_empty(struct mp_stack *stack); MP_PROTO bool mp_stack_is_full(struct mp_stack *stack); +/** + * \brief Return the top mp_stack \a stack frame. + * \param stack - the pointer to a mp_stack to operate with. + * \pre mp_stack_is_empty(stack) == false + */ +MP_PROTO struct mp_frame * +mp_stack_top(struct mp_stack *stack); + /** * \brief Pop the top mp_stack \a stack frame. * \param stack - the pointer to a mp_stack to operate with. @@ -1298,33 +1306,13 @@ MP_PROTO void mp_stack_push(struct mp_stack *stack, enum mp_type type, int count); /** - * \brief Get type attribute of the \a stack top frame. - * \param stack - the pointer to a stack to operate with. - * \retval enum mp_type value - the top stack frame type. - * \pre mp_stack_is_empty(stack) == false - */ -MP_PROTO enum mp_type -mp_stack_type(struct mp_stack *stack); - -/** - * \brief Get count attribute of the \a stack top frame. - * \param stack - the pointer to a stack to operate with. - * \retval count - the top stack frame items count. - * \pre mp_stack_is_empty(stack) == false - */ -MP_PROTO int -mp_stack_count(struct mp_stack *stack); - -/** - * \brief Advance curr attribute of the \a stack top frame. - * \param stack - the pointer to a stack to operate with. - * \retval index of the element to process if the top - * mp_frame::curr is less than top mp_frame::count field. - * -1 otherwise. - * \pre mp_stack_is_empty(stack) == false + * \brief Advance idx attribute of the \a frame. + * \param frame - the frame pointer to operate with. + * \retval true when mp_frame::idx is less than mp_frame::count. + * false otherwise. */ -MP_PROTO int -mp_stack_advance(struct mp_stack *stack); +MP_PROTO bool +mp_frame_advance(struct mp_frame *frame); /* * }}} @@ -2349,6 +2337,12 @@ mp_stack_is_full(struct mp_stack *stack) return stack->used >= stack->size; } +MP_IMPL struct mp_frame * +mp_stack_top(struct mp_stack *stack) +{ + return &stack->frames[stack->used - 1]; +} + MP_IMPL void mp_stack_pop(struct mp_stack *stack) { @@ -2363,30 +2357,16 @@ mp_stack_push(struct mp_stack *stack, enum mp_type type, int count) int idx = stack->used++; stack->frames[idx].type = type; stack->frames[idx].count = count; - stack->frames[idx].curr = -1; -} - -MP_IMPL enum mp_type -mp_stack_type(struct mp_stack *stack) -{ - assert(!mp_stack_is_empty(stack)); - return stack->frames[stack->used - 1].type; + stack->frames[idx].idx = -1; } -MP_IMPL int -mp_stack_count(struct mp_stack *stack) -{ - return stack->frames[stack->used - 1].count; -} - -MP_IMPL int -mp_stack_advance(struct mp_stack *stack) +MP_IMPL bool +mp_frame_advance(struct mp_frame *frame) { - assert(!mp_stack_is_empty(stack)); - struct mp_frame *frame = &stack->frames[stack->used - 1]; - if (frame->curr < frame->count - 1) - return ++frame->curr; - return -1; + if (frame->idx >= frame->count - 1) + return false; + ++frame->idx; + return true; } /** \endcond */ -- 2.21.0