From a7dc6fb5b4d023669ad7226c87b7fa46e44d59b2 Mon Sep 17 00:00:00 2001 From: Myles Watson Date: Mon, 26 Jun 2017 13:09:11 -0700 Subject: [PATCH] Bluetooth: Change CHECK() to LOG_ALWAYS_FATAL() Bug: 62641184 Test: Compilation Change-Id: I21192c7a5c15def379d040b586a66ee1534c7c15 (cherry picked from commit 3e272a70763ff6191ac96d8560724e0a02f827ac) --- bluetooth/1.0/default/bluetooth_address.cc | 18 +++++++---- bluetooth/1.0/default/h4_protocol.cc | 30 ++++++++++++------- bluetooth/1.0/default/hci_packetizer.cc | 27 ++++++++++++----- bluetooth/1.0/default/mct_protocol.cc | 3 +- .../default/test/bluetooth_address_test.cc | 1 + bluetooth/1.0/default/vendor_interface.cc | 11 ++++--- 6 files changed, 61 insertions(+), 29 deletions(-) diff --git a/bluetooth/1.0/default/bluetooth_address.cc b/bluetooth/1.0/default/bluetooth_address.cc index 656d78dd5d..65dc6a6164 100644 --- a/bluetooth/1.0/default/bluetooth_address.cc +++ b/bluetooth/1.0/default/bluetooth_address.cc @@ -16,8 +16,8 @@ #include "bluetooth_address.h" -#include #include +#include #include #include @@ -54,19 +54,25 @@ bool BluetoothAddress::get_local_address(uint8_t* local_addr) { addr_fd = open(property, O_RDONLY); if (addr_fd != -1) { - int bytes_read = read(addr_fd, property, kStringLength); - CHECK(bytes_read == kStringLength); + char address[kStringLength + 1] = {0}; + int bytes_read = read(addr_fd, address, kStringLength); + if (bytes_read == -1) { + ALOGE("%s: Error reading address from %s: %s", __func__, property, + strerror(errno)); + } close(addr_fd); // Null terminate the string. - property[kStringLength] = '\0'; + address[kStringLength] = '\0'; // If the address is not all zeros, then use it. const uint8_t zero_bdaddr[kBytes] = {0, 0, 0, 0, 0, 0}; - if ((string_to_bytes(property, local_addr)) && + if ((string_to_bytes(address, local_addr)) && (memcmp(local_addr, zero_bdaddr, kBytes) != 0)) { valid_bda = true; - ALOGD("%s: Got Factory BDA %s", __func__, property); + ALOGD("%s: Got Factory BDA %s", __func__, address); + } else { + ALOGE("%s: Got Invalid BDA '%s' from %s", __func__, address, property); } } } diff --git a/bluetooth/1.0/default/h4_protocol.cc b/bluetooth/1.0/default/h4_protocol.cc index 2df2b3b914..2008b00093 100644 --- a/bluetooth/1.0/default/h4_protocol.cc +++ b/bluetooth/1.0/default/h4_protocol.cc @@ -17,7 +17,7 @@ #include "h4_protocol.h" #define LOG_TAG "android.hardware.bluetooth-hci-h4" -#include + #include #include #include @@ -38,9 +38,9 @@ size_t H4Protocol::Send(uint8_t type, const uint8_t* data, size_t length) { if (ret == -1) { ALOGE("%s error writing to UART (%s)", __func__, strerror(errno)); - } else if (ret == 0) { - // Nothing written :( - ALOGE("%s zero bytes written - something went wrong...", __func__); + } else if (ret < static_cast(length + 1)) { + ALOGE("%s: %d / %d bytes written - something went wrong...", __func__, + static_cast(ret), static_cast(length + 1)); } return ret; } @@ -56,10 +56,9 @@ void H4Protocol::OnPacketReady() { case HCI_PACKET_TYPE_SCO_DATA: sco_cb_(hci_packetizer_.GetPacket()); break; - default: { - bool bad_packet_type = true; - CHECK(!bad_packet_type); - } + default: + LOG_ALWAYS_FATAL("%s: Unimplemented packet type %d", __func__, + static_cast(hci_packet_type_)); } // Get ready for the next type byte. hci_packet_type_ = HCI_PACKET_TYPE_UNKNOWN; @@ -68,8 +67,19 @@ void H4Protocol::OnPacketReady() { void H4Protocol::OnDataReady(int fd) { if (hci_packet_type_ == HCI_PACKET_TYPE_UNKNOWN) { uint8_t buffer[1] = {0}; - size_t bytes_read = TEMP_FAILURE_RETRY(read(fd, buffer, 1)); - CHECK(bytes_read == 1); + ssize_t bytes_read = TEMP_FAILURE_RETRY(read(fd, buffer, 1)); + if (bytes_read != 1) { + if (bytes_read == 0) { + LOG_ALWAYS_FATAL("%s: Unexpected EOF reading the packet type!", + __func__); + } else if (bytes_read < 0) { + LOG_ALWAYS_FATAL("%s: Read packet type error: %s", __func__, + strerror(errno)); + } else { + LOG_ALWAYS_FATAL("%s: More bytes read than expected (%u)!", __func__, + static_cast(bytes_read)); + } + } hci_packet_type_ = static_cast(buffer[0]); } else { hci_packetizer_.OnDataReady(fd, hci_packet_type_); diff --git a/bluetooth/1.0/default/hci_packetizer.cc b/bluetooth/1.0/default/hci_packetizer.cc index 9549858b2c..2da1254531 100644 --- a/bluetooth/1.0/default/hci_packetizer.cc +++ b/bluetooth/1.0/default/hci_packetizer.cc @@ -17,11 +17,11 @@ #include "hci_packetizer.h" #define LOG_TAG "android.hardware.bluetooth.hci_packetizer" -#include -#include #include +#include #include +#include namespace { @@ -45,15 +45,22 @@ namespace hardware { namespace bluetooth { namespace hci { -const hidl_vec& HciPacketizer::GetPacket() const { return packet_; } +const hidl_vec& HciPacketizer::GetPacket() const { + return packet_; +} void HciPacketizer::OnDataReady(int fd, HciPacketType packet_type) { switch (state_) { case HCI_PREAMBLE: { - size_t bytes_read = TEMP_FAILURE_RETRY( + ssize_t bytes_read = TEMP_FAILURE_RETRY( read(fd, preamble_ + bytes_read_, preamble_size_for_type[packet_type] - bytes_read_)); - CHECK(bytes_read > 0); + if (bytes_read <= 0) { + LOG_ALWAYS_FATAL_IF((bytes_read == 0), + "%s: Unexpected EOF reading the header!", __func__); + LOG_ALWAYS_FATAL("%s: Read header error: %s", __func__, + strerror(errno)); + } bytes_read_ += bytes_read; if (bytes_read_ == preamble_size_for_type[packet_type]) { size_t packet_length = @@ -68,11 +75,17 @@ void HciPacketizer::OnDataReady(int fd, HciPacketType packet_type) { } case HCI_PAYLOAD: { - size_t bytes_read = TEMP_FAILURE_RETRY(read( + ssize_t bytes_read = TEMP_FAILURE_RETRY(read( fd, packet_.data() + preamble_size_for_type[packet_type] + bytes_read_, bytes_remaining_)); - CHECK(bytes_read > 0); + if (bytes_read <= 0) { + LOG_ALWAYS_FATAL_IF((bytes_read == 0), + "%s: Unexpected EOF reading the payload!", + __func__); + LOG_ALWAYS_FATAL("%s: Read payload error: %s", __func__, + strerror(errno)); + } bytes_remaining_ -= bytes_read; bytes_read_ += bytes_read; if (bytes_remaining_ == 0) { diff --git a/bluetooth/1.0/default/mct_protocol.cc b/bluetooth/1.0/default/mct_protocol.cc index 813cebd239..2a59187cfe 100644 --- a/bluetooth/1.0/default/mct_protocol.cc +++ b/bluetooth/1.0/default/mct_protocol.cc @@ -19,7 +19,6 @@ #include #define LOG_TAG "android.hardware.bluetooth-hci-mct" -#include #include #include @@ -45,7 +44,7 @@ size_t MctProtocol::Send(uint8_t type, const uint8_t* data, size_t length) { return WriteSafely(uart_fds_[CH_CMD], data, length); if (type == HCI_PACKET_TYPE_ACL_DATA) return WriteSafely(uart_fds_[CH_ACL_OUT], data, length); - CHECK(type == HCI_PACKET_TYPE_COMMAND || type == HCI_PACKET_TYPE_ACL_DATA); + LOG_ALWAYS_FATAL("%s: Unimplemented packet type = %d", __func__, type); return 0; } diff --git a/bluetooth/1.0/default/test/bluetooth_address_test.cc b/bluetooth/1.0/default/test/bluetooth_address_test.cc index adcd9c157c..e60729e8af 100644 --- a/bluetooth/1.0/default/test/bluetooth_address_test.cc +++ b/bluetooth/1.0/default/test/bluetooth_address_test.cc @@ -15,6 +15,7 @@ // #include +#include #include #include diff --git a/bluetooth/1.0/default/vendor_interface.cc b/bluetooth/1.0/default/vendor_interface.cc index a291e149e1..ffc283e465 100644 --- a/bluetooth/1.0/default/vendor_interface.cc +++ b/bluetooth/1.0/default/vendor_interface.cc @@ -17,7 +17,6 @@ #include "vendor_interface.h" #define LOG_TAG "android.hardware.bluetooth@1.0-impl" -#include #include #include @@ -163,14 +162,16 @@ bool VendorInterface::Initialize( InitializeCompleteCallback initialize_complete_cb, PacketReadCallback event_cb, PacketReadCallback acl_cb, PacketReadCallback sco_cb) { - CHECK(!g_vendor_interface); + LOG_ALWAYS_FATAL_IF(g_vendor_interface, "%s: No previous Shutdown()?", + __func__); g_vendor_interface = new VendorInterface(); return g_vendor_interface->Open(initialize_complete_cb, event_cb, acl_cb, sco_cb); } void VendorInterface::Shutdown() { - CHECK(g_vendor_interface); + LOG_ALWAYS_FATAL_IF(!g_vendor_interface, "%s: No Vendor interface!", + __func__); g_vendor_interface->Close(); delete g_vendor_interface; g_vendor_interface = nullptr; @@ -204,7 +205,9 @@ bool VendorInterface::Open(InitializeCompleteCallback initialize_complete_cb, // Get the local BD address uint8_t local_bda[BluetoothAddress::kBytes]; - CHECK(BluetoothAddress::get_local_address(local_bda)); + if (!BluetoothAddress::get_local_address(local_bda)) { + LOG_ALWAYS_FATAL("%s: No Bluetooth Address!", __func__); + } int status = lib_interface_->init(&lib_callbacks, (unsigned char*)local_bda); if (status) { ALOGE("%s unable to initialize vendor library: %d", __func__, status);