Code review comment for lp:~mir-team/mir/public-cookie-api

394 +/* Returns an allocated MirCookie that you must release with mir_cookie_release
394 + *
395 + * \params[in] ev The input event
396 + * \return An allocated and ref'ed cookie
397 + */

There's no need to say “allocated” or “ref'ed” here.

394 +/* Returns the MirCookie associated with this input event.
394 + *
395 + * \params[in] ev The input event
396 + * \return A reference to the MirCookie associated with this input event.
397 + * This must be released with a call to mir_cookie_release().
398 + */

Likewise:
415 +/* Turns the user copyed buffer back into a MirCookie, which must be
416 + * released with mir_cookie_release
417 + *
418 + * \params[in] buffer The buffer used to get a MirCookie back
419 + * \return A newely allocated MirCookie
420 + */
421 +MirCookie const* mir_cookie_from_buffer(void const* buffer);

should maybe be:
415 +/* Create a MirCookie from a serialised representation.
417 + *
418 + * \params[in] buffer The buffer containing a serialised MirCookie.
418 + * The buffer may be freed immediately after this call.
419 + * \return A reference to a MirCookie.
418 + * This must be released with a call to mir_cookie_release().
420 + */
421 +MirCookie const* mir_cookie_from_buffer(void const* buffer);

This can never return NULL, right?

Actually: we should probably pass in the length of the buffer here and return NULL if it doesn't match what we expect. Clients will get this passed in from other sources, and it'll courteous to tell them if it's easy to detect that it's wrong.

443 +* \param [in] cookie The void* from the input event that you want to raise the window from.
Ba baw!

1143 + mir_surface_raise_with_cookie;
1144 + mir_input_event_has_cookie;
1145 + mir_cookie_get_size;
1146 + mir_input_event_get_cookie;
1147 + mir_cookie_copy_to_buffer;
1148 + mir_cookie_from_buffer;
1149 + mir_cookie_release;

These should be in MIR_CLIENT_unreleased, not MIR_CLIENT_DETAIL (which is pseudo-private implementation details).

+ if (raw_cookie.size() < cookie_size_from_format(mir::cookie::Format::HMAC_SHA_1_8))

Should be !=, right?

Why did you move mir::SecurityCheckFailed?

2029 + auto const& cookie_ptr = cookie_authority->unmarshall_cookie(cookie_bytes);

I'm surprised this passed valgrind? unmarshall_cookie() returns a std::unique_ptr<>, that you take a *reference* to, and then the std::unique_ptr<> gets *destroyed* at the end of the statement.

Likewise in a bunch of other places. Why doesn't this blow up?

A: http://herbsutter.com/2008/01/01/gotw-88-a-candidate-for-the-most-important-const/. I wonder why people think C++ is needlessly arcane‽.