binder: Use ifname as key in BinderManager |IIface| map
Since ifname is unique, its probably a better key to lookup the
corresponding |IIface| binder object than using raw pointer.
This deprecates the usage |binder_object_key| added in |wpa_supplicant|
struct.
While there,
1. Change all comments in |BinderManager| to c++ style.
2. Add braces for all one line if statements.
BUG: 29998764
Change-Id: Ie0b571204b8363ac49c5db7666249b9a39006f31
TEST: Ran the integration tests under |wificond|.
Signed-off-by: Roshan Pius <rpius@google.com>
diff --git a/wpa_supplicant/binder/binder.cpp b/wpa_supplicant/binder/binder.cpp
index eb6006c..9fee001 100644
--- a/wpa_supplicant/binder/binder.cpp
+++ b/wpa_supplicant/binder/binder.cpp
@@ -85,7 +85,7 @@
int wpas_binder_register_interface(struct wpa_supplicant *wpa_s)
{
- if (!wpa_s->global->binder)
+ if (!wpa_s->global->binder || !wpa_s)
return 1;
wpa_printf(
@@ -102,7 +102,7 @@
int wpas_binder_unregister_interface(struct wpa_supplicant *wpa_s)
{
- if (!wpa_s->global->binder)
+ if (!wpa_s->global->binder || !wpa_s)
return 1;
wpa_printf(
diff --git a/wpa_supplicant/binder/binder_manager.cpp b/wpa_supplicant/binder/binder_manager.cpp
index 7b54b66..00fa70f 100644
--- a/wpa_supplicant/binder/binder_manager.cpp
+++ b/wpa_supplicant/binder/binder_manager.cpp
@@ -37,8 +37,8 @@
int BinderManager::registerBinderService(struct wpa_global *global)
{
- /* Create the main binder service object and register with
- * system service manager. */
+ // Create the main binder service object and register with system
+ // ServiceManager.
supplicant_object_ = new Supplicant(global);
android::String16 service_name(binder_constants::kServiceName);
android::defaultServiceManager()->addService(
@@ -46,55 +46,73 @@
return 0;
}
+/**
+ * Register an interface to binder manager.
+ *
+ * @param wpa_s |wpa_supplicant| struct corresponding to the interface.
+ *
+ * @return 0 on success, 1 on failure.
+ */
int BinderManager::registerInterface(struct wpa_supplicant *wpa_s)
{
if (!wpa_s)
return 1;
- /* Using the corresponding wpa_supplicant pointer as key to our
- * object map. */
- const void *iface_key = wpa_s;
+ // Using the corresponding ifname as key to our object map.
+ const std::string ifname(wpa_s->ifname);
- /* Return failure if we already have an object for that iface_key. */
- if (iface_object_map_.find(iface_key) != iface_object_map_.end())
+ // Return failure if we already have an object for that |ifname|.
+ if (iface_object_map_.find(ifname) != iface_object_map_.end())
return 1;
- iface_object_map_[iface_key] = new Iface(wpa_s->global, wpa_s->ifname);
- if (!iface_object_map_[iface_key].get())
+ iface_object_map_[ifname] = new Iface(wpa_s->global, wpa_s->ifname);
+ if (!iface_object_map_[ifname].get())
return 1;
- wpa_s->binder_object_key = iface_key;
-
return 0;
}
+/**
+ * Unregister an interface from binder manager.
+ *
+ * @param wpa_s |wpa_supplicant| struct corresponding to the interface.
+ *
+ * @return 0 on success, 1 on failure.
+ */
int BinderManager::unregisterInterface(struct wpa_supplicant *wpa_s)
{
- if (!wpa_s || !wpa_s->binder_object_key)
+ if (!wpa_s)
return 1;
- const void *iface_key = wpa_s;
- if (iface_object_map_.find(iface_key) == iface_object_map_.end())
+ const std::string ifname(wpa_s->ifname);
+ if (iface_object_map_.find(ifname) == iface_object_map_.end())
return 1;
/* Delete the corresponding iface object from our map. */
- iface_object_map_.erase(iface_key);
- wpa_s->binder_object_key = NULL;
+ iface_object_map_.erase(ifname);
return 0;
}
-int BinderManager::getIfaceBinderObjectByKey(
- const void *iface_object_key,
+/**
+ * Retrieve the |IIface| binder object reference using the provided ifname.
+ *
+ * @param ifname Name of the corresponding interface.
+ * @param iface_object Binder reference corresponding to the iface.
+ *
+ * @return 0 on success, 1 on failure.
+ */
+int BinderManager::getIfaceBinderObjectByIfname(
+ const std::string &ifname,
android::sp<fi::w1::wpa_supplicant::IIface> *iface_object)
{
- if (!iface_object_key || !iface_object)
+ if (ifname.empty() || !iface_object)
return 1;
- if (iface_object_map_.find(iface_object_key) == iface_object_map_.end())
+ if (iface_object_map_.find(ifname) == iface_object_map_.end())
return 1;
- *iface_object = iface_object_map_[iface_object_key];
+ *iface_object = iface_object_map_[ifname];
return 0;
}
-} /* namespace wpa_supplicant_binder */
+} // namespace wpa_supplicant_binder
diff --git a/wpa_supplicant/binder/binder_manager.h b/wpa_supplicant/binder/binder_manager.h
index d8b7dd0..b4b8f76 100644
--- a/wpa_supplicant/binder/binder_manager.h
+++ b/wpa_supplicant/binder/binder_manager.h
@@ -35,24 +35,26 @@
int registerBinderService(struct wpa_global *global);
int registerInterface(struct wpa_supplicant *wpa_s);
int unregisterInterface(struct wpa_supplicant *wpa_s);
- int getIfaceBinderObjectByKey(
- const void *iface_object_key,
+ int getIfaceBinderObjectByIfname(
+ const std::string &ifname,
android::sp<fi::w1::wpa_supplicant::IIface> *iface_object);
private:
BinderManager() = default;
~BinderManager() = default;
+ BinderManager(const BinderManager &) = default;
+ BinderManager &operator=(const BinderManager &) = default;
- /* Singleton instance of this class. */
+ // Singleton instance of this class.
static BinderManager *instance_;
- /* The main binder service object. */
+ // The main binder service object.
android::sp<Supplicant> supplicant_object_;
- /* Map of all the interface specific binder objects controlled by
- * wpa_supplicant. This map is keyed in by the corresponding
- * wpa_supplicant structure pointer. */
- std::map<const void *, android::sp<Iface>> iface_object_map_;
+ // Map of all the interface specific binder objects controlled by
+ // wpa_supplicant. This map is keyed in by the corresponding
+ // |ifname|.
+ std::map<const std::string, android::sp<Iface>> iface_object_map_;
};
-} /* namespace wpa_supplicant_binder */
+} // namespace wpa_supplicant_binder
-#endif /* WPA_SUPPLICANT_BINDER_BINDER_MANAGER_H */
+#endif // WPA_SUPPLICANT_BINDER_BINDER_MANAGER_H
diff --git a/wpa_supplicant/binder/supplicant.cpp b/wpa_supplicant/binder/supplicant.cpp
index d85f2cf..5a0db75 100644
--- a/wpa_supplicant/binder/supplicant.cpp
+++ b/wpa_supplicant/binder/supplicant.cpp
@@ -21,10 +21,11 @@
android::String16 driver, ifname, confname, bridge_ifname;
/* Check if required Ifname argument is missing */
- if (!params.getString(android::String16("Ifname"), &ifname))
+ if (!params.getString(android::String16("Ifname"), &ifname)) {
return android::binder::Status::fromExceptionCode(
android::binder::Status::EX_ILLEGAL_ARGUMENT,
"Ifname missing in params.");
+ }
/* Retrieve the remaining params from the dictionary */
params.getString(android::String16("Driver"), &driver);
params.getString(android::String16("ConfigFile"), &confname);
@@ -35,10 +36,11 @@
* an error if we already control it.
*/
if (wpa_supplicant_get_iface(
- wpa_global_, android::String8(ifname).string()) != NULL)
+ wpa_global_, android::String8(ifname).string()) != NULL) {
return android::binder::Status::fromServiceSpecificError(
ERROR_IFACE_EXISTS,
"wpa_supplicant already controls this interface.");
+ }
android::binder::Status status;
struct wpa_supplicant *wpa_s = NULL;
@@ -54,7 +56,7 @@
wpa_s = wpa_supplicant_add_iface(wpa_global_, &iface, NULL);
/* The supplicant core creates a corresponding binder object via
* BinderManager when |wpa_supplicant_add_iface| is called. */
- if (!wpa_s || !wpa_s->binder_object_key) {
+ if (!wpa_s) {
status = android::binder::Status::fromServiceSpecificError(
ERROR_GENERIC,
"wpa_supplicant couldn't grab this interface.");
@@ -62,14 +64,15 @@
BinderManager *binder_manager = BinderManager::getInstance();
if (!binder_manager ||
- binder_manager->getIfaceBinderObjectByKey(
- wpa_s->binder_object_key, aidl_return))
+ binder_manager->getIfaceBinderObjectByIfname(
+ wpa_s->ifname, aidl_return)) {
status =
android::binder::Status::fromServiceSpecificError(
ERROR_GENERIC,
"wpa_supplicant encountered a binder error.");
- else
+ } else {
status = android::binder::Status::ok();
+ }
}
os_free((void *)iface.driver);
os_free((void *)iface.ifname);
@@ -83,14 +86,16 @@
struct wpa_supplicant *wpa_s;
wpa_s = wpa_supplicant_get_iface(wpa_global_, ifname.c_str());
- if (!wpa_s || !wpa_s->binder_object_key)
+ if (!wpa_s) {
return android::binder::Status::fromServiceSpecificError(
ERROR_IFACE_UNKNOWN,
"wpa_supplicant does not control this interface.");
- if (wpa_supplicant_remove_iface(wpa_global_, wpa_s, 0))
+ }
+ if (wpa_supplicant_remove_iface(wpa_global_, wpa_s, 0)) {
return android::binder::Status::fromServiceSpecificError(
ERROR_GENERIC,
"wpa_supplicant couldn't remove this interface.");
+ }
return android::binder::Status::ok();
}
@@ -101,18 +106,20 @@
struct wpa_supplicant *wpa_s;
wpa_s = wpa_supplicant_get_iface(wpa_global_, ifname.c_str());
- if (!wpa_s || !wpa_s->binder_object_key)
+ if (!wpa_s) {
return android::binder::Status::fromServiceSpecificError(
ERROR_IFACE_UNKNOWN,
"wpa_supplicant does not control this interface.");
+ }
BinderManager *binder_manager = BinderManager::getInstance();
if (!binder_manager ||
- binder_manager->getIfaceBinderObjectByKey(
- wpa_s->binder_object_key, aidl_return))
+ binder_manager->getIfaceBinderObjectByIfname(
+ wpa_s->ifname, aidl_return)) {
return android::binder::Status::fromServiceSpecificError(
ERROR_GENERIC,
"wpa_supplicant encountered a binder error.");
+ }
return android::binder::Status::ok();
}