Clean up errno logs and sto* conversions

I learned that we should be using PLOG to log errno strings, and we
should be avoiding stoi, stol, etc... conversions and instead use the
built in Android ParseInt/ParseUint functions.

Bug: 150250606
Bug: 150245058
Test: Manual for CLI tools, VTS for everything else
Merged-In: Icdd8a6af8564d5de3bedd1bc934f7928eb5e66e9
Change-Id: Icdd8a6af8564d5de3bedd1bc934f7928eb5e66e9
(cherry picked from commit 1173a7253b)
This commit is contained in:
chrisweir 2020-02-26 14:39:56 -08:00 committed by Chris Weir
parent 0df912df57
commit 75a80f6b60
7 changed files with 32 additions and 30 deletions

View file

@ -56,7 +56,7 @@ ICanController::Result CanBusSlcan::updateIfaceName(base::unique_fd& uartFd) {
* that has already been configured and brought up.
*/
if (ioctl(uartFd.get(), SIOCGIFNAME, ifrequest.ifr_name) < 0) {
LOG(ERROR) << "Failed to get the name of the created device: " << strerror(errno);
PLOG(ERROR) << "Failed to get the name of the created device";
return ICanController::Result::UNKNOWN_ERROR;
}
@ -80,7 +80,7 @@ ICanController::Result CanBusSlcan::preUp() {
* controlling terminal */
mFd = base::unique_fd(open(mUartName.c_str(), O_RDWR | O_NONBLOCK | O_NOCTTY));
if (!mFd.ok()) {
LOG(ERROR) << "SLCAN Failed to open " << mUartName << ": " << strerror(errno);
PLOG(ERROR) << "SLCAN Failed to open " << mUartName;
return ICanController::Result::BAD_INTERFACE_ID;
}
@ -92,7 +92,7 @@ ICanController::Result CanBusSlcan::preUp() {
// blank terminal settings and pull them from the device
struct termios terminalSettings = {};
if (tcgetattr(mFd.get(), &terminalSettings) < 0) {
LOG(ERROR) << "Failed to read attrs of" << mUartName << ": " << strerror(errno);
PLOG(ERROR) << "Failed to read attrs of" << mUartName;
return ICanController::Result::UNKNOWN_ERROR;
}
@ -107,42 +107,40 @@ ICanController::Result CanBusSlcan::preUp() {
struct serial_struct serialSettings;
// get serial settings
if (ioctl(mFd.get(), TIOCGSERIAL, &serialSettings) < 0) {
LOG(ERROR) << "Failed to read serial settings from " << mUartName << ": "
<< strerror(errno);
PLOG(ERROR) << "Failed to read serial settings from " << mUartName;
return ICanController::Result::UNKNOWN_ERROR;
}
// set low latency mode
serialSettings.flags |= ASYNC_LOW_LATENCY;
// apply serial settings
if (ioctl(mFd.get(), TIOCSSERIAL, &serialSettings) < 0) {
LOG(ERROR) << "Failed to set low latency mode on " << mUartName << ": " << strerror(errno);
PLOG(ERROR) << "Failed to set low latency mode on " << mUartName;
return ICanController::Result::UNKNOWN_ERROR;
}
/* TCSADRAIN applies settings after we finish writing the rest of our
* changes (as opposed to TCSANOW, which changes immediately) */
if (tcsetattr(mFd.get(), TCSADRAIN, &terminalSettings) < 0) {
LOG(ERROR) << "Failed to apply terminal settings to " << mUartName << ": "
<< strerror(errno);
PLOG(ERROR) << "Failed to apply terminal settings to " << mUartName;
return ICanController::Result::UNKNOWN_ERROR;
}
// apply speed setting for CAN
if (write(mFd.get(), canBitrateCommand->c_str(), canBitrateCommand->length()) <= 0) {
LOG(ERROR) << "Failed to apply CAN bitrate: " << strerror(errno);
PLOG(ERROR) << "Failed to apply CAN bitrate";
return ICanController::Result::UNKNOWN_ERROR;
}
// set open flag TODO: also support listen only
if (write(mFd.get(), slcanprotocol::kOpenCommand.c_str(),
slcanprotocol::kOpenCommand.length()) <= 0) {
LOG(ERROR) << "Failed to set open flag: " << strerror(errno);
PLOG(ERROR) << "Failed to set open flag";
return ICanController::Result::UNKNOWN_ERROR;
}
// set line discipline to slcan
if (ioctl(mFd.get(), TIOCSETD, &slcanprotocol::kSlcanDiscipline) < 0) {
LOG(ERROR) << "Failed to set line discipline to slcan: " << strerror(errno);
PLOG(ERROR) << "Failed to set line discipline to slcan";
return ICanController::Result::UNKNOWN_ERROR;
}

View file

@ -67,7 +67,7 @@ CanSocket::~CanSocket() {
bool CanSocket::send(const struct canfd_frame& frame) {
const auto res = write(mSocket.get(), &frame, CAN_MTU);
if (res < 0) {
LOG(DEBUG) << "CanSocket send failed: " << errno;
PLOG(DEBUG) << "CanSocket send failed";
return false;
}
if (res != CAN_MTU) {
@ -102,7 +102,7 @@ void CanSocket::readerThread() {
const auto sel = selectRead(mSocket, kReadPooling);
if (sel == 0) continue; // timeout
if (sel == -1) {
LOG(ERROR) << "Select failed: " << errno;
PLOG(ERROR) << "Select failed";
break;
}
@ -130,7 +130,7 @@ void CanSocket::readerThread() {
if (errno == EAGAIN) continue;
errnoCopy = errno;
LOG(ERROR) << "Failed to read CAN packet: " << strerror(errno) << " (" << errno << ")";
PLOG(ERROR) << "Failed to read CAN packet";
break;
}

View file

@ -23,7 +23,7 @@ namespace android::netdevice {
NetlinkSocket::NetlinkSocket(int protocol) {
mFd.reset(socket(AF_NETLINK, SOCK_RAW, protocol));
if (!mFd.ok()) {
LOG(ERROR) << "Can't open Netlink socket: " << errno;
PLOG(ERROR) << "Can't open Netlink socket";
mFailed = true;
return;
}
@ -32,7 +32,7 @@ NetlinkSocket::NetlinkSocket(int protocol) {
sa.nl_family = AF_NETLINK;
if (bind(mFd.get(), reinterpret_cast<struct sockaddr*>(&sa), sizeof(sa)) < 0) {
LOG(ERROR) << "Can't bind Netlink socket: " << errno;
PLOG(ERROR) << "Can't bind Netlink socket";
mFd.reset();
mFailed = true;
}
@ -57,7 +57,7 @@ bool NetlinkSocket::send(struct nlmsghdr* nlmsg) {
msg.msg_iovlen = 1;
if (sendmsg(mFd.get(), &msg, 0) < 0) {
LOG(ERROR) << "Can't send Netlink message: " << errno;
PLOG(ERROR) << "Can't send Netlink message";
return false;
}
return true;
@ -79,7 +79,7 @@ bool NetlinkSocket::receiveAck() {
const ssize_t status = recvmsg(mFd.get(), &msg, 0);
if (status < 0) {
LOG(ERROR) << "Failed to receive Netlink message: " << errno;
PLOG(ERROR) << "Failed to receive Netlink message";
return false;
}
size_t remainingLen = status;

View file

@ -48,7 +48,7 @@ base::unique_fd socket(const std::string& ifname) {
}
if (setsockopt(sock.get(), SOL_CAN_RAW, CAN_RAW_ERR_FILTER, &kErrMask, sizeof(kErrMask)) < 0) {
LOG(ERROR) << "Can't receive error frames, CAN setsockpt failed: " << strerror(errno);
PLOG(ERROR) << "Can't receive error frames, CAN setsockpt failed";
return {};
}

View file

@ -41,7 +41,7 @@ static bool sendIfreq(unsigned long request, struct ifreq& ifr) {
}
if (ioctl(sock.get(), request, &ifr) < 0) {
LOG(ERROR) << "ioctl(" << std::hex << request << std::dec << ") failed: " << errno;
PLOG(ERROR) << "ioctl(" << std::hex << request << std::dec << ") failed";
return false;
}

View file

@ -15,6 +15,7 @@
*/
#include <android-base/logging.h>
#include <android-base/parseint.h>
#include <android/hardware/automotive/can/1.0/ICanController.h>
#include <android/hidl/manager/1.2/IServiceManager.h>
#include <hidl-utils/hidl-utils.h>
@ -88,8 +89,8 @@ static int up(const std::string& busName, ICanController::InterfaceType type,
slcan.ttyname(interface);
config.interfaceId.slcan(slcan);
} else if (type == ICanController::InterfaceType::INDEXED) {
auto idx = std::stol(interface);
if (idx < 0 || idx > UINT8_MAX) {
unsigned idx;
if (!android::base::ParseUint(interface, &idx, unsigned(UINT8_MAX))) {
std::cerr << "Interface index out of range: " << idx;
return -1;
}
@ -162,9 +163,11 @@ static int main(int argc, char* argv[]) {
return -1;
}
long long bitrate = 0;
if (argc == 4) {
bitrate = std::stoll(argv[3]);
uint32_t bitrate = 0;
if (argc == 4 && !android::base::ParseUint(argv[3], &bitrate)) {
std::cerr << "Invalid bitrate!" << std::endl;
usage();
return -1;
}
return up(busName, *type, interface, bitrate);

View file

@ -81,16 +81,17 @@ static std::optional<std::tuple<V1_0::CanMessageId, std::vector<uint8_t>>> parse
const std::string msgidStr = msg.substr(0, hashpos);
const std::string payloadStr = msg.substr(hashpos + 1);
size_t idx = 0;
V1_0::CanMessageId msgid = std::stoi(msgidStr, &idx, 16);
if (msgidStr[idx] != '\0') return std::nullopt;
V1_0::CanMessageId msgid;
// "0x" must be prepended to msgidStr, since ParseUint doesn't accept a base argument.
if (!android::base::ParseUint("0x" + msgidStr, &msgid)) return std::nullopt;
std::vector<uint8_t> payload;
if (payloadStr.size() % 2 != 0) return std::nullopt;
for (size_t i = 0; i < payloadStr.size(); i += 2) {
std::string byteStr(payloadStr, i, 2);
payload.emplace_back(std::stoi(byteStr, &idx, 16));
if (byteStr[idx] != '\0') return std::nullopt;
uint8_t byteBuf;
if (!android::base::ParseUint("0x" + byteStr, &byteBuf)) return std::nullopt;
payload.emplace_back(byteBuf);
}
return {{msgid, payload}};