dbus: add support for passing file descriptors

dbus/message.cc

Index: dbus/message.cc
diff --git a/dbus/message.cc b/dbus/message.cc
index 43fb3bb8b593718b6fc01961c1f97a705f66fde8..1d2fa032e4cf040286e73df8698d8ef3aada6dd4 100644
--- a/dbus/message.cc
+++ b/dbus/message.cc
@@ -9,6 +9,7 @@
 #include "base/basictypes.h"
 #include "base/format_macros.h"
 #include "base/logging.h"
+#include "base/platform_file.h"
 #include "base/stringprintf.h"
 #include "dbus/object_path.h"
 #include "third_party/protobuf/src/google/protobuf/message_lite.h"
@@ -199,6 +200,16 @@ std::string Message::ToStringInternal(const std::string& indent,
         output += ToStringInternal(indent + "  ", &sub_reader);
         break;
       }
+      case UNIX_FD: {
+        CHECK(kDBusTypeUnixFdIsSupported);
+
+        FileDescriptor value;

[satorux1, 2012/03/28 20:49:37]

nit: value -> file_descriptor?

[Sam Leffler, 2012/03/28 22:06:33]

Done.

+        if (!reader->PopFileDescriptor(&value))
+          return kBrokenMessage;
+        output += indent + "fd#" +
+                  base::StringPrintf("%u", value.value()) + "\n";

[satorux1, 2012/03/28 20:49:37]

nit: %d? fd is an int.

[Sam Leffler, 2012/03/28 22:06:33]

Done.

+        break;
+      }
       default:
         LOG(FATAL) << "Unknown type: " << type;
     }
@@ -677,6 +688,17 @@ void MessageWriter::AppendVariantOfBasic(int dbus_type, const void* value) {
   CloseContainer(&variant_writer);
 }
 
+void MessageWriter::AppendFileDescriptor(const FileDescriptor& value) {
+  CHECK(kDBusTypeUnixFdIsSupported);
+
+  base::PlatformFileInfo info;
+  int fd = value.value();
+  bool ok = base::GetPlatformFileInfo(fd, &info);
+  if (!ok || info.is_directory)

[satorux1, 2012/03/28 20:49:37]

These are different errors. !ok means it just failed to get the file info, which
likely means the fd is invalid. LOG(FATAL) in this case is rather questionable.

Trying to send an invalid fd is a bad idea, but it's the caller's fault, so I'd
go with something like:

if (ok && info.is_directory)
  LOG(FATAL) << "Attempt to pass invalid file descriptor";
// |ok| can be false, which means |fd| is invalid, but it's caller's fault,
// so we don't stop.

[Jorge Lucangeli Obes, 2012/03/28 21:33:37]

And in this case, we can display an error message specifically pointing out the
fact that this is a dir fd, and we don't want to send those around.

[Sam Leffler, 2012/03/28 22:06:33]

Yes they are different errors; the message is intentionally general to cover
both cases.  I think it is a bad idea to carry on with a known bad file
descriptor even though it is the fault of the caller.  I expect the dbus library
will abort if handed an invalid descriptor but there's no guarantee and if it
continues on then I can imaging difficult-to-debug problems that can be
trivially caught here.  I can spend some more code to print out the fd or treat
the two cases separately but not catching it here seems bad.

+    LOG(FATAL) << "Attempt to pass invalid file descriptor";

[satorux1, 2012/03/28 20:49:37]

Please add some comment why we are disallowing the directory fd passing.
otherwise, it looks rather cryptic

[Sam Leffler, 2012/03/28 22:06:33]

Done.

+  AppendBasic(DBUS_TYPE_UNIX_FD, &fd);
+}
+
 //
 // MessageReader implementation.
 //
@@ -936,4 +958,23 @@ bool MessageReader::PopVariantOfBasic(int dbus_type, void* value) {
   return variant_reader.PopBasic(dbus_type, value);
 }
 
+bool MessageReader::PopFileDescriptor(FileDescriptor* value) {
+  CHECK(kDBusTypeUnixFdIsSupported);
+
+  int fd;

[satorux1, 2012/03/28 20:49:37]

nit: initialize it to -1 to be extra defensive.

[Sam Leffler, 2012/03/28 22:06:33]

Done.

+  const bool success = PopBasic(DBUS_TYPE_UNIX_FD, &fd);
+  if (!success)
+    return false;
+
+  base::PlatformFileInfo info;
+  bool ok = base::GetPlatformFileInfo(fd, &info);
+  if (!ok || info.is_directory) {

[satorux1, 2012/03/28 20:49:37]

ditto. these are different errors.

[Sam Leffler, 2012/03/28 22:06:33]

see above

+    base::ClosePlatformFile(fd);
+    LOG(FATAL) << "Attempt to receive invalid file descriptor";
+    return false;  // NB: not reached
+  }
+  value->PutValue(fd);
+  return true;
+}
+
 }  // namespace dbus