Mir

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

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

First pass - cosmetic stuff

+/* Returns an allocated MirCookie that you must release with mir_cookie_release
+ *
+ * \params[in] ev The input event
+ * \return An allocated and ref'ed cookie
+ */
+MirCookie const* mir_input_event_get_cookie(MirInputEvent const* ev);

1. The /precondition/ that the event has a cookie is missing.

2. "An allocated and ref'ed cookie" is implementation detail the user isn't interested in.

~~~~

+ * \return The size needed to allocate a buffer
+ */
+size_t mir_cookie_get_size(MirCookie const* cookie);

While allocation is likely what the client will do, surely this is just the size of buffer needed by mir_cookie_copy_to_buffer().

~~~~

+ * \params[in] buffer The allocated buffer to copy the MirCookie into
+ * \params[in] size The size of the allocated buffer
+ */
+void mir_cookie_copy_to_buffer(MirCookie const* cookie, void* buffer, size_t size);

%s/allocated //

~~~~

+MirCookie const* mir_cookie_from_buffer(void const* buffer);

I think it would be useful validation for the buffer size to be supplied and checked. While not bulletproof the it is a reminder to the user that size matters.

~~~~

+* \param [in] cookie The void* from the input event that you want to raise the window from.
+*/
+void mir_surface_raise_with_cookie(MirSurface* surface, MirCookie const* cookie);

The comment and the code disagree

~~~~

+ HMAC_SHA_1_8

Is there a reason for the non standard naming style? https://unity.ubuntu.com/mir/cppguide/index.html?showone=Enumerator_Names#Enumerator_Names

review: Needs Fixing

« Back to merge proposal