From e908adfde306a4b9767fe6ab8aa390af37ce43d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Kosi=C5=84ski?= Date: Sat, 29 Apr 2023 12:50:09 +0000 Subject: [PATCH] Refactor BindToDeviceSocketMutator. Only expose a factory function in the header. This limits the internal gRPC dependency to the .cpp file and simplifies the implementation. Bug: 280043032 Test: local build Change-Id: Ic7ea8dac9935231ceb05bec22c2a5902c50ea8db --- .../bind_to_device_socket_mutator/Android.bp | 2 +- .../include/BindToDeviceSocketMutator.h | 15 ++---- .../src/BindToDeviceSocketMutator.cpp | 48 +++++++++++-------- .../hal/default/src/RemoteAccessImpl.cpp | 2 +- 4 files changed, 34 insertions(+), 33 deletions(-) diff --git a/automotive/remoteaccess/bind_to_device_socket_mutator/Android.bp b/automotive/remoteaccess/bind_to_device_socket_mutator/Android.bp index 113b14e63f..58df21ac52 100644 --- a/automotive/remoteaccess/bind_to_device_socket_mutator/Android.bp +++ b/automotive/remoteaccess/bind_to_device_socket_mutator/Android.bp @@ -37,7 +37,7 @@ cc_defaults { cc_library { name: "BindToDeviceSocketMutatorLib", vendor_available: true, - srcs: ["src/*"], + srcs: ["src/BindToDeviceSocketMutator.cpp"], export_include_dirs: ["include"], defaults: ["BindToDeviceSocketMutatorDefaults"], } diff --git a/automotive/remoteaccess/bind_to_device_socket_mutator/include/BindToDeviceSocketMutator.h b/automotive/remoteaccess/bind_to_device_socket_mutator/include/BindToDeviceSocketMutator.h index bafcc654c4..5974c4b366 100644 --- a/automotive/remoteaccess/bind_to_device_socket_mutator/include/BindToDeviceSocketMutator.h +++ b/automotive/remoteaccess/bind_to_device_socket_mutator/include/BindToDeviceSocketMutator.h @@ -16,20 +16,11 @@ #pragma once -#include -#include -#include +#include +#include namespace android::hardware::automotive::remoteaccess { -class BindToDeviceSocketMutator final : public grpc_socket_mutator { - public: - BindToDeviceSocketMutator(const std::string_view& interface_name); - - bool mutateFd(int fd); - - private: - std::string mIfname; -}; +grpc_socket_mutator* MakeBindToDeviceSocketMutator(std::string_view interface_name); } // namespace android::hardware::automotive::remoteaccess diff --git a/automotive/remoteaccess/bind_to_device_socket_mutator/src/BindToDeviceSocketMutator.cpp b/automotive/remoteaccess/bind_to_device_socket_mutator/src/BindToDeviceSocketMutator.cpp index c6a96deb88..04a4c5bde9 100644 --- a/automotive/remoteaccess/bind_to_device_socket_mutator/src/BindToDeviceSocketMutator.cpp +++ b/automotive/remoteaccess/bind_to_device_socket_mutator/src/BindToDeviceSocketMutator.cpp @@ -18,40 +18,50 @@ #include #include +#include #include -namespace android::hardware::automotive::remoteaccess { +#include -bool BindToDeviceSocketMutator::mutateFd(int fd) { - int ret = setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE, mIfname.c_str(), mIfname.size()); +namespace android::hardware::automotive::remoteaccess { +namespace { + +struct BindToDeviceMutator : grpc_socket_mutator { + std::string ifname; +}; + +bool MutateFd(int fd, grpc_socket_mutator* mutator) { + auto* bdm = static_cast(mutator); + int ret = setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE, bdm->ifname.c_str(), bdm->ifname.size()); if (ret != 0) { - PLOG(ERROR) << "Can't bind socket to interface " << mIfname; + PLOG(ERROR) << "Can't bind socket to interface " << bdm->ifname; return false; } return true; } -bool bind_to_device_mutator_mutate_fd(int fd, grpc_socket_mutator* mutator) { - BindToDeviceSocketMutator* bsm = (BindToDeviceSocketMutator*)mutator; - return bsm->mutateFd(fd); -} - -int bind_to_device_mutator_compare(grpc_socket_mutator* a, grpc_socket_mutator* b) { +int Compare(grpc_socket_mutator* a, grpc_socket_mutator* b) { return ((a) < (b) ? -1 : ((a) > (b) ? 1 : 0)); } -void bind_to_device_mutator_destroy(grpc_socket_mutator* mutator) { - BindToDeviceSocketMutator* bsm = (BindToDeviceSocketMutator*)mutator; - delete bsm; +void Destroy(grpc_socket_mutator* mutator) { + auto* bdm = static_cast(mutator); + delete bdm; } -grpc_socket_mutator_vtable bind_to_device_mutator_vtable = {bind_to_device_mutator_mutate_fd, - bind_to_device_mutator_compare, - bind_to_device_mutator_destroy}; +constexpr grpc_socket_mutator_vtable kMutatorVtable = { + .mutate_fd = MutateFd, + .compare = Compare, + .destroy = Destroy, +}; -BindToDeviceSocketMutator::BindToDeviceSocketMutator(const std::string_view& interface_name) { - mIfname = interface_name; - grpc_socket_mutator_init(this, &bind_to_device_mutator_vtable); +} // namespace + +grpc_socket_mutator* MakeBindToDeviceSocketMutator(std::string_view interface_name) { + auto* bdm = new BindToDeviceMutator; + grpc_socket_mutator_init(bdm, &kMutatorVtable); + bdm->ifname = interface_name; + return bdm; } } // namespace android::hardware::automotive::remoteaccess diff --git a/automotive/remoteaccess/hal/default/src/RemoteAccessImpl.cpp b/automotive/remoteaccess/hal/default/src/RemoteAccessImpl.cpp index d5251410bb..b091162000 100644 --- a/automotive/remoteaccess/hal/default/src/RemoteAccessImpl.cpp +++ b/automotive/remoteaccess/hal/default/src/RemoteAccessImpl.cpp @@ -40,7 +40,7 @@ int main(int /* argc */, char* /* argv */[]) { #ifdef GRPC_SERVICE_IFNAME grpcargs.SetSocketMutator( - new android::hardware::automotive::remoteaccess::BindToDeviceSocketMutator( + android::hardware::automotive::remoteaccess::MakeBindToDeviceSocketMutator( GRPC_SERVICE_IFNAME)); LOG(DEBUG) << "GRPC_SERVICE_IFNAME specified as: " << GRPC_SERVICE_IFNAME; LOG(INFO) << "Waiting for interface: " << GRPC_SERVICE_IFNAME;