[tarantool-patches] Re: [PATCH v3 5/7] sql: create interface vstream
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Wed Nov 28 21:25:42 MSK 2018
Hi! Thanks for the fixes!
But sorry, I see one new problem: vstream for unknown reason is
defined in box/vstream.h, but src/mpstream.c depends on it though
should not. box/ depends on src/, not vice versa. Since vstream.h
is just a header and do not use anything from box, it can be moved
to src/. I did it.
Also, I removed mp_vstream_init_vtab by introduction of a new
function mpvstream_init, which does the same as mpstream_init +
initializes vtab. It allows to construct usable vstream in one
call instead of mpstream_init + init_vtab.
See my review fixes below (they are on the branch as well).
Please, test all my fixes before squashing. I've run only sql/iproto
test.
=================================================================
commit 6bae55769199fdc549362d1d2fd758c124f0b1db
Author: Vladislav Shpilevoy <v.shpilevoy at tarantool.org>
Date: Wed Nov 28 15:26:26 2018 +0300
Review fixes
diff --git a/src/box/iproto.cc b/src/box/iproto.cc
index 1c4c65176..6e284f78d 100644
--- a/src/box/iproto.cc
+++ b/src/box/iproto.cc
@@ -1611,11 +1611,10 @@ tx_process_sql(struct cmsg *m)
goto error;
struct vstream stream;
- mpstream_init((struct mpstream *)&stream, out, obuf_reserve_cb,
- obuf_alloc_cb, set_encode_error, &is_error);
+ mpvstream_init(&stream, out, obuf_reserve_cb, obuf_alloc_cb,
+ set_encode_error, &is_error);
if (is_error)
goto error;
- mp_vstream_init_vtab(&stream);
if (sql_response_dump(&response, &keys, &stream) != 0 || is_error) {
obuf_rollback_to_svp(out, &header_svp);
goto error;
diff --git a/src/mpstream.c b/src/mpstream.c
index 4091ead75..6636da1a6 100644
--- a/src/mpstream.c
+++ b/src/mpstream.c
@@ -33,8 +33,8 @@
#include <assert.h>
#include <stdint.h>
#include "msgpuck.h"
-#include "box/vstream.h"
-#include "box/port.h"
+#include "vstream.h"
+#include "port.h"
void
mpstream_reserve_slow(struct mpstream *stream, size_t size)
@@ -178,7 +178,7 @@ mpstream_encode_bool(struct mpstream *stream, bool val)
mpstream_advance(stream, pos - data);
}
-int
+static int
mp_vstream_encode_port(struct vstream *stream, struct port *port)
{
struct mpstream *mpstream = (struct mpstream *)stream;
@@ -191,7 +191,7 @@ mp_vstream_encode_port(struct vstream *stream, struct port *port)
return 0;
}
-void
+static void
mp_vstream_encode_enum(struct vstream *stream, int64_t num, const char *str)
{
(void)str;
@@ -201,13 +201,13 @@ mp_vstream_encode_enum(struct vstream *stream, int64_t num, const char *str)
mpstream_encode_uint((struct mpstream *)stream, num);
}
-void
+static void
mp_vstream_noop(struct vstream *stream, ...)
{
(void) stream;
}
-const struct vstream_vtab mp_vstream_vtab = {
+static const struct vstream_vtab mp_vstream_vtab = {
/** encode_array = */ (encode_array_f)mpstream_encode_array,
/** encode_map = */ (encode_map_f)mpstream_encode_map,
/** encode_uint = */ (encode_uint_f)mpstream_encode_uint,
@@ -224,7 +224,11 @@ const struct vstream_vtab mp_vstream_vtab = {
};
void
-mp_vstream_init_vtab(struct vstream *vstream)
+mpvstream_init(struct vstream *stream, void *ctx, mpstream_reserve_f reserve,
+ mpstream_alloc_f alloc, mpstream_error_f error, void *error_ctx)
{
- vstream->vtab = &mp_vstream_vtab;
+ stream->vtab = &mp_vstream_vtab;
+ assert(sizeof(stream->inheritance_padding) >= sizeof(struct mpstream));
+ mpstream_init((struct mpstream *) stream, ctx, reserve, alloc, error,
+ error_ctx);
}
diff --git a/src/mpstream.h b/src/mpstream.h
index e22d05241..ce0e25dae 100644
--- a/src/mpstream.h
+++ b/src/mpstream.h
@@ -78,6 +78,13 @@ mpstream_init(struct mpstream *stream, void *ctx,
mpstream_reserve_f reserve, mpstream_alloc_f alloc,
mpstream_error_f error, void *error_ctx);
+struct vstream;
+
+/** Initialize a vstream object as an instance of mpstream. */
+void
+mpvstream_init(struct vstream *stream, void *ctx, mpstream_reserve_f reserve,
+ mpstream_alloc_f alloc, mpstream_error_f error, void *error_ctx);
+
static inline void
mpstream_flush(struct mpstream *stream)
{
diff --git a/src/box/vstream.h b/src/vstream.h
similarity index 99%
rename from src/box/vstream.h
rename to src/vstream.h
index 01a5212fb..fe7c49a36 100644
--- a/src/box/vstream.h
+++ b/src/vstream.h
@@ -35,7 +35,6 @@ extern "C" {
#endif /* defined(__cplusplus) */
struct vstream;
-struct lua_State;
struct port;
typedef void (*encode_array_f)(struct vstream *stream, uint32_t size);
More information about the Tarantool-patches
mailing list