Fix subtle memory issues detected by valgrind.

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"
@@ skipping 243 matching lines @@
 }
 
 MessageWriter::~MessageWriter() {
 }
 
 void MessageWriter::AppendByte(uint8 value) {
   AppendBasic(DBUS_TYPE_BYTE, &value);
 }
 
 void MessageWriter::AppendBool(bool value) {
-  AppendBasic(DBUS_TYPE_BOOLEAN, &value);
+  // The size of dbus_bool_t and the size of bool are different. The
+  // former is always 4 per dbus-types.h, whereas the latter is usually 1.
+  // dbus_message_iter_append_basic() used in AppendBasic() expects four
+  // bytes for DBUS_TYPE_BOOLEAN, so we must pass a dbus_bool_t, instead
+  // of a bool, to AppendBasic().
+  dbus_bool_t dbus_value = value;
+  AppendBasic(DBUS_TYPE_BOOLEAN, &dbus_value);
 }
 
 void MessageWriter::AppendInt16(int16 value) {
   AppendBasic(DBUS_TYPE_INT16, &value);
 }
 
 void MessageWriter::AppendUint16(uint16 value) {
   AppendBasic(DBUS_TYPE_UINT16, &value);
 }
 
@@ skipping 108 matching lines @@
       static_cast<int>(length));
   CHECK(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);
+  // See the comment at MessageWriter::AppendBool().
+  dbus_bool_t dbus_value = value;
+  AppendVariantOfBasic(DBUS_TYPE_BOOLEAN, &dbus_value);
 }
 
 void MessageWriter::AppendVariantOfInt16(int16 value) {
   AppendVariantOfBasic(DBUS_TYPE_INT16, &value);
 }
 
 void MessageWriter::AppendVariantOfUint16(uint16 value) {
   AppendVariantOfBasic(DBUS_TYPE_UINT16, &value);
 }
 
@@ skipping 62 matching lines @@
 bool MessageReader::HasMoreData() {
   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);
+  // Like MessageWriter::AppendBool(), we should copy |value| to
+  // dbus_bool_t, as dbus_message_iter_get_basic() used in PopBasic()
+  // expects four bytes for DBUS_TYPE_BOOLEAN.
+  dbus_bool_t dbus_value = FALSE;
+  const bool success = PopBasic(DBUS_TYPE_BOOLEAN, &dbus_value);
+  *value = dbus_value;

[stevenjb, 2011/08/03 20:40:00]

on some compilers this may require a static_cast<> (probably a good idea
anyway).

[satorux1, 2011/08/03 20:43:04]

Done.

+  return success;
 }
 
 bool MessageReader::PopInt16(int16* value) {
   return PopBasic(DBUS_TYPE_INT16, value);
 }
 
 bool MessageReader::PopUint16(uint16* value) {
   return PopBasic(DBUS_TYPE_UINT16, value);
 }
 
@@ skipping 75 matching lines @@
     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);
+  // See the comment at MessageReader::PopBool().
+  dbus_bool_t dbus_value = FALSE;
+  const bool success = PopVariantOfBasic(DBUS_TYPE_BOOLEAN, &dbus_value);
+  *value = dbus_value;

[stevenjb, 2011/08/03 20:40:00]

static_cast

[satorux1, 2011/08/03 20:43:04]

Done.

+  return success;
 }
 
 bool MessageReader::PopVariantOfInt16(int16* value) {
   return PopVariantOfBasic(DBUS_TYPE_INT16, value);
 }
 
 bool MessageReader::PopVariantOfUint16(uint16* value) {
   return PopVariantOfBasic(DBUS_TYPE_UINT16, value);
 }
 
@@ skipping 72 matching lines @@
 }
 
 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