simplify error handling. CrashWatcher.cpp lost a few wrapper functions

-4k of code

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

9 files changed, 84 insertions, 163 deletions

diff --git a/lib/Plugins/FileTransfer.cpp b/lib/Plugins/FileTransfer.cpp
index dbda17fa..76dbb07c 100644
--- a/lib/Plugins/FileTransfer.cpp
+++ b/lib/Plugins/FileTransfer.cpp
@@ -106,8 +106,7 @@ void CFileTransfer::SendFile(const std::string& pURL,
         fclose(f);
     }
     /*retry the upload if not succesful, wait a bit before next try*/
-    while( result!=0 && count-- != 0 && (sleep(m_nRetryDelay),1) );
-
+    while (result != 0 && --count >= 0 && (sleep(m_nRetryDelay), 1));
 }
 
 void CFileTransfer::CreateArchive(const std::string& pArchiveName,
diff --git a/lib/Plugins/SQLite3.cpp b/lib/Plugins/SQLite3.cpp
index 57b33172..a57da240 100644
--- a/lib/Plugins/SQLite3.cpp
+++ b/lib/Plugins/SQLite3.cpp
@@ -333,7 +333,6 @@ void CSQLite3::SetReported(const std::string& pUUID, const std::string& pUID, co
                  "SET "DATABASE_COLUMN_MESSAGE" = '" + pMessage + "' "
                  "WHERE "DATABASE_COLUMN_UUID" = '"+pUUID+"' "
                  "AND "DATABASE_COLUMN_UID" = '"+pUID+"';");
-
     }
     else
     {
diff --git a/src/Daemon/CommLayerServer.cpp b/src/Daemon/CommLayerServer.cpp
index ed47c2ac..d61da399 100644
--- a/src/Daemon/CommLayerServer.cpp
+++ b/src/Daemon/CommLayerServer.cpp
@@ -3,6 +3,7 @@
 
 CCommLayerServer::CCommLayerServer()
 {
+    m_init_error = 0;
 }
 
 CCommLayerServer::~CCommLayerServer()
diff --git a/src/Daemon/CommLayerServer.h b/src/Daemon/CommLayerServer.h
index 3a1177db..34700c43 100644
--- a/src/Daemon/CommLayerServer.h
+++ b/src/Daemon/CommLayerServer.h
@@ -7,6 +7,8 @@
 
 class CCommLayerServer {
     public:
+        int m_init_error;
+
         CCommLayerServer();
         virtual ~CCommLayerServer();
 
diff --git a/src/Daemon/CommLayerServerDBus.cpp b/src/Daemon/CommLayerServerDBus.cpp
index 8077757e..a1758ca5 100644
--- a/src/Daemon/CommLayerServerDBus.cpp
+++ b/src/Daemon/CommLayerServerDBus.cpp
@@ -2,6 +2,7 @@
 #include "abrtlib.h"
 #include "ABRTException.h"
 #include "CrashWatcher.h"
+#include "Daemon.h"
 #include "CommLayerServerDBus.h"
 
 void attach_dbus_dispatcher_to_glib_main_context()
@@ -29,11 +30,9 @@ CCommLayerServerDBus::CCommLayerServerDBus()
     }
     catch (DBus::Error err)
     {
-        throw CABRTException(EXCEP_FATAL, std::string(__func__) +
-                             "\nPlease check if:\n"
-                             + " * abrt is being run with root permissions\n"
-                             + " * you have reloaded the dbus\n"+
-                             + "Original exception was:\n " + err.what());
+        error_msg("error requesting DBus name %s, possible reasons: abrt run by non-root; dbus config is incorrect", CC_DBUS_NAME);
+        m_init_error = 1;
+        return;
     }
     register_method(CCommLayerServerDBus, GetCrashInfos, _GetCrashInfos_stub);
     register_method(CCommLayerServerDBus, CreateReport, _CreateReport_stub);
@@ -132,7 +131,7 @@ DBus::Message CCommLayerServerDBus::_GetJobResult_stub(const DBus::CallMessage &
 
 DBus::Message CCommLayerServerDBus::_GetPluginsInfo_stub(const DBus::CallMessage &call)
 {
-    vector_map_string_string_t plugins_info = GetPluginsInfo();
+    vector_map_string_string_t plugins_info = g_pPluginManager->GetPluginsInfo();
 
     DBus::ReturnMessage reply(call);
     DBus::MessageIter wi = reply.writer();
@@ -148,7 +147,7 @@ DBus::Message CCommLayerServerDBus::_GetPluginSettings_stub(const DBus::CallMess
     ri >> PluginName;
 
     unsigned long unix_uid = m_pConn->sender_unix_uid(call.sender());
-    map_plugin_settings_t plugin_settings = GetPluginSettings(PluginName, to_string(unix_uid));
+    map_plugin_settings_t plugin_settings = g_pPluginManager->GetPluginSettings(PluginName, to_string(unix_uid));
 
     DBus::ReturnMessage reply(call);
     DBus::MessageIter wi = reply.writer();
@@ -165,7 +164,7 @@ DBus::Message CCommLayerServerDBus::_SetPluginSettings_stub(const DBus::CallMess
     ri >> plugin_settings;
 
     unsigned long unix_uid = m_pConn->sender_unix_uid(call.sender());
-    SetPluginSettings(PluginName, to_string(unix_uid), plugin_settings);
+    g_pPluginManager->SetPluginSettings(PluginName, to_string(unix_uid), plugin_settings);
 
     DBus::ReturnMessage reply(call);
     return reply;
@@ -177,7 +176,7 @@ DBus::Message CCommLayerServerDBus::_RegisterPlugin_stub(const DBus::CallMessage
     std::string PluginName;
     ri >> PluginName;
 
-    RegisterPlugin(PluginName);
+    g_pPluginManager->RegisterPlugin(PluginName);
 
     DBus::ReturnMessage reply(call);
     //DBus::MessageIter wi = reply.writer();
@@ -191,7 +190,7 @@ DBus::Message CCommLayerServerDBus::_UnRegisterPlugin_stub(const DBus::CallMessa
     std::string PluginName;
     ri >> PluginName;
 
-    UnRegisterPlugin(PluginName);
+    g_pPluginManager->UnRegisterPlugin(PluginName);
 
     DBus::ReturnMessage reply(call);
     return reply;
diff --git a/src/Daemon/CommLayerServerSocket.cpp b/src/Daemon/CommLayerServerSocket.cpp
index 99fe0c26..3e38a709 100644
--- a/src/Daemon/CommLayerServerSocket.cpp
+++ b/src/Daemon/CommLayerServerSocket.cpp
@@ -170,21 +170,29 @@ CCommLayerServerSocket::CCommLayerServerSocket()
     struct sockaddr_un local;
 
     unlink(SOCKET_FILE);
-    if ((m_nSocket = socket(AF_UNIX, SOCK_STREAM, 0)) == -1)
+    m_nSocket = socket(AF_UNIX, SOCK_STREAM, 0);
+    if (m_nSocket == -1)
     {
-        throw CABRTException(EXCEP_FATAL, "CCommLayerServerSocket::CCommLayerServerSocket(): Can not create socket.");
+        m_init_error = 1;
+        perror_msg("CCommLayerServerSocket: can't create AF_UNIX socket");
+        return;
     }
     fcntl(m_nSocket, F_SETFD, FD_CLOEXEC);
+
+    memset(&local, 0, sizeof(local));
     local.sun_family = AF_UNIX;
     strcpy(local.sun_path, SOCKET_FILE);
-    len = strlen(local.sun_path) + sizeof(local.sun_family);
-    if (bind(m_nSocket, (struct sockaddr *)&local, len) == -1)
+    if (bind(m_nSocket, (struct sockaddr *)&local, sizeof(local)) == -1)
     {
-        throw CABRTException(EXCEP_FATAL, "CCommLayerServerSocket::CCommLayerServerSocket(): Can not bind to the socket.");
+        m_init_error = 1;
+        perror_msg("CCommLayerServerSocket: can't bind AF_UNIX socket to '%s'", SOCKET_FILE);
+        return;
     }
     if (listen(m_nSocket, 5) == -1)
     {
-        throw CABRTException(EXCEP_FATAL, "CCommLayerServerSocket::CCommLayerServerSocket(): Can not listen on the socket.");
+        m_init_error = 1;
+        perror_msg("CCommLayerServerSocket: can't listen on AF_UNIX socket");
+        return;
     }
     chmod(SOCKET_FILE, SOCKET_PERMISSION);
 
@@ -194,7 +202,9 @@ CCommLayerServerSocket::CCommLayerServerSocket()
                         static_cast<GIOFunc>(server_socket_cb),
                         this))
     {
-        throw CABRTException(EXCEP_FATAL, "CCommLayerServerSocket::CCommLayerServerSocket(): Can not init g_io_channel.");
+        m_init_error = 1;
+        perror_msg("CCommLayerServerSocket: can't hook AF_UNIX socket to glb main loop");
+        return;
     }
 }
 
diff --git a/src/Daemon/CrashWatcher.cpp b/src/Daemon/CrashWatcher.cpp
index 67121a67..06f05ee8 100644
--- a/src/Daemon/CrashWatcher.cpp
+++ b/src/Daemon/CrashWatcher.cpp
@@ -81,8 +81,8 @@ vector_crash_infos_t GetCrashInfos(const std::string &pUID)
                 case MW_FILE_ERROR:
                     {
                         std::string debugDumpDir;
-                        warn_client("Can not open file in debug dump directory for UUID: " + UUIDsUIDs[ii].first + ", deleting ");
-                        update_client("Can not open file in debug dump directory for UUID: " + UUIDsUIDs[ii].first + ", deleting ");
+                        warn_client("Can not open file in debug dump directory for UUID: " + UUIDsUIDs[ii].first + ", deleting");
+                        update_client("Can not open file in debug dump directory for UUID: " + UUIDsUIDs[ii].first + ", deleting");
                         debugDumpDir = DeleteCrashInfo(UUIDsUIDs[ii].first, UUIDsUIDs[ii].second);
                         DeleteDebugDumpDir(debugDumpDir);
                     }
@@ -187,7 +187,7 @@ uint64_t CreateReport_t(const std::string& pUUID, const std::string& pUID, const
         free(thread_data->dest);
         free(thread_data);
         /* The only reason this may happen is system-wide resource starvation,
-         * or ulimit is exceeded (someoune floods us with CreateReport() Dbus calls?)
+         * or ulimit is exceeded (someoune floods us with CreateReport() dbus calls?)
          */
         error_msg("cannot create thread");
         return 0;
@@ -224,89 +224,3 @@ map_crash_report_t GetJobResult(uint64_t pJobID, const std::string& pSender)
     */
     return g_pending_jobs[pSender][pJobID];
 }
-
-vector_map_string_string_t GetPluginsInfo()
-{
-    try
-    {
-        return g_pPluginManager->GetPluginsInfo();
-    }
-    catch (CABRTException &e)
-    {
-        if (e.type() == EXCEP_FATAL)
-        {
-            throw e;
-        }
-        warn_client(e.what());
-    }
-    // TODO: is it right? I added it just to disable a warning...
-    // but maybe returning empty map is wrong here?
-    return vector_map_string_string_t();
-}
-
-map_plugin_settings_t GetPluginSettings(const std::string& pName, const std::string& pUID)
-{
-    try
-    {
-        return g_pPluginManager->GetPluginSettings(pName, pUID);
-    }
-    catch(CABRTException &e)
-    {
-        if (e.type() == EXCEP_FATAL)
-        {
-            throw e;
-        }
-        warn_client(e.what());
-    }
-    // TODO: is it right? I added it just to disable a warning...
-    // but maybe returning empty map is wrong here?
-    return map_plugin_settings_t();
-}
-
-void RegisterPlugin(const std::string& pName)
-{
-    try
-    {
-        g_pPluginManager->RegisterPlugin(pName);
-    }
-    catch(CABRTException &e)
-    {
-        if (e.type() == EXCEP_FATAL)
-        {
-            throw e;
-        }
-        warn_client(e.what());
-    }
-}
-
-void UnRegisterPlugin(const std::string& pName)
-{
-    try
-    {
-        g_pPluginManager->UnRegisterPlugin(pName);
-    }
-    catch(CABRTException &e)
-    {
-        if (e.type() == EXCEP_FATAL)
-        {
-            throw e;
-        }
-        warn_client(e.what());
-    }
-}
-
-void SetPluginSettings(const std::string& pName, const std::string& pUID, const map_plugin_settings_t& pSettings)
-{
-    try
-    {
-        g_pPluginManager->SetPluginSettings(pName, pUID, pSettings);
-    }
-    catch(CABRTException &e)
-    {
-        if (e.type() == EXCEP_FATAL)
-        {
-            throw e;
-        }
-        warn_client(e.what());
-    }
-}
diff --git a/src/Daemon/CrashWatcher.h b/src/Daemon/CrashWatcher.h
index b2e5341b..4fd9926e 100644
--- a/src/Daemon/CrashWatcher.h
+++ b/src/Daemon/CrashWatcher.h
@@ -60,11 +60,4 @@ uint64_t CreateReport_t(const std::string& pUUID, const std::string& pUID, const
 bool DeleteDebugDump(const std::string& pUUID, const std::string& pUID);
 map_crash_report_t GetJobResult(uint64_t pJobID, const std::string& pSender);
 
-/* plugins related */
-vector_map_string_string_t GetPluginsInfo();
-map_plugin_settings_t GetPluginSettings(const std::string& pName, const std::string& pUID);
-void SetPluginSettings(const std::string& pName, const std::string& pUID, const map_plugin_settings_t& pSettings);
-void RegisterPlugin(const std::string& pName);
-void UnRegisterPlugin(const std::string& pName);
-
 #endif /*CRASHWATCHER_H_*/
diff --git a/src/Daemon/Daemon.cpp b/src/Daemon/Daemon.cpp
index a7252ac9..1ee53645 100644
--- a/src/Daemon/Daemon.cpp
+++ b/src/Daemon/Daemon.cpp
@@ -72,37 +72,31 @@ static bool dot_or_dotdot(const char *filename)
 
 static double GetDirSize(const std::string &pPath)
 {
-    double size = 0;
+    DIR *dp = opendir(pPath.c_str());
+    if (dp == NULL)
+        return 0;
+
     struct dirent *ep;
     struct stat stats;
-    DIR *dp;
-
-    dp = opendir(pPath.c_str());
-    if (dp != NULL)
+    double size = 0;
+    while ((ep = readdir(dp)) != NULL)
     {
-        while ((ep = readdir(dp)) != NULL)
+        if (dot_or_dotdot(ep->d_name))
+            continue;
+        std::string dname = pPath + "/" + ep->d_name;
+        if (lstat(dname.c_str(), &stats) == 0)
         {
-            if (dot_or_dotdot(ep->d_name))
-                continue;
-            std::string dname = pPath + "/" + ep->d_name;
-            if (lstat(dname.c_str(), &stats) == 0)
+            if (S_ISDIR(stats.st_mode))
             {
-                if (S_ISDIR(stats.st_mode))
-                {
-                    size += GetDirSize(dname);
-                }
-                else if (S_ISREG(stats.st_mode))
-                {
-                    size += stats.st_size;
-                }
+                size += GetDirSize(dname);
+            }
+            else if (S_ISREG(stats.st_mode))
+            {
+                size += stats.st_size;
             }
         }
-        closedir(dp);
-    }
-    else
-    {
-        throw CABRTException(EXCEP_FATAL, std::string(__func__) + ": Init Failed");
     }
+    closedir(dp);
     return size;
 }
 
@@ -178,7 +172,7 @@ static void SetUpMW()
     }
 }
 
-static void SetUpCron()
+static int SetUpCron()
 {
     map_cron_t::iterator it_c = g_settings_mapCron.begin();
     for (; it_c != g_settings_mapCron.end(); it_c++)
@@ -239,7 +233,9 @@ static void SetUpCron()
             time_t nextTime = mktime(&locTime);
             if (nextTime == ((time_t)-1))
             {
-                throw CABRTException(EXCEP_FATAL, "SetUpCron(): Cannot set up time");
+                /* paranoia */
+                perror_msg("can't set up cron time");
+                return -1;
             }
             if (actTime > nextTime)
             {
@@ -252,7 +248,6 @@ static void SetUpCron()
             vector_pair_strings_t::iterator it_ar;
             for (it_ar = it_c->second.begin(); it_ar != it_c->second.end(); it_ar++)
             {
-
                 cron_callback_data_t* cronOneCallbackData = new cron_callback_data_t((*it_ar).first, (*it_ar).second, timeout);
                 g_timeout_add_seconds_full(G_PRIORITY_DEFAULT,
                                            timeout,
@@ -268,27 +263,28 @@ static void SetUpCron()
             }
         }
     }
+    return 0;
 }
 
-static void FindNewDumps(const std::string& pPath)
+static int FindNewDumps(const std::string& pPath)
 {
     log("Scanning for unsaved entries...");
     struct stat stats;
     DIR *dp;
     std::vector<std::string> dirs;
-    std::string dname;
     // get potential unsaved debugdumps
     dp = opendir(pPath.c_str());
     if (dp == NULL)
     {
-        throw CABRTException(EXCEP_FATAL, "FindNewDumps(): Couldn't open the directory:" + pPath);
+        perror_msg("can't open directory '%s'", pPath.c_str());
+        return -1;
     }
     struct dirent *ep;
     while ((ep = readdir(dp)))
     {
         if (dot_or_dotdot(ep->d_name))
             continue;
-        dname = pPath + "/" + ep->d_name;
+        std::string dname = pPath + "/" + ep->d_name;
         if (lstat(dname.c_str(), &stats) == 0)
         {
             if (S_ISDIR(stats.st_mode))
@@ -337,9 +333,10 @@ static void FindNewDumps(const std::string& pPath)
             warn_client(e.what());
         }
     }
+    return 0;
 }
 
-static void CreatePidFile()
+static int CreatePidFile()
 {
     int fd;
 
@@ -355,27 +352,29 @@ static void CreatePidFile()
         int len = sprintf(buf, "%u\n", (unsigned)getpid());
         write(fd, buf, len);
         close(fd);
-        return;
+        return 0;
     }
 
     /* something went wrong */
-    throw CABRTException(EXCEP_FATAL, "can not open pid file");
+    perror_msg("can't open '%s'", VAR_RUN_PIDFILE);
+    return -1;
 }
 
-static void Lock()
+static int Lock()
 {
-    int lfp = open(VAR_RUN_LOCK_FILE, O_RDWR|O_CREAT, 0640);
-    if (lfp < 0)
+    int lfd = open(VAR_RUN_LOCK_FILE, O_RDWR|O_CREAT, 0640);
+    if (lfd < 0)
     {
-        throw CABRTException(EXCEP_FATAL, "can not open lock file");
+        perror_msg("can't open '%s'", VAR_RUN_LOCK_FILE);
+        return -1;
     }
-    if (lockf(lfp, F_TLOCK, 0) < 0)
+    if (lockf(lfd, F_TLOCK, 0) < 0)
     {
-        throw CABRTException(EXCEP_FATAL, "cannot create lock on lockfile");
+        perror_msg("can't lock file '%s'", VAR_RUN_LOCK_FILE);
+        return -1;
     }
-    /* only first instance continues */
-    //sprintf(str,"%d\n",getpid());
-    //write(lfp,str,strlen(str)); /* record pid to lockfile */
+    return 0;
+    /* we leak opened lfd intentionally */
 }
 
 static void handle_fatal_signal(int signal)
@@ -427,10 +426,11 @@ static gboolean handle_event_cb(GIOChannel *gio, GIOCondition condition, gpointe
     char *buf = new char[INOTIFY_BUFF_SIZE];
     gsize len;
     gsize i = 0;
+    errno = 0;
     err = g_io_channel_read(gio, buf, INOTIFY_BUFF_SIZE, &len);
     if (err != G_IO_ERROR_NONE)
     {
-        warn_client("Error reading inotify fd.");
+        perror_msg("Error reading inotify fd");
         delete[] buf;
         return FALSE;
     }
@@ -585,8 +585,10 @@ int main(int argc, char** argv)
         g_pPluginManager = new CPluginManager();
         g_pPluginManager->LoadPlugins();
         SetUpMW();
-        SetUpCron();
-        FindNewDumps(DEBUG_DUMPS_DIR);
+        if (SetUpCron() != 0)
+            throw 1;
+        if (FindNewDumps(DEBUG_DUMPS_DIR) != 0)
+            throw 1;
         /* (comment here) */
 #ifdef ENABLE_DBUS
         attach_dbus_dispatcher_to_glib_main_context();
@@ -594,6 +596,8 @@ int main(int argc, char** argv)
 #elif ENABLE_SOCKET
         g_pCommLayer = new CCommLayerServerSocket();
 #endif
+        if (g_pCommLayer->m_init_error)
+            throw 1;
         /* (comment here) */
         pGio = g_io_channel_unix_new(inotify_fd);
         g_io_add_watch(pGio, G_IO_IN, handle_event_cb, NULL);
@@ -607,8 +611,8 @@ int main(int argc, char** argv)
         GSource *waitsignal_src = (GSource*) g_source_new(&waitsignal_funcs, sizeof(*waitsignal_src));
         g_source_attach(waitsignal_src, g_main_context_default());
         /* Mark the territory */
-        Lock();
-        CreatePidFile();
+        if (Lock() != 0 || CreatePidFile() != 0)
+            throw 1;
     }
     catch (...)
     {