Mir

Code review comment for lp://qastaging/~mir-team/mir/160bit-hash-mac

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

Mostly style nits:

+ event_cookie->set_mac(reinterpret_cast<char const*>(cookie.mac), sizeof(cookie.mac));

reinterpret_cast<> should be the penultimate resort to do something non-portable. That isn't the case here as *no cast is required*.

~~~~

+ std::array<uint8_t, 20> mac;
             int32_t deviceId;
             int32_t source;
             int32_t action;
@@ -81,7 +81,7 @@
         struct Motion {
             uint32_t seq;
             int64_t eventTime;
- uint64_t mac;
+ std::array<uint8_t, 20> mac;

mir::cookie::Mac

~~~~

std::begin(cookie.mac) + sizeof(cookie.mac)

can be written

std::end(cookie.mac)

~~~~

+ mir::cookie::Mac mac;
+ auto cookie = cookie_factory->timestamp_to_cookie(timestamp.count());
+ std::copy(std::begin(cookie.mac), std::begin(cookie.mac) + sizeof(cookie.mac), mac.begin());

Why stick the "auto cookie..." line between creating and initializing mac?

~~~~

=== modified file 'src/client/input/input_event.cpp'
...
+#include <string.h>
+

Not needed.

~~~~

=== modified file 'src/server/input/default_event_builder.cpp'
...
+#include <string.h>

Not needed.

~~~~

Finally, just a (non-blocking) comment:

+ std::copy(old_kev.mac.begin(), old_kev.mac.end(), std::begin(cookie.mac));

could be written as

    std::copy(begin(old_kev.mac), end(old_kev.mac), std::begin(cookie.mac));

review: Needs Fixing

« Back to merge proposal