Fix a bug in dbus::Bus::AddFilterFunction().

dbus/bus.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.
 //
 // TODO(satorux):
 // - Handle "disconnected" signal.
 
 #include "dbus/bus.h"
 
 #include "base/bind.h"
@@ skipping 421 matching lines @@
 }
 
 void Bus::Send(DBusMessage* request, uint32* serial) {
   DCHECK(connection_);
   AssertOnDBusThread();
 
   const bool success = dbus_connection_send(connection_, request, serial);
   CHECK(success) << "Unable to allocate memory";
 }
 
-void Bus::AddFilterFunction(DBusHandleMessageFunction filter_function,
+bool Bus::AddFilterFunction(DBusHandleMessageFunction filter_function,
                             void* user_data) {
   DCHECK(connection_);
   AssertOnDBusThread();
 
-  if (filter_functions_added_.find(filter_function) !=
+  std::pair<DBusHandleMessageFunction, void*> filter_data_pair =
+      std::make_pair(filter_function, user_data);
+  if (filter_functions_added_.find(filter_data_pair) !=
       filter_functions_added_.end()) {
-    LOG(ERROR) << "Filter function already exists: " << filter_function;
-    return;
+    VLOG(1) << "Filter function already exists: " << filter_function
+            << " with associated data: " << user_data;
+    return false;

[cwolfe, 2011/10/05 22:39:16]

Do not know our rules for logging pointer-like things. For debug, I tend to cast
them to (void*) so they become unique hex strings.

[satorux1, 2011/10/05 23:04:40]

I thought our logging library automatically converted pointers to hex strings,
but I might be wrong.

C-style cast like (void*) is prohibited by the Google C++ style guide. we may
need to use reinterpret_cast for function pointers, but for now. I'd keep the
code as is...

   }
 
   const bool success = dbus_connection_add_filter(
       connection_, filter_function, user_data, NULL);
   CHECK(success) << "Unable to allocate memory";
-  filter_functions_added_.insert(filter_function);
+  filter_functions_added_.insert(filter_data_pair);
+  return true;
 }
 
-void Bus::RemoveFilterFunction(DBusHandleMessageFunction filter_function,
+bool Bus::RemoveFilterFunction(DBusHandleMessageFunction filter_function,
                                void* user_data) {
   DCHECK(connection_);
   AssertOnDBusThread();
 
-  if (filter_functions_added_.find(filter_function) ==
+  std::pair<DBusHandleMessageFunction, void*> filter_data_pair =
+      std::make_pair(filter_function, user_data);
+  if (filter_functions_added_.find(filter_data_pair) ==
       filter_functions_added_.end()) {
-    LOG(ERROR) << "Requested to remove an unknown filter function: "
-               << filter_function;
-    return;
+    VLOG(1) << "Requested to remove an unknown filter function: "
+            << filter_function
+            << " with associated data: " << user_data;
+    return false;
   }
 
   dbus_connection_remove_filter(connection_, filter_function, user_data);
-  filter_functions_added_.erase(filter_function);
+  filter_functions_added_.erase(filter_data_pair);
+  return true;
 }
 
 void Bus::AddMatch(const std::string& match_rule, DBusError* error) {
   DCHECK(connection_);
   AssertOnDBusThread();
 
   if (match_rules_added_.find(match_rule) != match_rules_added_.end()) {
     LOG(ERROR) << "Match rule already exists: " << match_rule;
     return;
   }
@@ skipping 249 matching lines @@
 }
 
 void Bus::OnDispatchStatusChangedThunk(DBusConnection* connection,
                                        DBusDispatchStatus status,
                                        void* data) {
   Bus* self = static_cast<Bus*>(data);
   return self->OnDispatchStatusChanged(connection, status);
 }
 
 }  // namespace dbus