Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@freelists.org, imeevma@tarantool.org
Subject: [tarantool-patches] Re: [PATCH v3 5/7] sql: create interface vstream
Date: Wed, 28 Nov 2018 21:25:42 +0300	[thread overview]
Message-ID: <24767faf-cab9-abe0-615f-3e404672b9e4@tarantool.org> (raw)
In-Reply-To: <795ccb629260269c8ffffb52d5e426e35ed0a088.1543344471.git.imeevma@gmail.com>

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@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);

  reply	other threads:[~2018-11-28 18:25 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-27 19:25 [tarantool-patches] [PATCH v3 0/7] Remove box.sql.execute imeevma
2018-11-27 19:25 ` [tarantool-patches] [PATCH v3 1/7] box: store sql text and length in sql_request imeevma
2018-11-29 10:45   ` Vladimir Davydov
2018-11-27 19:25 ` [tarantool-patches] [PATCH v3 3/7] iproto: remove iproto functions from execute.c imeevma
2018-11-29 10:51   ` Vladimir Davydov
2018-11-27 19:25 ` [tarantool-patches] [PATCH v3 4/7] iproto: replace obuf by mpstream in execute.c imeevma
2018-11-28 13:10   ` Vladislav Shpilevoy
2018-11-29 10:53   ` Vladimir Davydov
2018-11-29 14:04     ` [tarantool-patches] " Vladislav Shpilevoy
2018-11-30 10:19       ` Vladimir Davydov
2018-11-30 10:45         ` Vladislav Shpilevoy
2018-11-30 10:55           ` Vladimir Davydov
2018-11-27 19:25 ` [tarantool-patches] [PATCH v3 7/7] sql: check new box.sql.execute() imeevma
2018-11-28 13:33 ` [tarantool-patches] [PATCH v3 2/7] box: add method dump_lua to port imeevma
2018-11-29 10:48   ` Vladimir Davydov
2018-11-28 13:45 ` [tarantool-patches] [PATCH v3 5/7] sql: create interface vstream imeevma
2018-11-28 18:25   ` Vladislav Shpilevoy [this message]
2018-11-28 13:50 ` [tarantool-patches] [PATCH v3 6/7] lua: create vstream implementation for Lua imeevma
2018-11-28 18:25   ` [tarantool-patches] " Vladislav Shpilevoy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=24767faf-cab9-abe0-615f-3e404672b9e4@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=imeevma@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH v3 5/7] sql: create interface vstream' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox