Mir

Code review comment for lp:~nick-dedekind/mir/trusted_sessions

Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

> There's a lot of this to digest - and I don't know what feature(s) it is
> supposed to support.

Thanks for the review.
This adds/implements the API required for client & server to manage "trust sessions" (as the name suggests).
The easiest way to get an overview of what this code "does" is by looking at the diagram:
https://docs.google.com/a/canonical.com/drawings/d/1Sx3Q6IugYY0DjrQfN3em6PBhyeNotXtYAa4UioVdT8c/edit

>
> Accordingly, I get bogged down in details like:
>
> @SessionId - why is this useful (compared to, say,
> std::[shared|weak]_ptr<Session>)

:/ This was a bit of a mixed implementation. The client side supports multiple trust sessions per connection, while the server does not. So [at least for now] I've removed support for multiple trust sessions (start refused). So removed the SessionId in favour of std::shared_ptr<TrustSession>

>
> @TrustedSessionCreationParameters - this has one public mutable member (which
> initializes itself). Why does it need a constructor? (And does it really
> belong in the shell namespace?)
>
> 61 + * Copyright © 2012 Canonical Ltd.
>
> Wrong year.
>
> @examples/trusted_session_trusted_session.c
>
> This is written using the original (async) APIs it would probably be clearer
> using the (slightly) more recent sync ones. (I suspect cut & paste from an
> example that should have been updated last year.)

Refactored example & added sync methods to trust session API.

« Back to merge proposal