[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