From 508d2050a6cc010307648cb4439d580e41f6dfb7 Mon Sep 17 00:00:00 2001 From: Norman Feske Date: Wed, 18 Jul 2012 14:48:37 +0200 Subject: [PATCH] linux: Fix 'explicit_reply' semantics By storing the reply socket descriptor inside the 'Ipc_ostream::_dst' capability instead as part of the connection state object, we can use the 'explicit_reply' mechanism as usual. Right now, we store both the tid and socket handle in 'Native_capability::Dst'. In the final version, the 'tid' member will be gone. --- base-linux/include/base/native_types.h | 50 +++++--------------------- base-linux/src/base/ipc/ipc.cc | 42 ++++++++++++++-------- base-linux/src/platform/linux_socket.h | 28 +++++++-------- 3 files changed, 50 insertions(+), 70 deletions(-) diff --git a/base-linux/include/base/native_types.h b/base-linux/include/base/native_types.h index d30526945..a91e4ecbe 100644 --- a/base-linux/include/base/native_types.h +++ b/base-linux/include/base/native_types.h @@ -94,14 +94,17 @@ namespace Genode { inline bool operator != (Native_thread_id t1, Native_thread_id t2) { return (t1.tid != t2.tid) || (t1.pid != t2.pid); } - struct Cap_dst_policy { - + struct Cap_dst_policy + { struct Dst { long tid; /* XXX to be removed once the transition to SCM rights is completed */ int socket; + /** + * Default constructor creates invalid destination + */ Dst() : tid(0), socket(-1) { } Dst(long tid, int socket) : tid(tid), socket(socket) { } @@ -119,45 +122,10 @@ namespace Genode { typedef Native_capability_tpl Native_capability; - class Native_connection_state - { - public: - - struct Reply_socket_error { }; - - private: - - int _socket; /* server-entrypoint socket */ - int _reply_socket; /* reply socket */ - - public: - - Native_connection_state() : _socket(-1), _reply_socket(-1) { } - - void socket(int socket) { _socket = socket; } - int socket() const { return _socket; } - - /* - * FIXME Check for unsupported usage pattern: Reply sockets should - * only be set once and read once. - */ - - void reply_socket(int socket) - { - if (_reply_socket != -1) throw Reply_socket_error(); - - _reply_socket = socket; - } - - int reply_socket() - { - if (_reply_socket == -1) throw Reply_socket_error(); - - int s = _reply_socket; - _reply_socket = -1; - return s; - } - }; + /** + * The connection state is the socket handle of the RPC entrypoint + */ + typedef int Native_connection_state; struct Native_config { diff --git a/base-linux/src/base/ipc/ipc.cc b/base-linux/src/base/ipc/ipc.cc index 9abf1a651..582079b34 100644 --- a/base-linux/src/base/ipc/ipc.cc +++ b/base-linux/src/base/ipc/ipc.cc @@ -108,8 +108,8 @@ Ipc_istream::~Ipc_istream() { } void Ipc_client::_prepare_next_call() { /* prepare next request in buffer */ - long local_name = Ipc_ostream::_dst.local_name(); - long tid = Native_capability::dst().tid; + long const local_name = Ipc_ostream::_dst.local_name(); + long const tid = Native_capability::dst().tid; _write_offset = 0; _write_to_buf(local_name); @@ -151,9 +151,10 @@ void Ipc_server::_prepare_next_reply_wait() /* read client thread id from request buffer */ long tid = 0; if (_reply_needed) { + /* XXX to be removed */ + long tid; + if (_reply_needed) _read_from_buf(tid); - Ipc_ostream::_dst = Native_capability(Dst(tid, -1), 0); /* only _tid member is used */ - } /* prepare next reply */ _write_offset = 0; @@ -167,21 +168,34 @@ void Ipc_server::_prepare_next_reply_wait() void Ipc_server::_wait() { - /* wait for new server request */ - try { - lx_wait(_rcv_cs, _rcv_msg->buf, _rcv_msg->size()); - } catch (Blocking_canceled) { } - - /* now we have a request to reply, determine reply destination */ _reply_needed = true; - _prepare_next_reply_wait(); + + try { + int const reply_socket = lx_wait(_rcv_cs, _rcv_msg->buf, _rcv_msg->size()); + + /* + * Remember reply capability + * + * The 'local_name' of a capability is meaningful for addressing server + * objects only. Because a reply capabilities does not address a server + * object, the 'local_name' is meaningless. + */ + enum { DUMMY_LOCAL_NAME = -1 }; + + typedef Native_capability::Dst Dst; + enum { DUMMY_TID = -1 }; + Dst dst(DUMMY_TID, reply_socket); + Ipc_ostream::_dst = Native_capability(dst, DUMMY_LOCAL_NAME); + + _prepare_next_reply_wait(); + } catch (Blocking_canceled) { } } void Ipc_server::_reply() { try { - lx_reply(_rcv_cs, _snd_msg->buf, _write_offset); + lx_reply(Ipc_ostream::_dst.dst().socket, _snd_msg->buf, _write_offset); } catch (Ipc_error) { } _prepare_next_reply_wait(); @@ -192,7 +206,7 @@ void Ipc_server::_reply_wait() { /* when first called, there was no request yet */ if (_reply_needed) - lx_reply(_rcv_cs, _snd_msg->buf, _write_offset); + lx_reply(Ipc_ostream::_dst.dst().socket, _snd_msg->buf, _write_offset); _wait(); } @@ -202,7 +216,7 @@ Ipc_server::Ipc_server(Msgbuf_base *snd_msg, Msgbuf_base *rcv_msg) : Ipc_istream(rcv_msg), Ipc_ostream(Native_capability(), snd_msg), _reply_needed(false) { - _rcv_cs.socket(lx_server_socket(Thread_base::myself())); + _rcv_cs = lx_server_socket(Thread_base::myself()); _prepare_next_reply_wait(); } diff --git a/base-linux/src/platform/linux_socket.h b/base-linux/src/platform/linux_socket.h index ec50052be..9d527cd12 100644 --- a/base-linux/src/platform/linux_socket.h +++ b/base-linux/src/platform/linux_socket.h @@ -199,7 +199,7 @@ static void lx_call(long thread_id, ret = lx_sendmsg(reply_channel[LOCAL_SOCKET], send_msg.msg(), 0); if (ret < 0) { - PRAW("lx_sendmsg failed with %d in call()", ret); + PRAW("lx_sendmsg failed with %d in lx_call()", ret); throw Genode::Ipc_error(); } @@ -209,7 +209,7 @@ static void lx_call(long thread_id, ret = lx_recvmsg(reply_channel[LOCAL_SOCKET], recv_msg.msg(), 0); if (ret < 0) { - PRAW("lx_recvmsg failed with %d in call()", ret); + PRAW("lx_recvmsg failed with %d in lx_call()", ret); throw Genode::Ipc_error(); } @@ -221,8 +221,10 @@ static void lx_call(long thread_id, /** * Utility: Wait for request from client + * + * \return socket descriptor of reply capability */ -static void lx_wait(Genode::Native_connection_state &cs, +static int lx_wait(Genode::Native_connection_state &cs, void *buf, Genode::size_t buf_len) { int ret; @@ -232,37 +234,33 @@ static void lx_wait(Genode::Native_connection_state &cs, msg.prepare_reply_socket_slot(); msg.buffer(buf, buf_len); - ret = lx_recvmsg(cs.socket(), msg.msg(), 0); + ret = lx_recvmsg(cs, msg.msg(), 0); if (ret < 0) { - PRAW("lx_recvmsg failed with %d in wait()", ret); + PRAW("lx_recvmsg failed with %d in lx_wait()", ret); throw Genode::Ipc_error(); } - /* remember socket descriptor for reply */ - cs.reply_socket(msg.reply_socket()); + return msg.reply_socket(); } /** * Utility: Send reply to client */ -static void lx_reply(Genode::Native_connection_state &cs, +static void lx_reply(int reply_socket, void *buf, Genode::size_t buf_len) { int ret; - int sd = cs.reply_socket(); Message msg; msg.buffer(buf, buf_len); - ret = lx_sendmsg(sd, msg.msg(), 0); - if (ret < 0) { - PRAW("lx_sendmsg failed with %d in reply()", ret); - throw Genode::Ipc_error(); - } + ret = lx_sendmsg(reply_socket, msg.msg(), 0); + if (ret < 0) + PRAW("lx_sendmsg failed with %d in lx_reply()", ret); - lx_close(sd); + lx_close(reply_socket); } #endif /* _PLATFORM__LINUX_SOCKET_H_ */