Mir

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

Revision history for this message
Chris Halse Rogers (raof) wrote :

The Story Of The Void*:
Brandon and I had a *long* back and forth about client API wrt MirCookie, ranging from having a MirCookie type that can be converted into a MirBlob, to essentially duplicating MirBlob's API against a MirCookie, to a non-opaque struct MirCookie { size_t length, void* data };

Neither of us were particularly happy with any of them.

The void* API here is (a) transparent - it's clear to everyone that they've just got a blob of data, (b) quick to implement, and (c) is minimally constraining for future, better, implementations with better APIs.

General comment: I'm pretty sure you don't need reinterpret_cast<> to go between void* and MirCookie*; static_cast<MirCookie*> should be sufficient. (Wheras you *do* need to reinterpret_cast between uint8_t* and uint64_t*)

42 + * \params[in] ev The input event
43 + * \params[in] cookie An allocated void* with exactly cookie_size bytes
44 + * \params[in] size The size of the MirCookie
45 + */
46 +void mir_input_event_copy_cookie(MirInputEvent const* ev, void* cookie, size_t size);

Two things: do we need to pass size in here? We're only going to assert that it's exactly mir_input_event_get_cookie_size(), right?

And: If we do keep the size, you need to update the comment; no MirCookie here ☺.

Also a couple of inline comments.

review: Needs Fixing

« Back to merge proposal