Implement classes used for manipulating D-Bus messages.

dbus/message.cc

+// Copyright (c) 2011 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 "dbus/message.h"
+
+#include "base/basictypes.h"
+#include "base/format_macros.h"
+#include "base/logging.h"
+#include "base/stringprintf.h"
+
+namespace dbus {
+
+Message::Message()
+    : raw_message_(NULL) {
+}
+
+Message::~Message() {
+  if (raw_message_)
+    dbus_message_unref(raw_message_);
+}
+
+Message::MessageType Message::GetMessageType() {
+  if (!raw_message_)
+    return MESSAGE_INVALID;
+  const int type = dbus_message_get_type(raw_message_);
+  return static_cast<Message::MessageType>(type);
+}
+
+void Message::reset_raw_message(DBusMessage* raw_message) {
+  if (raw_message_)
+    dbus_message_unref(raw_message_);
+  raw_message_ = raw_message;
+}
+
+std::string Message::ToStringInternal(const std::string& indent,
+                                      MessageReader* reader) {
+  std::string output;
+  while (reader->HasMore()) {
+    const DataType type = reader->GetDataType();
+    switch (type) {
+      case BYTE: {
+        uint8 value = 0;
+        CHECK(reader->PopByte(&value)) << "Broken message";

[stevenjb, 2011/07/27 17:56:26]

I don't think we should force a crash on a failed message parse. Instead we
should take a string* as an output argument and return 'false' on error so that
the calling code can decide whether to crash or continue. 

[satorux1, 2011/07/27 21:20:35]

Good point. The function is mainly for debugging, so having ability to do
something like LOG(INFO) << message.ToString() is desirable.

But you are right that crashing is not a good idea. I changed it so it won't
crash.

+        output += indent + "byte " + base::StringPrintf("%d", value) + "\n";
+        break;
+      }
+      case BOOL: {
+        bool value = false;
+        CHECK(reader->PopBool(&value)) << "Broken message";
+        output += indent + "bool " + (value ? "true" : "false") + "\n";
+        break;
+      }
+      case INT16: {
+        int16 value = 0;
+        CHECK(reader->PopInt16(&value)) << "Broken message";
+        output += indent + "int16 " + base::StringPrintf("%d", value) + "\n";
+        break;
+      }
+      case UINT16: {
+        uint16 value = 0;
+        CHECK(reader->PopUint16(&value)) << "Broken message";
+        output += indent + "uint16 " + base::StringPrintf("%d", value) + "\n";
+        break;
+      }
+      case INT32: {
+        int32 value = 0;
+        CHECK(reader->PopInt32(&value)) << "Broken message";
+        output += indent + "int32 " + base::StringPrintf("%d", value) + "\n";
+        break;
+      }
+      case UINT32: {
+        uint32 value = 0;
+        CHECK(reader->PopUint32(&value)) << "Broken message";
+        output += indent + "uint32 " + base::StringPrintf("%u", value) + "\n";
+        break;
+      }
+      case INT64: {
+        int64 value = 0;
+        CHECK(reader->PopInt64(&value)) << "Broken message";
+        output += (indent + "int64 " +
+                   base::StringPrintf("%" PRId64, value) + "\n");
+        break;
+      }
+      case UINT64: {
+        uint64 value = 0;
+        CHECK(reader->PopUint64(&value)) << "Broken message";
+        output += (indent + "uint64 " +
+                   base::StringPrintf("%" PRIu64, value) + "\n");
+        break;
+      }
+      case DOUBLE: {
+        double value = 0;
+        CHECK(reader->PopDouble(&value)) << "Broken message";
+        output += indent + "double " + base::StringPrintf("%f", value) + "\n";
+        break;
+      }
+      case STRING: {
+        std::string value;
+        CHECK(reader->PopString(&value)) << "Broken message";
+        output += indent + "string \"" + value + "\"\n";
+        break;
+      }
+      case OBJECT_PATH: {
+        std::string value;
+        CHECK(reader->PopObjectPath(&value)) << "Broken message";
+        output += indent + "object_path \"" + value + "\"\n";
+        break;
+      }
+      case ARRAY: {
+        MessageReader sub_reader(this);
+        CHECK(reader->PopArray(&sub_reader)) << "Broken message";
+        output += indent + "array [\n";
+        output += ToStringInternal(indent + "  ", &sub_reader);
+        output += indent + "]\n";
+        break;
+      }
+      case STRUCT: {
+        MessageReader sub_reader(this);
+        CHECK(reader->PopStruct(&sub_reader)) << "Broken message";
+        output += indent + "struct {\n";
+        output += ToStringInternal(indent + "  ", &sub_reader);
+        output += indent + "}\n";
+        break;
+      }
+      case DICT_ENTRY: {
+        MessageReader sub_reader(this);
+        CHECK(reader->PopDictEntry(&sub_reader)) << "Broken message";
+        output += indent + "dict entry {\n";
+        output += ToStringInternal(indent + "  ", &sub_reader);
+        output += indent + "}\n";
+        break;
+      }
+      case VARIANT: {
+        MessageReader sub_reader(this);
+        CHECK(reader->PopVariant(&sub_reader)) << "Broken message";
+        output += indent + "variant ";
+        output += ToStringInternal(indent + "  ", &sub_reader);
+        break;
+      }
+      default:
+        LOG(FATAL) << "Unknown type: " << type;
+    }
+  }
+  return output;
+}
+
+std::string Message::ToString() {
+  std::string output;
+  MessageReader reader(this);
+  return ToStringInternal("", &reader);
+}
+
+//
+// MethodCall implementation.
+//
+
+MethodCall::MethodCall(const std::string& interface_name,
+                       const std::string& method_name)
+    : Message(),
+      interface_name_(interface_name),
+      method_name_(method_name) {
+  reset_raw_message(dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_CALL));
+
+  bool success = false;

[stevenjb, 2011/07/27 17:56:26]

nit: don't need to initialize success here.

[satorux1, 2011/07/27 21:20:35]

Done.

+  success = dbus_message_set_interface(raw_message(), interface_name.c_str());
+  DCHECK(success) << "Unable to allocate memory";
+
+  success = dbus_message_set_member(raw_message(), method_name.c_str());
+  DCHECK(success) << "Unable to allocate memory";
+}
+
+void MethodCall::SetServiceName(const std::string& service_name) {
+  const bool success = dbus_message_set_destination(raw_message(),
+                                                    service_name.c_str());
+  DCHECK(success) << "Unable to allocate memory";
+}
+
+void MethodCall::SetObjectPath(const std::string& object_path) {
+  const bool success = dbus_message_set_path(raw_message(),
+                                             object_path.c_str());
+  DCHECK(success) << "Unable to allocate memory";
+}
+
+//
+// Response implementation.
+//
+
+Response::Response() : Message() {
+}
+
+//
+// MessageWriter implementation.
+//
+
+MessageWriter::MessageWriter(Message* message) :
+    message_(message),
+    container_is_open_(false) {
+  dbus_message_iter_init_append(message_->raw_message(), &raw_message_iter_);
+}
+
+MessageWriter::~MessageWriter() {
+}
+
+void MessageWriter::AppendByte(uint8 value) {
+  DCHECK(!container_is_open_);
+
+  const bool success = dbus_message_iter_append_basic(
+      &raw_message_iter_, DBUS_TYPE_BYTE, &value);
+  // dbus_message_iter_append_basic() fails only when there is not enough
+  // memory. We don't return this error as there is nothing we can do when
+  // it fails to allocate memory for a byte.
+  DCHECK(success) << "Unable to allocate memory";

[stevenjb, 2011/07/27 17:56:26]

Why not use AppendBasic here?

[satorux1, 2011/07/27 21:20:35]

Good catch. It was just an oversight. Fixed!

+}
+
+void MessageWriter::AppendBool(bool value) {
+  AppendBasic(DBUS_TYPE_BOOLEAN, &value);
+}
+
+void MessageWriter::AppendInt16(int16 value) {
+  AppendBasic(DBUS_TYPE_INT16, &value);
+}
+
+void MessageWriter::AppendUint16(uint16 value) {
+  AppendBasic(DBUS_TYPE_UINT16, &value);
+}
+
+void MessageWriter::AppendInt32(int32 value) {
+  AppendBasic(DBUS_TYPE_INT32, &value);
+}
+
+void MessageWriter::AppendUint32(uint32 value) {
+  AppendBasic(DBUS_TYPE_UINT32, &value);
+}
+
+void MessageWriter::AppendInt64(int64 value) {
+  AppendBasic(DBUS_TYPE_INT64, &value);
+}
+
+void MessageWriter::AppendUint64(uint64 value) {
+  AppendBasic(DBUS_TYPE_UINT64, &value);
+}
+
+void MessageWriter::AppendDouble(double value) {
+  AppendBasic(DBUS_TYPE_DOUBLE, &value);
+}
+
+void MessageWriter::AppendString(const std::string& value) {
+  const char* pointer = value.c_str();
+  AppendBasic(DBUS_TYPE_STRING, &pointer);
+  // It may make sense to return an error here, as the input string can be
+  // large. If needed, we could add something like
+  // bool AppendStringWithErrorChecking().
+}
+
+void MessageWriter::AppendObjectPath(const std::string& value) {
+  const char* pointer = value.c_str();
+  AppendBasic(DBUS_TYPE_OBJECT_PATH, &pointer);
+}
+
+void MessageWriter::OpenArray(const std::string& signature,
+                              MessageWriter* writer) {
+  DCHECK(!container_is_open_);
+
+  const bool success = dbus_message_iter_open_container(
+      &raw_message_iter_,
+      DBUS_TYPE_ARRAY,
+      signature.c_str(),
+      &writer->raw_message_iter_);
+  DCHECK(success) << "Unable to allocate memory";
+  container_is_open_ = true;
+}
+
+void MessageWriter::OpenVariant(const std::string& signature,
+                                MessageWriter* writer) {
+  DCHECK(!container_is_open_);
+
+  const bool success = dbus_message_iter_open_container(
+      &raw_message_iter_,
+      DBUS_TYPE_VARIANT,
+      signature.c_str(),
+      &writer->raw_message_iter_);
+  DCHECK(success) << "Unable to allocate memory";
+  container_is_open_ = true;
+}
+
+void MessageWriter::OpenStruct(MessageWriter* writer) {
+  DCHECK(!container_is_open_);
+
+  const bool success = dbus_message_iter_open_container(
+      &raw_message_iter_,
+      DBUS_TYPE_STRUCT,
+      NULL,  // Signature should be NULL.
+      &writer->raw_message_iter_);
+  DCHECK(success) << "Unable to allocate memory";
+  container_is_open_ = true;
+}
+
+void MessageWriter::OpenDictEntry(MessageWriter* writer) {
+  DCHECK(!container_is_open_);
+
+  const bool success = dbus_message_iter_open_container(
+      &raw_message_iter_,
+      DBUS_TYPE_DICT_ENTRY,
+      NULL,  // Signature should be NULL.
+      &writer->raw_message_iter_);
+  DCHECK(success) << "Unable to allocate memory";
+  container_is_open_ = true;
+}
+
+void MessageWriter::CloseContainer(MessageWriter* writer) {
+  DCHECK(container_is_open_);
+
+  const bool success = dbus_message_iter_close_container(
+      &raw_message_iter_, &writer->raw_message_iter_);
+  DCHECK(success) << "Unable to allocate memory";
+  container_is_open_ = false;
+}
+
+void MessageWriter::AppendArrayOfBytes(const std::vector<uint8>& bytes) {
+  DCHECK(!container_is_open_);
+  MessageWriter array_writer(message_);
+  OpenArray("y", &array_writer);
+  // For some reason, dbus_message_iter_append_fixed_array() needs to take
+  // &(bytes.data()) instead of bytes.data(). &(bytes.data()) doesn't
+  // compile so we create a pointer here.
+  const uint8* pointer = bytes.data();

[stevenjb, 2011/07/27 17:56:26]

I don't think data() is actually a standard member of vector<>? Generally this
would just be &bytes[0].

If we provide two interfaces here, one taking a const uint8* + length, then the
vector<> version can just call the uint8* version.

[satorux1, 2011/07/27 21:20:35]

Changed the interface to take uint8* + length.

+  const bool success = dbus_message_iter_append_fixed_array(
+      &(array_writer.raw_message_iter_),
+      DBUS_TYPE_BYTE,
+      &pointer,
+      static_cast<int>(bytes.size()));
+  DCHECK(success) << "Unable to allocate memory";
+  CloseContainer(&array_writer);
+}
+
+void MessageWriter::AppendVariantOfByte(uint8 value) {
+  AppendVariantOfBasic(DBUS_TYPE_BYTE, &value);
+}
+
+void MessageWriter::AppendVariantOfBool(bool value) {
+  AppendVariantOfBasic(DBUS_TYPE_BOOLEAN, &value);
+}
+
+void MessageWriter::AppendVariantOfInt16(int16 value) {
+  AppendVariantOfBasic(DBUS_TYPE_INT16, &value);
+}
+
+void MessageWriter::AppendVariantOfUint16(uint16 value) {
+  AppendVariantOfBasic(DBUS_TYPE_UINT16, &value);
+}
+
+void MessageWriter::AppendVariantOfInt32(int32 value) {
+  AppendVariantOfBasic(DBUS_TYPE_INT32, &value);
+}
+
+void MessageWriter::AppendVariantOfUint32(uint32 value) {
+  AppendVariantOfBasic(DBUS_TYPE_UINT32, &value);
+}
+
+void MessageWriter::AppendVariantOfInt64(int64 value) {
+  AppendVariantOfBasic(DBUS_TYPE_INT64, &value);
+}
+
+void MessageWriter::AppendVariantOfUint64(uint64 value) {
+  AppendVariantOfBasic(DBUS_TYPE_UINT64, &value);
+}
+
+void MessageWriter::AppendVariantOfDouble(double value) {
+  AppendVariantOfBasic(DBUS_TYPE_DOUBLE, &value);
+}
+
+void MessageWriter::AppendVariantOfString(const std::string& value) {
+  const char* pointer = value.c_str();
+  AppendVariantOfBasic(DBUS_TYPE_STRING, &pointer);
+}
+
+void MessageWriter::AppendVariantOfObjectPath(const std::string& value) {
+  const char* pointer = value.c_str();
+  AppendVariantOfBasic(DBUS_TYPE_OBJECT_PATH, &pointer);
+}
+
+void MessageWriter::AppendBasic(int dbus_type, const void* value) {
+  DCHECK(!container_is_open_);
+
+  const bool success = dbus_message_iter_append_basic(
+      &raw_message_iter_, dbus_type, value);
+  // dbus_message_iter_append_basic() fails only when there is not enough
+  // memory. We don't return this error as there is nothing we can do when
+  // it fails to allocate memory for a byte.

[stevenjb, 2011/07/27 17:56:26]

We should either pass an error up for all of these so that the calling function
knows that it failed and can decide whether to continue or abort, or do a CHECK
here and force a crash if we are certain this will only fail on an unrecoverable
out of memory error.

[satorux1, 2011/07/27 21:20:35]

Changed it to CHECK(). We could propagate the error to the caller, but as
mentioned in the comment, there is almost nothing we can do when we cannot
allocate a very small chunk of memory for a single byte, etc. This is why I put
DCHECK() for out-of-memory errors from dbus function calls.

For out of memory like this, our code just crashes. For instance we don't use
exceptions in C++, so pieces of code like "new Foo" and "v.push_back(123)"  just
crash in case of out of memory.

+  DCHECK(success) << "Unable to allocate memory";
+}
+
+void MessageWriter::AppendVariantOfBasic(int dbus_type, const void* value) {
+  const std::string signature = base::StringPrintf("%c", dbus_type);
+  MessageWriter variant_writer(message_);
+  OpenVariant(signature, &variant_writer);
+  variant_writer.AppendBasic(dbus_type, value);
+  CloseContainer(&variant_writer);

[stevenjb, 2011/07/27 17:56:26]

See my comment in the header about writers for arrays. If we don't want
MessageWriter to manage its sub writer, we should at least provide
VariantMessageWriter (and ArrayMessageWriter, etc) that call open in the
constructor and close on destruction.

[satorux1, 2011/07/27 21:20:35]

Thank you for the suggestion. This style is consistent with PopArray() etc. so
I'd rather like to keep it this way.

Besides, close on destruction may be a bit error-prone, as we may forget to
scope the sub writers properly:

scoped_ptr<SubWriter> array_writer = writer.OpenArray(...)
array_writer.AppendString("foo");
...
// This code is wrong.
// Before opening a new sub writer from the same parent
// we should call the destructor of the former sub writer.
scoped_ptr<SubWriter> struct_writer = writer.OpenStruct(...)

+}
+
+//
+// MessageReader implementation.
+//
+
+MessageReader::MessageReader(Message* message)
+    : message_(message) {
+  dbus_message_iter_init(message_->raw_message(), &raw_message_iter_);
+}
+
+
+MessageReader::~MessageReader() {
+}
+
+bool MessageReader::HasMore() {
+  const int dbus_type = dbus_message_iter_get_arg_type(&raw_message_iter_);
+  return dbus_type != DBUS_TYPE_INVALID;
+}
+
+bool MessageReader::PopByte(uint8* value) {
+  return PopBasic(DBUS_TYPE_BYTE, value);
+}
+
+bool MessageReader::PopBool(bool* value) {
+  return PopBasic(DBUS_TYPE_BOOLEAN, value);
+}
+
+bool MessageReader::PopInt16(int16* value) {
+  return PopBasic(DBUS_TYPE_INT16, value);
+}
+
+bool MessageReader::PopUint16(uint16* value) {
+  return PopBasic(DBUS_TYPE_UINT16, value);
+}
+
+bool MessageReader::PopInt32(int32* value) {
+  return PopBasic(DBUS_TYPE_INT32, value);
+}
+
+bool MessageReader::PopUint32(uint32* value) {
+  return PopBasic(DBUS_TYPE_UINT32, value);
+}
+
+bool MessageReader::PopInt64(int64* value) {
+  return PopBasic(DBUS_TYPE_INT64, value);
+}
+
+bool MessageReader::PopUint64(uint64* value) {
+  return PopBasic(DBUS_TYPE_UINT64, value);
+}
+
+bool MessageReader::PopDouble(double* value) {
+  return PopBasic(DBUS_TYPE_DOUBLE, value);
+}
+
+bool MessageReader::PopString(std::string* value) {
+  char* tmp_value = NULL;
+  const bool success = PopBasic(DBUS_TYPE_STRING, &tmp_value);
+  if (success)
+    *value = tmp_value;  // Copy the string.

[stevenjb, 2011/07/27 17:56:26]

nit: = std::string(tmp_value) is more explicit and makes the comment
unnecessary.
I assume tmp_value is guaranteed not to be NULL on success? (Need to be certain
since std::string(NULL) will crash).

[satorux1, 2011/07/27 21:20:35]

Good point, but it might copy the string twice (1. when to create a
std::string(tmp_value), and 2. when to copy), unless there is a clever
optimization.

Changed the code to

  value->assign(tmp_value);
Yes. I also added some comment in PopBasic().

+  return success;
+}
+
+bool MessageReader::PopObjectPath(std::string* value) {
+  char* tmp_value = NULL;
+  const bool success = PopBasic(DBUS_TYPE_OBJECT_PATH, &tmp_value);
+  if (success)
+    *value = tmp_value;  // Copy the string.

[stevenjb, 2011/07/27 17:56:26]

Same comment as above.

[satorux1, 2011/07/27 21:20:35]

Done.

+  return success;
+}
+
+bool MessageReader::PopArray(MessageReader* sub_reader) {
+  return PopContainer(DBUS_TYPE_ARRAY, sub_reader);
+}
+
+bool MessageReader::PopStruct(MessageReader* sub_reader) {
+  return PopContainer(DBUS_TYPE_STRUCT, sub_reader);
+}
+
+bool MessageReader::PopDictEntry(MessageReader* sub_reader) {
+  return PopContainer(DBUS_TYPE_DICT_ENTRY, sub_reader);
+}
+
+bool MessageReader::PopVariant(MessageReader* sub_reader) {
+  return PopContainer(DBUS_TYPE_VARIANT, sub_reader);
+}
+
+bool MessageReader::PopArrayOfBytes(
+    std::vector<uint8>* bytes) {
+  MessageReader array_reader(message_);
+  if (!PopArray(&array_reader))
+      return false;
+  if (!array_reader.CheckDataType(DBUS_TYPE_BYTE))
+    return false;
+  char* values = NULL;
+  int num_values = 0;
+  dbus_message_iter_get_fixed_array(&array_reader.raw_message_iter_,
+                                    &values, &num_values);
+  if (!values)
+    return false;
+
+  bytes->assign(values, values + num_values);  // Copy the data.
+  return true;
+}
+
+bool MessageReader::PopArrayOfObjectPaths(
+    std::vector<std::string> *object_paths) {
+  MessageReader array_reader(message_);
+  if (!PopArray(&array_reader))
+      return false;
+  while (array_reader.HasMore()) {
+    std::string object_path;
+    if (!array_reader.PopObjectPath(&object_path))
+      return false;
+    object_paths->push_back(object_path);
+  }
+  return true;
+}
+
+bool MessageReader::PopVariantOfByte(uint8* value) {
+  return PopVariantOfBasic(DBUS_TYPE_BYTE, value);
+}
+
+bool MessageReader::PopVariantOfBool(bool* value) {
+  return PopVariantOfBasic(DBUS_TYPE_BOOLEAN, value);
+}
+
+bool MessageReader::PopVariantOfInt16(int16* value) {
+  return PopVariantOfBasic(DBUS_TYPE_INT16, value);
+}
+
+bool MessageReader::PopVariantOfUint16(uint16* value) {
+  return PopVariantOfBasic(DBUS_TYPE_UINT16, value);
+}
+
+bool MessageReader::PopVariantOfInt32(int32* value) {
+  return PopVariantOfBasic(DBUS_TYPE_INT32, value);
+}
+
+bool MessageReader::PopVariantOfUint32(uint32* value) {
+  return PopVariantOfBasic(DBUS_TYPE_UINT32, value);
+}
+
+bool MessageReader::PopVariantOfInt64(int64* value) {
+  return PopVariantOfBasic(DBUS_TYPE_INT64, value);
+}
+
+bool MessageReader::PopVariantOfUint64(uint64* value) {
+  return PopVariantOfBasic(DBUS_TYPE_UINT64, value);
+}
+
+bool MessageReader::PopVariantOfDouble(double* value) {
+  return PopVariantOfBasic(DBUS_TYPE_DOUBLE, value);
+}
+
+bool MessageReader::PopVariantOfString(std::string* value) {
+  char* tmp_value = NULL;
+  const bool success = PopVariantOfBasic(DBUS_TYPE_STRING, &tmp_value);
+  if (success)
+    *value = tmp_value;  // Copy the string.

[stevenjb, 2011/07/27 17:56:26]

= std::string(tmp_value)

[satorux1, 2011/07/27 21:20:35]

Done.

+  return success;
+}
+
+bool MessageReader::PopVariantOfObjectPath(std::string* value) {
+  char* tmp_value = NULL;
+  const bool success = PopVariantOfBasic(DBUS_TYPE_OBJECT_PATH, &tmp_value);
+  if (success)
+    *value = tmp_value;  // Copy the string.

[stevenjb, 2011/07/27 17:56:26]

= std::string(tmp_value)

[satorux1, 2011/07/27 21:20:35]

Done.

+  return success;
+}
+
+Message::DataType MessageReader::GetDataType() {
+  const int dbus_type = dbus_message_iter_get_arg_type(&raw_message_iter_);
+  return static_cast<Message::DataType>(dbus_type);
+}
+
+bool MessageReader::CheckDataType(int dbus_type) {
+  const int actual_type = dbus_message_iter_get_arg_type(&raw_message_iter_);
+  if (actual_type != dbus_type) {
+    VLOG(1) << "Type " << dbus_type  << " is expected but got "
+            << actual_type;
+    return false;
+  }
+  return true;
+}
+
+bool MessageReader::PopBasic(int dbus_type, void* value) {
+  if (!CheckDataType(dbus_type))
+    return false;
+  dbus_message_iter_get_basic(&raw_message_iter_, value);
+  dbus_message_iter_next(&raw_message_iter_);
+  return true;
+}
+
+bool MessageReader::PopContainer(int dbus_type, MessageReader* sub_reader) {
+  DCHECK_NE(this, sub_reader);
+
+  if (!CheckDataType(dbus_type))
+    return false;
+  dbus_message_iter_recurse(&raw_message_iter_,
+                            &sub_reader->raw_message_iter_);
+  dbus_message_iter_next(&raw_message_iter_);
+  return true;
+}
+
+bool MessageReader::PopVariantOfBasic(int dbus_type, void* value) {
+  dbus::MessageReader variant_reader(message_);
+  if (!PopVariant(&variant_reader))
+    return false;
+  return variant_reader.PopBasic(dbus_type, value);
+}
+
+}  // namespace dbus