From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Thu, 10 May 2018 22:20:12 +0300 From: Konstantin Osipov Subject: Re: [tarantool-patches] [PATCH v2 06/10] session: introduce session vtab and meta Message-ID: <20180510192012.GF30593@atlas> References: <5413183e3cc0d65220452a9c1f72cea1ca1dc546.1524228894.git.v.shpilevoy@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5413183e3cc0d65220452a9c1f72cea1ca1dc546.1524228894.git.v.shpilevoy@tarantool.org> To: tarantool-patches@freelists.org Cc: vdavydov.dev@gmail.com List-ID: * Vladislav Shpilevoy [18/04/20 16:25]: > +int > +iproto_session_fd(struct session *session) > +{ > + struct iproto_connection *conn = > + (struct iproto_connection *) session->meta.conn; > + return conn->output.fd; > +} Nitpick: I would not abbreviate connection when in a member, and use 'con' as in other places for name of the variable declared on the stack. > +/** > + * Push a message using a protocol, depending on a session type. > + * @param data Data to push, first argument on a stack. > + * @retval true Success. > + * @retval nil, error Error occured. > + */ > +static int > +lbox_session_push(struct lua_State *L) > +{ > + if (lua_gettop(L) != 1) > + return luaL_error(L, "Usage: box.session.push(data)"); > + > + if (session_push(current_session(), NULL) != 0) { > + lua_pushnil(L); > + luaT_pusherror(L, box_error_last()); > + return 2; > + } else { > + lua_pushboolean(L, true); > + return 1; > + } > +} I'm not sure we the calling convention should differ from the rest of box API, i.e. we should return nil, error rather than exceptions. Could you run a poll in the community chat? > + > /** > * Sets trigger on_access_denied. > * For test purposes only. > @@ -429,6 +451,7 @@ box_lua_session_init(struct lua_State *L) > {"on_disconnect", lbox_session_on_disconnect}, > {"on_auth", lbox_session_on_auth}, > {"on_access_denied", lbox_session_on_access_denied}, > + {"push", lbox_session_push}, > {NULL, NULL} > }; > luaL_register_module(L, sessionlib_name, sessionlib); > diff --git a/src/box/session.cc b/src/box/session.cc > index 3d787bd51..4a1397c24 100644 > --- a/src/box/session.cc > +++ b/src/box/session.cc > @@ -45,6 +45,20 @@ const char *session_type_strs[] = { > "unknown", > }; > > +static struct session_vtab generic_session_vtab = { > + /* .push = */ generic_session_push, > + /* .fd = */ generic_session_fd, > + /* .sync = */ generic_session_sync, > +}; > + > +struct session_vtab session_vtab_registry[] = { > + /* BACKGROUND */ generic_session_vtab, > + /* BINARY */ generic_session_vtab, > + /* CONSOLE */ generic_session_vtab, > + /* REPL */ generic_session_vtab, > + /* APPLIER */ generic_session_vtab, > +}; > + > static struct mh_i64ptr_t *session_registry; > > struct mempool session_pool; > @@ -79,7 +93,7 @@ session_on_stop(struct trigger *trigger, void * /* event */) > } > > struct session * > -session_create(int fd, enum session_type type) > +session_create(enum session_type type) > { > struct session *session = > (struct session *) mempool_alloc(&session_pool); > @@ -89,8 +103,7 @@ session_create(int fd, enum session_type type) > return NULL; > } > session->id = sid_max(); > - session->fd = fd; > - session->sync = 0; > + memset(&session->meta, 0, sizeof(session->meta)); > session->type = type; > /* For on_connect triggers. */ > credentials_init(&session->credentials, guest_user->auth_token, > @@ -110,12 +123,12 @@ session_create(int fd, enum session_type type) > } > > struct session * > -session_create_on_demand(int fd) > +session_create_on_demand() > { > assert(fiber_get_session(fiber()) == NULL); > > /* Create session on demand */ > - struct session *s = session_create(fd, SESSION_TYPE_BACKGROUND); > + struct session *s = session_create(SESSION_TYPE_BACKGROUND); > if (s == NULL) > return NULL; > s->fiber_on_stop = { > @@ -278,3 +291,27 @@ access_check_universe(user_access_t access) > } > return 0; > } > + > +int > +generic_session_push(struct session *session, struct port *port) > +{ > + (void) port; > + const char *name = > + tt_sprintf("Session '%s'", session_type_strs[session->type]); > + diag_set(ClientError, ER_UNSUPPORTED, name, "push()"); > + return -1; > +} > + > +int > +generic_session_fd(struct session *session) > +{ > + (void) session; > + return -1; > +} > + > +int64_t > +generic_session_sync(struct session *session) > +{ > + (void) session; > + return 0; > +} > diff --git a/src/box/session.h b/src/box/session.h > index c387e6f95..e583c8c6b 100644 > --- a/src/box/session.h > +++ b/src/box/session.h > @@ -41,6 +41,9 @@ > extern "C" { > #endif /* defined(__cplusplus) */ > > +struct port; > +struct session_vtab; > + > void > session_init(); > > @@ -58,6 +61,23 @@ enum session_type { > > extern const char *session_type_strs[]; > > +/** > + * Session meta is used in different ways by sessions of different > + * types, and allows to do not store attributes in struct session, > + * that are used only by a session of particular type. > + */ > +struct session_meta { > + union { > + /** IProto connection meta. */ > + struct { > + uint64_t sync; > + void *conn; > + }; > + /** Only by console is used. */ > + int fd; > + }; > +}; > + > /** Otherwise the patch LGTM. -- Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 http://tarantool.io - www.twitter.com/kostja_osipov