Linux sandbox: refactor BrokerProcess class

sandbox/linux/syscall_broker/broker_common.h

+// Copyright 2014 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.
+
+#ifndef SANDBOX_LINUX_SYSCALL_BROKER_BROKER_COMMON_H_
+#define SANDBOX_LINUX_SYSCALL_BROKER_BROKER_COMMON_H_
+
+#include <fcntl.h>
+#include <stdint.h>

[mdempsky, 2014/10/31 22:13:43]

size_t is in <stddef.h>, not <stdint.h>

[jln (very slow on Chromium), 2014/10/31 23:15:49]

Done.

+
+#include <string>

[mdempsky, 2014/10/31 22:13:43]

These aren't needed I think.

[jln (very slow on Chromium), 2014/10/31 23:15:49]

Done.

+#include <vector>
+
+namespace sandbox {
+
+namespace syscall_broker {
+
+static const size_t kMaxMessageLength = 4096;

[mdempsky, 2014/10/31 22:13:43]

Probably don't want 'static' if you're putting these in a header file?

[jln (very slow on Chromium), 2014/10/31 23:15:49]

Done.

+
+// Some flags are local to the current process and cannot be sent over a Unix
+// socket. They need special treatment from the client.
+// O_CLOEXEC is tricky because in theory another thread could call execve()
+// before special treatment is made on the client, so a client needs to call
+// recvmsg(2) with MSG_CMSG_CLOEXEC.
+// To make things worse, there are two CLOEXEC related flags, FD_CLOEXEC (see
+// F_GETFD in fcntl(2)) and O_CLOEXEC (see F_GETFL in fcntl(2)). O_CLOEXEC
+// doesn't affect the semantics on execve(), it's merely a note that the
+// descriptor was originally opened with O_CLOEXEC as a flag. And it is sent
+// over unix sockets just fine, so a receiver that would (incorrectly) look at
+// O_CLOEXEC instead of FD_CLOEXEC may be tricked in thinking that the file
+// descriptor will or won't be closed on execve().
+static const int kCurrentProcessOpenFlagsMask = O_CLOEXEC;
+
+enum IPCCommands {
+  kCommandInvalid = 0,

[mdempsky, 2014/10/31 22:13:43]

Chromium style says enums use SHOUTY_CASE, though I know this is an existing
style violation.  If you do change it, it might be worth making it:

    enum class IPCCommand {
      INVALID = 0,
      OPEN,
      ACCESS,
    };

since the "Command" part becomes redundant when you have to write it
"IPCCommand::OPEN" anyway.

[jln (very slow on Chromium), 2014/10/31 23:15:49]

Done, but I haven't converted to an enum class: unfortunately I'm deserializing
an int from the IPC channel and I would need to explicitly set-up serialization
for this enum instead.

+  kCommandOpen,
+  kCommandAccess,
+};
+
+}  // namespace syscall_broker
+
+}  // namespace sandbox
+
+#endif  // SANDBOX_LINUX_SYSCALL_BROKER_BROKER_COMMON_H_