Mir

Code review comment for lp:~robertcarr/mir/implement-client-credentials

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

My larger concerns are addressed. But a few smaller ones:

350 - std::shared_ptr<mfd::SocketSession> const& session,
351 + std::shared_ptr<mfd::SocketSession>& session,
...
393 - void on_new_connection(const std::shared_ptr<detail::SocketSession>& session, const boost::system::error_code& ec);
394 + void on_new_connection(std::shared_ptr<detail::SocketSession>& session, const boost::system::error_code& ec);

we don't need non-const access to session.

~~~~

300 + template<class ResultMessage>
301 + void invoke(
302 + void (protobuf::DisplayServer::*function)(
303 + ::google::protobuf::RpcController* controller,
304 + const ::mir::protobuf::ConnectParameters* request,
305 + ResultMessage* response,
306 + ::google::protobuf::Closure* done),
307 + mir::protobuf::wire::Invocation const& invocation);

No need for this to be a template - the exact return type (and even the exact function) is known.

~~~~

362 + session->read_next_message();
363 connected_sessions->add(session);
364
365 - session->read_next_message();

I don't believe it makes a difference, but do we need this ordering changed?

~~~~

134 +
135 +

Do we need this whitespace?

review: Needs Fixing

« Back to merge proposal