[PATCH 4/5] session: introduce session_owner
Vladimir Davydov
vdavydov.dev at gmail.com
Tue Mar 20 21:29:20 MSK 2018
On Mon, Mar 19, 2018 at 04:34:51PM +0300, Vladislav Shpilevoy wrote:
> Session owner stores a session type specific data. For example,
> IProto session has authentication salt, console session has
> file descriptor.
>
> For #2677 session owner of IProto and console will have push()
> virtual function to do box.session.push, which implementation
> depends on a session type.
>
> Needed for #2677
> diff --git a/src/box/session.h b/src/box/session.h
> @@ -60,6 +58,49 @@ enum session_type {
>
> extern const char *session_type_strs[];
>
> +struct session_owner_vtab;
> +
> +/**
> + * Object to store session type specific data. For example, IProto
> + * stores iproto_connection, console stores file descriptor.
> + */
> +struct session_owner {
I don't really like the name. Please come up with some alternatives
(session_context, creator, data? dunno) so that we can pick the best
one.
> + /** Session type. */
> + enum session_type type;
> + /** Virtual session owner methods. */
> + const struct session_owner_vtab *vtab;
> +};
> +
> +struct session_owner_vtab {
> + /** Allocate a duplicate of an owner. */
> + struct session_owner *(*dup)(struct session_owner *);
> + /** Destroy an owner, and free its memory. */
> + void (*free)(struct session_owner *);
> + /** Get the descriptor of an owner, if has. Else -1. */
> + int (*fd)(const struct session_owner *);
> +};
> +
> +static inline struct session_owner *
> +session_owner_dup(struct session_owner *owner)
> +{
> + return owner->vtab->dup(owner);
> +}
> +
> +static inline void
> +session_owner_delete(struct session_owner *owner)
> +{
> + owner->vtab->free(owner);
> +}
> diff --git a/src/box/session.cc b/src/box/session.cc
> @@ -112,13 +155,25 @@ session_create(int fd, enum session_type type)
> return session;
> }
>
> +int
> +session_set_owner(struct session *session, struct session_owner *new_owner)
> +{
> + struct session_owner *dup = session_owner_dup(new_owner);
> + if (dup == NULL)
> + return -1;
> + if (session->owner != NULL)
> + session_owner_delete(session->owner);
> + session->owner = dup;
> + return 0;
> +}
> diff --git a/src/box/applier.cc b/src/box/applier.cc
> @@ -533,7 +533,9 @@ applier_f(va_list ap)
> * Set correct session type for use in on_replace()
> * triggers.
> */
> - current_session()->type = SESSION_TYPE_APPLIER;
> + struct session_owner applier_owner;
> + session_owner_create(&applier_owner, SESSION_TYPE_APPLIER);
> + session_set_owner(current_session(), &applier_owner);
I don't like the way you set the owner: first you initialize it on
stack, then duplicate it on heap. This doesn't look good.
Let's try to embed the session_owner structure (or whatever it will be
called) in struct session with some padding so that we don't need to
allocate anything, i.e.:
struct session_owner {
int type;
struct session_owner_vtab vtab;
char pad[128];
};
struct session {
struct session_owner owner;
We can use static assertions to make sure all deriving structures fit in
(see struct port for example).
Then we would add
iproto_init_session(session, ...)
console_init_session(session, ...)
session_clear_owner(session)
(elaborate the names pls)
that would initialize session->owner appropriately.
More information about the Tarantool-patches
mailing list