[tarantool-patches] Re: [PATCH v1 1/1] Implement mp_stack_top for mp_stack class
Kirill Shcherbatov
kshcherbatov at tarantool.org
Fri Apr 5 20:17:23 MSK 2019
> Nit:
> assert(!mp_stack_is_empty(stack));
Tnx.
> Other than that, the patch looks good, BUT for some reason it doesn't
> compile on CentOS 6 on Travis, see
>
> https://travis-ci.org/tarantool/msgpuck/jobs/515314874
>
> It fails with
>
> BFD: /build/BUILDROOT/msgpuck-2.0.23-1.el6.x86_64/usr/lib64/libmsgpuck.a(msgpuck.c.o): invalid relocation type 42
>
> Please figure out why is that and fix if possible.
Answered on other 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 | 77 +++++++++++++++++++++----------------------------------
2 files changed, 37 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..2d39b40 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,13 @@ 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)
+{
+ assert(!mp_stack_is_empty(stack));
+ return &stack->frames[stack->used - 1];
+}
+
MP_IMPL void
mp_stack_pop(struct mp_stack *stack)
{
@@ -2363,30 +2358,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;
-}
-
-MP_IMPL int
-mp_stack_count(struct mp_stack *stack)
-{
- return stack->frames[stack->used - 1].count;
+ stack->frames[idx].idx = -1;
}
-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
More information about the Tarantool-patches
mailing list