Add multi_process_lock to chrome/common

base/shared_mutex_linux.cc

+// Copyright (c) 2010 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 "base/shared_mutex.h"
+
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <unistd.h>
+
+#include "base/logging.h"

[agl, 2010/11/08 23:57:49]

include "base/eintr_wrapper.h"

[dmac, 2010/11/11 21:47:59]

Done.

+
+namespace base {
+
+SharedMutex::SharedMutex(const std::string& name) : fd_(-1), name_(name) {
+}

[Mark Mentovai, 2010/11/09 17:43:14]

Blank line before, and |}  // namespace base|.

+
+bool SharedMutex::TryLock() {
+  struct sockaddr_un address;
+  address.sun_family = AF_UNIX;
+  // Add 1 for the \0 that goes at the front to make it an abstract name port.
+  size_t address_length = sizeof(address.sun_family) + name_.length() + 1;
+  size_t max_length = sizeof(address.sun_path);

[agl, 2010/11/08 23:57:49]

I think this bit gets simpler if you just do
if (name_.size() + 1 > sizeof(address.sun_path))

[dmac, 2010/11/11 21:47:59]

Done.

+  if (snprintf(address.sun_path, max_length, "#%s", name_.c_str())

[agl, 2010/11/08 23:57:49]

IMPORTANT: do this:
memset(address.sun_path, 0, sizeof(address.sun_path));
memcpy(&address.sun_path[1], name_.data(), name_.size());

(the code, as is, silently fails.)

[dmac, 2010/11/11 21:47:59]

Done.

+      >= static_cast<int>(max_length)) {

[Mark Mentovai, 2010/11/09 17:43:14]

I think it’s way more usual for the >= to be at the end of the preceding line.

[dmac, 2010/11/11 21:47:59]

Done.

+    NOTREACHED() << "Socket name to long";

[Mark Mentovai, 2010/11/09 17:43:14]

What, now? to?

[dmac, 2010/11/11 21:47:59]

Done.

+    return false;
+  }
+  // Set up the abstract name port.
+  address.sun_path[0] = 0;

[agl, 2010/11/08 23:57:49]

drop this line

[dmac, 2010/11/11 21:47:59]

Done.

+  int socket_fd = socket(PF_UNIX, SOCK_STREAM, 0);

[agl, 2010/11/08 23:57:49]

AF_UNIX, not PF_UNIX

[dmac, 2010/11/11 21:47:59]

Done.

+  if (socket_fd < 0) {
+    NOTREACHED() << "Couldn't create socket";

[Mark Mentovai, 2010/11/09 17:43:14]

For failling sysaclls, you can use [D]PLOG to be sure that the errno is
reported.

[dmac, 2010/11/11 21:47:59]

Done.

+    return false;
+  }
+  if (bind(socket_fd,
+           reinterpret_cast<sockaddr *>(&address),
+           address_length) == 0) {

[agl, 2010/11/08 23:57:49]

sizeof(address), not address_length

[dmac, 2010/11/11 21:47:59]

Done.

+    fd_ = socket_fd;
+  }

[agl, 2010/11/08 23:57:49]

All the other conditionals are failure tests, then suddenly you turn it inside
out. I think this should also be a failure test with:
  NOTREACHED() << ...
  HANDLE_EINTR(close(socket_fd));
  return false;
}

fd_ = socket_fd;
return true;

[dmac, 2010/11/11 21:47:59]

Done.

+  return fd_ != -1;
+}
+
+void SharedMutex::Unlock() {
+  close(fd_);

[agl, 2010/11/08 23:57:49]

HANDLE_EINTR(close(fd_));

[Mark Mentovai, 2010/11/09 17:43:14]

This should be wrapped in if (fd_ != -1) to avoid having close set errno to
EBADF on, say, destruction.

[dmac, 2010/11/11 21:47:59]

Done.

[dmac, 2010/11/11 21:47:59]

Done.

+  fd_ = -1;
+}
+
+}  // namespace base