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

Mostly cosmetic things.

mir::cookie::Array => mir::cookie::Blob

Comment suggestions
---
+
386 +/* Tells if the MirInputEvent has a MirCookie
387 + *
388 + * \params[in] ev The input event
389 + * \return True if the input event has a MirCookie
390 + */

change to:

+
386 +/* Query if an input event contains a cookie
387 + *
388 + * \params[in] ev The input event
389 + * \return True if the input event contains a cookie
390 + */

---

---
/* Returns the MirCookie associated with this input event.
 *
 * \pre The input event must have a MirCookie
 * \params[in] ev The input event
 * \return A reference to the MirCookie associated with this input event
 * This must be released with a call to mir_cookie_release()
 */

 change to:

/* Returns the cookie associated with an input event.
 *
 * \pre The input event must have a MirCookie
 * \params[in] ev An input event
 * \return The cookie associated with the given input event
 * The cookie must be released by calling mir_cookie_release
 */
---

/* The size of the buffer needed to serialize this MirCookie
 *
 * \params[in] cookie The MirCookie
 * \return The size needed for a buffer
 */
size_t mir_cookie_get_size(MirCookie const* cookie);

change to:

/* Queries the size needed to serialize a given cookie
 *
 * \params[in] cookie A cookie instance
 * \return The size of the serialized representation of the given cookie
 */
size_t mir_cookie_size(MirCookie const* cookie);
---

---
/* Copy the MirCookie into an allocated buffer
 *
 * \pre The size must be equal to mir_cookie_get_size
 * \params[in] cookie The MirCookie
 * \params[in] buffer The allocated buffer to copy the MirCookie into
 * \params[in] size The size of the allocated buffer
 */

 change to:

/* Serializes a cookie into the given buffer
 *
 * \pre The size must be equal to mir_cookie_get_size
 * \params[in] cookie A cookie instance
 * \params[in] buffer A buffer which is filled with the serialized representation
                      of the given cookie
 * \params[in] size The size of the given buffer
 */
void mir_cookie_copy_to_buffer(MirCookie const* cookie, void* buffer, size_t size);

To be consistent with mir_cookie_from_buffer
mir_cookie_copy_to_buffer => mir_cookie_to_buffer

---
/* Create a MirCookie from a serialised representation
 *
 * \pre The size must be equal to mir_cookie_get_size
 * \params[in] buffer The buffer containing a serialised MirCookie.
 * The buffer may be freed immediately after this call.
 * \return A reference to a MirCookie.
 * This must be released with a call to mir_cookie_release().
 */
MirCookie const* mir_cookie_from_buffer(void const* buffer, size_t size);

change to:

/* Create a cookie from a serialized representation
 *
 * \pre The size must be equal to mir_cookie_get_size
 * \params[in] buffer The buffer containing a serialized cookie.
 * The buffer may be freed immediately after this call.
 * \return A MirCookie instance. The instance must be released
                      with a call to mir_cookie_release.
 */
---

---
/*
* Attempts to raise the surface based on a keyboard/touch/pointer cookie.
*
* \param [in] surface The surface to operate on.
* \param [in] cookie The MirCookie from an input event.
* \post Sumbittiming an invalid cookie will temrinate your connection.
*/
void mir_surface_raise_with_cookie(MirSurface* surface, MirCookie const* cookie);

change to:

/*
* Attempts to raise the surface to the front.
*
* \param [in] surface The surface to raise
* \param [in] cookie A cookie instance obtained from an input event.
* An invalid cookie will terminate the client connection.
*/
void mir_surface_raise(MirSurface* surface, MirCookie const* cookie);

---

SecurityCheckFailed => SecurityCheckError

This is pre-existing, but:

---

    /**
    * Construction function used to create a CookieAuthority. The secret size must be
    * no less then minimum_secret_size otherwise an exception will be thrown
    *
    * \param [in] secret A filled in secret used to set the key for the hash function
    * \return A unique_ptr CookieAuthority
    */
    static std::unique_ptr<CookieAuthority> create_from_secret(Secret const& secret);

change to (secret in the name is redundant given the parameter type):

/**
    * Construction function used to create a CookieAuthority. The secret size must be
    * no less then minimum_secret_size otherwise an exception will be thrown
    *
    * \param [in] secret A secret used to set the key for the hash function
    * \return A unique_ptr CookieAuthority
    */
    static std::unique_ptr<CookieAuthority> create_from(Secret const& secret);
---

---
    static std::unique_ptr<CookieAuthority> create_saving_secret(Secret& save_secret);

    This api is weird, If you really need the secret, shouldn't it be part of the CookieAuthority interface?
---

---
    /**
    * Construction function used to create a CookieAuthority and a secret which it keeps internally.
    *
    * \return A unique_ptr CookieAuthority
    */
    static std::unique_ptr<CookieAuthority> create_keeping_secret();

change to:

    /**
    * Create a default CookieAuthority instance
    *
    * \return A CookieAuthority instance
    */
    static std::unique_ptr<CookieAuthority> create();
---

---
    /**
    * Creates a cookie from a timestamp.
    *
    * \param [in] timestamp A timestamp
    * \return A cookie instance
    */
    virtual std::unique_ptr<MirCookie> make_cookie(uint64_t const& timestamp) = 0;

change to:
    /**
    * Creates a cookie from a serialized representation
    *
    * \param [in] blob A blob of bytes representing a serialized cookie
    * \return A cookie instance
    */
    virtual std::unique_ptr<MirCookie> make_cookie(std::vector<uint8_t> const& blob) = 0;
---

---

auto new_cookie = new uint8_t[old_ev->motion.cookie.size()];
        memcpy(new_cookie, old_ev->motion.cookie.data(), old_ev->motion.cookie.size());
        return reinterpret_cast<MirCookie*>(new_cookie);
----
I would prefer:

class MirCookie
{
public:
    MirCookie(mir::cookie::blob const& blob);

    void copy_to(void* buffer, size_t size);

private:
    mir::cookie::blob blob;
};

MirCookie::MirCookie(mir::cookie::blob const& blob)
    : blob{blobl}
    {}

MirCookie::copy_to(void* buffer, size_t size)
{
    mir::require(size == mir::cookie::default_blob_size);
    memcpy(buffer, cookie, size);
}

auto cookie = new MirCookie(old_ev->motion.cookie);
return cookie;

---
mir::cookie::array_size => mir::cookie::default_blob_size;
---

---
void mir_cookie_copy_to_buffer(MirCookie const* cookie, void* buffer, size_t size)
{
    mir::require(size == mir::cookie::array_size);
    memcpy(buffer, cookie, size);
}
---

---
marsharlled_cookie => marshalled_cookie