From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id BB99C300CF for ; Wed, 28 Nov 2018 13:25:45 -0500 (EST) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id c25ISWIX6-8c for ; Wed, 28 Nov 2018 13:25:45 -0500 (EST) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 7595C3003B for ; Wed, 28 Nov 2018 13:25:45 -0500 (EST) Subject: [tarantool-patches] Re: [PATCH v3 5/7] sql: create interface vstream References: <795ccb629260269c8ffffb52d5e426e35ed0a088.1543344471.git.imeevma@gmail.com> From: Vladislav Shpilevoy Message-ID: <24767faf-cab9-abe0-615f-3e404672b9e4@tarantool.org> Date: Wed, 28 Nov 2018 21:25:42 +0300 MIME-Version: 1.0 In-Reply-To: <795ccb629260269c8ffffb52d5e426e35ed0a088.1543344471.git.imeevma@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org, imeevma@tarantool.org 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 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 #include #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);