Centralize creation and error handling for VsockServerInfo
Provide helper functions that create VsockServerInfo objects
from either the command line or the property storage, and
have them return optional instead of relying on default values.
Also, avoid atoi() since it has the potential for undefined behavior.
Test: build
Change-Id: I0aab6b7d84e5084580e25217ac5ba4f5a1db6e2a
diff --git a/automotive/vehicle/2.0/default/VirtualizationGrpcServer.cpp b/automotive/vehicle/2.0/default/VirtualizationGrpcServer.cpp
index cca65d9..fb02c58 100644
--- a/automotive/vehicle/2.0/default/VirtualizationGrpcServer.cpp
+++ b/automotive/vehicle/2.0/default/VirtualizationGrpcServer.cpp
@@ -1,6 +1,4 @@
#include <android-base/logging.h>
-#include <getopt.h>
-#include <unistd.h>
#include "vhal_v2_0/virtualization/GrpcVehicleServer.h"
#include "vhal_v2_0/virtualization/Utils.h"
@@ -8,42 +6,10 @@
int main(int argc, char* argv[]) {
namespace vhal_impl = android::hardware::automotive::vehicle::V2_0::impl;
- vhal_impl::VsockServerInfo serverInfo;
+ auto serverInfo = vhal_impl::VsockServerInfo::fromCommandLine(argc, argv);
+ CHECK(serverInfo.has_value()) << "Invalid server CID/port combination";
- // unique values to identify the options
- constexpr int OPT_VHAL_SERVER_CID = 1001;
- constexpr int OPT_VHAL_SERVER_PORT_NUMBER = 1002;
-
- struct option longOptions[] = {
- {"server_cid", 1, 0, OPT_VHAL_SERVER_CID},
- {"server_port", 1, 0, OPT_VHAL_SERVER_PORT_NUMBER},
- {nullptr, 0, nullptr, 0},
- };
-
- int optValue;
- while ((optValue = getopt_long_only(argc, argv, ":", longOptions, 0)) != -1) {
- switch (optValue) {
- case OPT_VHAL_SERVER_CID:
- serverInfo.serverCid = std::atoi(optarg);
- LOG(DEBUG) << "Vehicle HAL server CID: " << serverInfo.serverCid;
- break;
- case OPT_VHAL_SERVER_PORT_NUMBER:
- serverInfo.serverPort = std::atoi(optarg);
- LOG(DEBUG) << "Vehicle HAL server port: " << serverInfo.serverPort;
- break;
- default:
- // ignore other options
- break;
- }
- }
-
- if (serverInfo.serverCid == 0 || serverInfo.serverPort == 0) {
- LOG(FATAL) << "Invalid server information, CID: " << serverInfo.serverCid
- << "; port: " << serverInfo.serverPort;
- // Will abort after logging
- }
-
- auto server = vhal_impl::makeGrpcVehicleServer(vhal_impl::getVsockUri(serverInfo));
+ auto server = vhal_impl::makeGrpcVehicleServer(serverInfo->toUri());
server->Start();
return 0;
}
diff --git a/automotive/vehicle/2.0/default/VirtualizedVehicleService.cpp b/automotive/vehicle/2.0/default/VirtualizedVehicleService.cpp
index 1de81ae..68813c9 100644
--- a/automotive/vehicle/2.0/default/VirtualizedVehicleService.cpp
+++ b/automotive/vehicle/2.0/default/VirtualizedVehicleService.cpp
@@ -15,7 +15,6 @@
*/
#include <android-base/logging.h>
-#include <cutils/properties.h>
#include <hidl/HidlTransportSupport.h>
#include <vhal_v2_0/EmulatedVehicleConnector.h>
@@ -29,30 +28,13 @@
using namespace android::hardware::automotive::vehicle::V2_0;
int main(int argc, char* argv[]) {
- constexpr const char* VHAL_SERVER_CID_PROPERTY_KEY = "ro.vendor.vehiclehal.server.cid";
- constexpr const char* VHAL_SERVER_PORT_PROPERTY_KEY = "ro.vendor.vehiclehal.server.port";
+ namespace vhal_impl = android::hardware::automotive::vehicle::V2_0::impl;
- auto property_get_uint = [](const char* key, unsigned int default_value) {
- auto value = property_get_int64(key, default_value);
- if (value < 0 || value > UINT_MAX) {
- LOG(DEBUG) << key << ": " << value << " is out of bound, using default value '"
- << default_value << "' instead";
- return default_value;
- }
- return static_cast<unsigned int>(value);
- };
-
- impl::VsockServerInfo serverInfo{property_get_uint(VHAL_SERVER_CID_PROPERTY_KEY, 0),
- property_get_uint(VHAL_SERVER_PORT_PROPERTY_KEY, 0)};
-
- if (serverInfo.serverCid == 0 || serverInfo.serverPort == 0) {
- LOG(FATAL) << "Invalid server information, CID: " << serverInfo.serverCid
- << "; port: " << serverInfo.serverPort;
- // Will abort after logging
- }
+ auto serverInfo = vhal_impl::VsockServerInfo::fromRoPropertyStore();
+ CHECK(serverInfo.has_value()) << "Invalid server CID/port combination";
auto store = std::make_unique<VehiclePropertyStore>();
- auto connector = impl::makeGrpcVehicleClient(impl::getVsockUri(serverInfo));
+ auto connector = impl::makeGrpcVehicleClient(serverInfo->toUri());
auto hal = std::make_unique<impl::EmulatedVehicleHal>(store.get(), connector.get());
auto emulator = std::make_unique<impl::VehicleEmulator>(hal.get());
auto service = std::make_unique<VehicleHalManager>(hal.get());
diff --git a/automotive/vehicle/2.0/default/android.hardware.automotive.vehicle@2.0-virtualization-grpc-server.rc b/automotive/vehicle/2.0/default/android.hardware.automotive.vehicle@2.0-virtualization-grpc-server.rc
index 29147ad..1101b08 100644
--- a/automotive/vehicle/2.0/default/android.hardware.automotive.vehicle@2.0-virtualization-grpc-server.rc
+++ b/automotive/vehicle/2.0/default/android.hardware.automotive.vehicle@2.0-virtualization-grpc-server.rc
@@ -3,8 +3,8 @@
# so the command line arguments are expected, though not conventionally used in Android
service vendor.vehicle-hal-2.0-server \
/vendor/bin/hw/android.hardware.automotive.vehicle@2.0-virtualization-grpc-server \
- -server_cid ${ro.vendor.vehiclehal.server.cid:-0} \
- -server_port ${ro.vendor.vehiclehal.server.port:-0}
+ -server_cid ${ro.vendor.vehiclehal.server.cid:-pleaseconfigurethis} \
+ -server_port ${ro.vendor.vehiclehal.server.port:-pleaseconfigurethis}
class hal
user vehicle_network
group system inet
diff --git a/automotive/vehicle/2.0/default/impl/vhal_v2_0/virtualization/Utils.cpp b/automotive/vehicle/2.0/default/impl/vhal_v2_0/virtualization/Utils.cpp
index 41d4827..184d8a4 100644
--- a/automotive/vehicle/2.0/default/impl/vhal_v2_0/virtualization/Utils.cpp
+++ b/automotive/vehicle/2.0/default/impl/vhal_v2_0/virtualization/Utils.cpp
@@ -16,6 +16,11 @@
#include "Utils.h"
+#include <cutils/properties.h>
+
+#include <getopt.h>
+#include <stdlib.h>
+#include <unistd.h>
#include <sstream>
namespace android {
@@ -25,12 +30,83 @@
namespace V2_0 {
namespace impl {
-std::string getVsockUri(const VsockServerInfo& serverInfo) {
+std::string VsockServerInfo::toUri() {
std::stringstream uri_stream;
- uri_stream << "vsock:" << serverInfo.serverCid << ":" << serverInfo.serverPort;
+ uri_stream << "vsock:" << serverCid << ":" << serverPort;
return uri_stream.str();
}
+static std::optional<unsigned> parseUnsignedIntFromString(const char* optarg, const char* name) {
+ auto v = strtoul(optarg, nullptr, 0);
+ if (((v == ULONG_MAX) && (errno == ERANGE)) || (v > UINT_MAX)) {
+ LOG(WARNING) << name << " value is out of range: " << optarg;
+ } else if (v != 0) {
+ return v;
+ } else {
+ LOG(WARNING) << name << " value is invalid or missing: " << optarg;
+ }
+
+ return std::nullopt;
+}
+
+static std::optional<unsigned> getNumberFromProperty(const char* key) {
+ auto value = property_get_int64(key, -1);
+ if ((value <= 0) || (value > UINT_MAX)) {
+ LOG(WARNING) << key << " is missing or out of bounds";
+ return std::nullopt;
+ }
+
+ return static_cast<unsigned int>(value);
+};
+
+std::optional<VsockServerInfo> VsockServerInfo::fromCommandLine(int argc, char* argv[]) {
+ std::optional<unsigned int> cid;
+ std::optional<unsigned int> port;
+
+ // unique values to identify the options
+ constexpr int OPT_VHAL_SERVER_CID = 1001;
+ constexpr int OPT_VHAL_SERVER_PORT_NUMBER = 1002;
+
+ struct option longOptions[] = {
+ {"server_cid", 1, 0, OPT_VHAL_SERVER_CID},
+ {"server_port", 1, 0, OPT_VHAL_SERVER_PORT_NUMBER},
+ {},
+ };
+
+ int optValue;
+ while ((optValue = getopt_long_only(argc, argv, ":", longOptions, 0)) != -1) {
+ switch (optValue) {
+ case OPT_VHAL_SERVER_CID:
+ cid = parseUnsignedIntFromString(optarg, "cid");
+ break;
+ case OPT_VHAL_SERVER_PORT_NUMBER:
+ port = parseUnsignedIntFromString(optarg, "port");
+ break;
+ default:
+ // ignore other options
+ break;
+ }
+ }
+
+ if (cid && port) {
+ return VsockServerInfo{*cid, *port};
+ }
+ return std::nullopt;
+}
+
+std::optional<VsockServerInfo> VsockServerInfo::fromRoPropertyStore() {
+ constexpr const char* VHAL_SERVER_CID_PROPERTY_KEY = "ro.vendor.vehiclehal.server.cid";
+ constexpr const char* VHAL_SERVER_PORT_PROPERTY_KEY = "ro.vendor.vehiclehal.server.port";
+
+ const auto cid = getNumberFromProperty(VHAL_SERVER_CID_PROPERTY_KEY);
+ const auto port = getNumberFromProperty(VHAL_SERVER_PORT_PROPERTY_KEY);
+
+ if (cid && port) {
+ return VsockServerInfo{*cid, *port};
+ }
+ return std::nullopt;
+}
+
} // namespace impl
} // namespace V2_0
} // namespace vehicle
diff --git a/automotive/vehicle/2.0/default/impl/vhal_v2_0/virtualization/Utils.h b/automotive/vehicle/2.0/default/impl/vhal_v2_0/virtualization/Utils.h
index 6b1049c..8a8bce7 100644
--- a/automotive/vehicle/2.0/default/impl/vhal_v2_0/virtualization/Utils.h
+++ b/automotive/vehicle/2.0/default/impl/vhal_v2_0/virtualization/Utils.h
@@ -17,8 +17,11 @@
#ifndef android_hardware_automotive_vehicle_V2_0_impl_virtualization_Utils_H_
#define android_hardware_automotive_vehicle_V2_0_impl_virtualization_Utils_H_
+#include <optional>
#include <string>
+#include <android-base/logging.h>
+
namespace android {
namespace hardware {
namespace automotive {
@@ -29,9 +32,12 @@
struct VsockServerInfo {
unsigned int serverCid{0};
unsigned int serverPort{0};
-};
-std::string getVsockUri(const VsockServerInfo& serverInfo);
+ static std::optional<VsockServerInfo> fromCommandLine(int argc, char* argv[]);
+ static std::optional<VsockServerInfo> fromRoPropertyStore();
+
+ std::string toUri();
+};
} // namespace impl
} // namespace V2_0