Mir

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

Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

> not too flummoxed about use of void*, especially if you & Chris gave it more
> thought than I had behind my comment. Some other things that I'm not too
> flummoxed about (esp given that (as I understand) this is needed in short
> order)
>
> + auto const* pev = mir_input_event_get_pointer_event(ev);
> no need for * (a few other places)

I was mainly doing that so you can see its a pointer (I like seeing types sometimes), but yeah auto captures that :). Fixed.

>
> + // FIXME Moveing to 160bits soon
> typo
>

Opps. Fixed.

> + void* cookie = malloc(size);
> would be a bit better to use a RAII type in allocation for this test (and
> would eliminate the need for a destructor too)
Yes! I attempted it but ran into issue when I was using the MirCookie* since the size was not known (should have just written my own destructor for the shared_ptr to cast and delete[].) Fixed.

>
> 768: std::mutex mutex;
> A bit strange to have the mutex be public, and require locking it before
> accessing some members. Would be better encapsulated to have functions on the
> object to do that.

Agreed, and it looks much cleaner :). Fixed.

« Back to merge proposal