From e33c52cbc771055e1200c770ed2c0288cd69264b Mon Sep 17 00:00:00 2001
From: Piotr Sawicki
Date: Tue, 11 Jul 2017 08:54:20 +0200
Subject: [PATCH] Add extensive logging to Sock and channel classes
* add logging of unusual situations
* add handling of unlink() errors
Change-Id: Ibc41a6bc1df29531f6405433f15817abe0bcc250
---
src/ipc-lib/ask-user-channel.cpp | 9 ++++----
src/ipc-lib/ask-user-client-channel.cpp | 10 +++++++--
src/ipc-lib/ask-user-server-channel.cpp | 18 +++++++++++----
src/ipc-lib/sock.cpp | 39 +++++++++++++++++++++++++++------
4 files changed, 59 insertions(+), 17 deletions(-)
diff --git a/src/ipc-lib/ask-user-channel.cpp b/src/ipc-lib/ask-user-channel.cpp
index 79c8a81..fe88252 100644
--- a/src/ipc-lib/ask-user-channel.cpp
+++ b/src/ipc-lib/ask-user-channel.cpp
@@ -38,7 +38,7 @@ int Channel::process(int fd, int mask) {
try {
auto it = m_sockets.find(fd);
if (it == m_sockets.end()) {
- ALOGE("Cannot find file descriptor " << fd);
+ ALOGE("process: Cannot find file descriptor " << fd);
return -EBADF;
}
@@ -110,7 +110,7 @@ int Channel::process(int fd, int mask) {
int result = desc.sock.send(desc.output);
if (result < 0) {
closeConnection(fd);
- ALOGE("Cannot send a message on file descritptor: " << fd);
+ ALOGE("Cannot send a message on file descriptor: " << fd);
return -ENOTCONN;
}
@@ -125,11 +125,11 @@ int Channel::process(int fd, int mask) {
}
} catch (const std::exception &e) {
closeConnection(fd);
- ALOGE("An exception occured while processing channel " << e.what());
+ ALOGE("An exception occurred while processing channel " << e.what());
throw;
} catch (...) {
closeConnection(fd);
- ALOGE("Unhandled exception while processing channel");
+ ALOGE("Unknown exception occurred while processing channel");
throw;
}
@@ -157,6 +157,7 @@ void Channel::closeConnection(int fd)
{
auto it = m_sockets.find(fd);
if (it == m_sockets.end()) {
+ ALOGE("closeConnection: Cannot find file descriptor fd: " << fd);
return;
}
diff --git a/src/ipc-lib/ask-user-client-channel.cpp b/src/ipc-lib/ask-user-client-channel.cpp
index 035d503..d79b81c 100644
--- a/src/ipc-lib/ask-user-client-channel.cpp
+++ b/src/ipc-lib/ask-user-client-channel.cpp
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include
#include
@@ -83,16 +84,20 @@ void ClientChannel::onClose(int fd) {
}
int ClientChannel::onReceive(UNUSED int fd, std::vector &&message) {
- if (!message.size())
+ if (!message.size()) {
+ ALOGE("Server received empty message");
return -EINVAL;
+ }
int command = std::stoi(message[0]);
switch (command) {
case MSGID_POPUP_RESPONSE:
{
- if (message.size() != 3)
+ if (message.size() != 3) {
+ ALOGE("Inappropriate message size for MSGID_POPUP_RESPONSE command, size: " << message.size());
return -EINVAL;
+ }
RequestId id = std::stoi(message[1]);
int response = std::stoi(message[2]);
@@ -101,6 +106,7 @@ int ClientChannel::onReceive(UNUSED int fd, std::vector &&message)
break;
}
default :
+ ALOGE("Client received unknown message, command: " << command);
return -EINVAL;
}
diff --git a/src/ipc-lib/ask-user-server-channel.cpp b/src/ipc-lib/ask-user-server-channel.cpp
index d6578f3..4a08a48 100644
--- a/src/ipc-lib/ask-user-server-channel.cpp
+++ b/src/ipc-lib/ask-user-server-channel.cpp
@@ -24,6 +24,7 @@
#include
#include
+#include
#include
#include
@@ -57,8 +58,10 @@ void ServerChannel::init() {
void ServerChannel::popupResponse(ConnectionFd fd, RequestId id, int response) {
try {
auto it = m_sockets.find(fd);
- if (it == m_sockets.end())
+ if (it == m_sockets.end()) {
+ ALOGE("popupResponse: Cannot find file descriptor " << fd);
return;
+ }
auto &desc = it->second;
@@ -68,7 +71,9 @@ void ServerChannel::popupResponse(ConnectionFd fd, RequestId id, int response) {
std::copy(o.begin(), o.end(), std::back_inserter(desc.output));
m_callbacks->updateConnection(fd, FdMask::READ | FdMask::WRITE);
- } catch (const std::exception &){}
+ } catch (const std::exception &e){
+ ALOGE("Exception occurred during popupResponse: " << e.what());
+ }
}
void ServerChannel::onAccept(int fd) {
@@ -81,16 +86,20 @@ void ServerChannel::onClose(int fd) {
}
int ServerChannel::onReceive(int fd, std::vector &&message) {
- if (!message.size())
+ if (!message.size()) {
+ ALOGE("Server received empty message");
return -EINVAL;
+ }
int command = std::stoi(message[0]);
switch (command) {
case MSGID_POPUP:
{
- if (message.size() != 3)
+ if (message.size() != 3) {
+ ALOGE("Inappropriate message size for MSGID_POPUP command, size: " << message.size());
return -EINVAL;
+ }
std::string privilege = base64Decode(message[1]);
RequestId requestId = std::stoi(message[2]);
@@ -99,6 +108,7 @@ int ServerChannel::onReceive(int fd, std::vector &&message) {
break;
}
default :
+ ALOGE("Server received unknown message, command: " << command);
return -EINVAL;
}
diff --git a/src/ipc-lib/sock.cpp b/src/ipc-lib/sock.cpp
index fe8f22b..e0f2df1 100644
--- a/src/ipc-lib/sock.cpp
+++ b/src/ipc-lib/sock.cpp
@@ -16,8 +16,10 @@
/**
* @file sock.cpp
* @author Bartlomiej Grzelewski
+ * @author Piotr Sawicki
* @brief The implementation of Sock.
*/
+#include
#include
#include
#include
@@ -33,6 +35,7 @@
#include
#include
+#include
namespace AskUser {
namespace Protocol {
@@ -80,8 +83,10 @@ int Sock::getSocketFromSystemD() const {
#ifdef BUILD_WITH_SYSTEMD_DAEMON
int n = sd_listen_fds(0);
- if (n < 0)
+ if (n < 0) {
+ ALOGE("sd_listen_fds failed");
return -1;
+ }
for (int fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START+n; ++fd)
if (0 < sd_is_socket_unix(fd, getUnixSockType(), -1, m_path.c_str(), 0))
@@ -91,8 +96,10 @@ int Sock::getSocketFromSystemD() const {
}
int Sock::connect(const std::string &path) {
- if (m_fd != -1)
+ if (m_fd != -1) {
+ ALOGE("Connect was called for already opened socket, file descriptor: " << m_fd);
return -1;
+ }
m_path = path;
@@ -135,14 +142,22 @@ int Sock::connect(const std::string &path) {
}
}
- if (policyUnlink)
- ::unlink(m_path.c_str()); // we ignore return value by design
+ if (policyUnlink) {
+ int ret = ::unlink(m_path.c_str());
+ if (ret < 0 && errno != ENOENT) {
+ close();
+ ALOGE("Unlink failed for path: " << m_path);
+ return -1;
+ }
+ }
if (policySocket)
m_fd = ::socket(AF_UNIX, getUnixSockType(), 0);
- if (m_fd < 0)
+ if (m_fd < 0) {
+ ALOGE("Cannot create UNIX socket");
return -1;
+ }
// remote is used in bind and in connect
sockaddr_un remote;
@@ -151,6 +166,7 @@ int Sock::connect(const std::string &path) {
remote.sun_family = AF_UNIX;
if (path.size() >= sizeof(remote.sun_path)) {
close();
+ ALOGE("Too long address (path) for UNIX socket");
return -1;
}
memcpy(remote.sun_path, path.c_str(), path.size()+1);
@@ -158,17 +174,20 @@ int Sock::connect(const std::string &path) {
if (policyBind && (-1 == ::bind(m_fd, reinterpret_cast(&remote), sizeof(remote)))) {
close();
+ ALOGE("Cannot bind socket, path: " << path);
return -1;
}
if (policyListen && (-1 == ::listen(m_fd, 5))) {
close();
+ ALOGE("Listen failed");
return -1;
}
if (policyConnect && (-1 == TEMP_FAILURE_RETRY(::connect(m_fd, reinterpret_cast(&remote), static_cast(length)))))
{
close();
+ ALOGE("Connection failed, path: " << path);
return -1;
}
return 0;
@@ -184,11 +203,14 @@ Sock Sock::accept() {
int Sock::send(const RawBuffer &buffer) {
static const int flags = MSG_NOSIGNAL | MSG_DONTWAIT;
- if (m_fd < 0)
+ if (m_fd < 0) {
+ ALOGE("Send was called on closed socket");
return -1;
+ }
switch(m_type) {
default:
+ ALOGE("Send was called for inappropriate socket type");
return -1;
case CLI_STREAM:
{
@@ -223,11 +245,14 @@ int Sock::wait(int mask) {
}
int Sock::recv(RawBuffer &output) {
- if (m_fd < 0)
+ if (m_fd < 0) {
+ ALOGE("Receive was called on closed socket");
return -1;
+ }
switch(m_type) {
default:
+ ALOGE("Receive was called for inappropriate socket type");
return -1;
case CLI_STREAM:
case SRV_DGRAM:
--
2.7.4