C types and utilities to validate and access struct and message headers

mojo/public/c/bindings/lib/message.c

Index: mojo/public/c/bindings/lib/message.c
diff --git a/mojo/public/c/bindings/lib/message.c b/mojo/public/c/bindings/lib/message.c
new file mode 100644
index 0000000000000000000000000000000000000000..4875fdb5b31e74ba5e0abb03d266265a9b7d3ed7
--- /dev/null
+++ b/mojo/public/c/bindings/lib/message.c
@@ -0,0 +1,54 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "mojo/public/c/bindings/message.h"
+
+_Static_assert(sizeof(mojo_message_header_t) == 16u,
+               "mojo_message_header_t should be 16 bytes");
+
+_Static_assert(sizeof(mojo_message_header_with_request_id_t) == 24u,
+               "mojo_message_header_t should be 24 bytes");
+
+#define MESSAGE_EXPECTS_RESPONSE (1 << 0u)

[viettrungluu, 2016/02/02 00:48:47]

Probably these should be in the header (probably with more elaborate names).

[jamesr, 2016/02/02 00:55:03]

Dumping a macro in a header sucks but I guess there isn't a great way around
this in C.  Can do.

+#define MESSAGE_IS_RESPONSE (1 << 1u)
+
+bool mojo_validate_message_header(mojo_struct_header_t* header, uint32_t size) {
+  if (header->num_bytes < sizeof(mojo_message_header_t) ||
+      size < sizeof(mojo_message_header_t)) {
+    return false;
+  }
+
+  mojo_message_header_t* message_header = (mojo_message_header_t*)header;
+  // Message expects response and message is response flags are mutually
+  // exclusive.
+  if ((message_header->flags & MESSAGE_EXPECTS_RESPONSE) &&
+      (message_header->flags & MESSAGE_IS_RESPONSE)) {
+    return false;
+  }
+
+  if (header->version == 0u) {
+    if (header->num_bytes != sizeof(mojo_message_header_t)) {
+      return false;
+    }
+
+    // Version 0 has no request id and should not have either of these flags.
+    if ((message_header->flags & MESSAGE_EXPECTS_RESPONSE) ||
+        (message_header->flags & MESSAGE_IS_RESPONSE)) {
+      return false;
+    }
+    return true;

[viettrungluu, 2016/02/02 00:48:47]

I'd skip the return true here, if you're going to have an "else (if)" (with the
return true at the end).

[jamesr, 2016/02/02 00:55:03]

d'oh, will do

+  } else if (header->version == 1u) {
+    if (header->num_bytes != sizeof(mojo_message_header_with_request_id_t)) {
+      return false;
+    }
+
+    // Version 1 has a request id and should have one of these flags set.

[viettrungluu, 2016/02/02 00:48:47]

Is this really what we do in the C++ bindings?

[jamesr, 2016/02/02 00:55:03]

No we don't
(https://github.com/domokit/mojo/blob/master/mojo/public/cpp/bindings/lib/mess...)
but I'm not sure how anyone is supposed to interpret a message with a request ID
that doesn't set one of these flags.  Maybe this should just be accepted?

[viettrungluu, 2016/02/02 18:19:16]

Yes. The reason is that the "version" is not a "type". One should be able to
always send the latest version of a struct (in this case, the header struct) if
one wishes. Sending a version 0 struct in the
not-expecting-response/non-response case is just an optimization.

+    if (!((message_header->flags & MESSAGE_EXPECTS_RESPONSE) ||
+          (message_header->flags & MESSAGE_IS_RESPONSE))) {
+      return false;
+    }
+  }

[viettrungluu, 2016/02/02 00:48:47]

What if header->version > 1?

[jamesr, 2016/02/02 00:55:03]

Those deliberately validate as I don't know what to check for.  See
https://github.com/domokit/mojo/blob/master/mojo/public/cpp/bindings/lib/mess....
 Maybe I should duplicate that comment

+
+  return true;
+}