Drop std::function from try_catch() and friends, deficient edition

When used as an argument to try_catch() and similar functions,
std::function may potentially introduce runtime overhead on the
exception-free path, possibly even allocate (and thus throw
std::bad_alloc).

This can be prevented by rewriting try_catch() as a generic wrapper with
perfect forwarding.

This has been coded deficiently on purpose, refusing to leverage any and
all kinds of bloat reduction opportunities. For the rationale, please
consult code review participants as I have none to give.

  "I'm only following orders."
    - A nameless soldier

Change-Id: I00adf24213a2e6bf8d148db8375a14200c64ff4f

diff --git a/src/common/include/utils.h b/src/common/include/utils.h
index 8ee30be..fa489dd 100644 (file)
--- a/src/common/include/utils.h
+++ b/src/common/include/utils.h
@@ -29,16 +29,20 @@
 #pragma once
 
 #include <algorithm>
-#include <functional>
+#include <cxxabi.h>
+#include <dpl/log/log.h>
+#include <exception>
+#include <iostream>
 #include <memory>
+#include <string>
 #include <sys/wait.h>
 #include <time.h>
 #include <type_traits>
 #include <unistd.h>
 #include <vector>
-#include <string>
 
 #include <credentials.h>
+#include <security-manager-types.h>
 
 #define SECURITY_MANAGER_API __attribute__((visibility("default")))
 
@@ -48,7 +52,31 @@ namespace SecurityManager {
  * Decorator function that performs frequently repeated exception handling in
  * SS client API functions. Accepts lambda expression as an argument.
  */
-int try_catch(const std::function<int()>& func);
+template <class F>
+int try_catch(F &&f) {
+    try {
+        return std::forward<F>(f)();
+    } catch (abi::__forced_unwind &) {
+        throw;
+    } catch (const Exception &e) {
+        LogError("SecurityManager::Exception " << e.DumpToString());
+        std::cerr << "SecurityManager::Exception " << e.DumpToString() << std::endl;
+    } catch (const std::bad_alloc &e) {
+        LogError("Memory allocation failed: " << e.what());
+        std::cerr << "Memory allocation failed: " << e.what() << std::endl;
+        return SECURITY_MANAGER_ERROR_MEMORY;
+    } catch (const std::system_error &e) {
+        LogError("STD system_error: " <<  e.code() << "-" << e.what());
+        std::cerr << "STD system_error: " <<  e.code() << "-" << e.what() << std::endl;
+    } catch (const std::exception &e) {
+        LogError("STD exception " << e.what());
+        std::cerr << "STD exception " << e.what() << std::endl;
+    } catch (...) {
+        LogError("Unknown exception occurred");
+        std::cerr << "Unknown exception occurred" << std::endl;
+    }
+    return SECURITY_MANAGER_ERROR_UNKNOWN;
+}
 
 time_t monotonicCoarseNow();
 

diff --git a/src/common/privilege_db.cpp b/src/common/privilege_db.cpp
index 1031875..f2eea2e 100644 (file)
--- a/src/common/privilege_db.cpp
+++ b/src/common/privilege_db.cpp
@@ -100,11 +100,10 @@ auto prepare(DB::SqlConnection &db, const char *fmt) {
 }
 
 /* Common code for handling SqlConnection exceptions */
-template <typename T>
-T try_catch(const std::function<T()> &func)
-{
+template <class F>
+auto try_catch_db(F &&f) {
     try {
-        return func();
+        return std::forward<F>(f)();
     } catch (DB::SqlConnection::Exception::SyntaxError &e) {
         LogError("Syntax error in command: " << e.DumpToString());
         ThrowMsg(PrivilegeDb::Exception::InternalError,
@@ -120,7 +119,7 @@ T try_catch(const std::function<T()> &func)
     }
 }
 
-void throwDbInitEx(const std::string &errDesc) {
+[[noreturn]] void throwDbInitEx(const std::string &errDesc) {
     auto s = "Database initialization error: " + errDesc;
     LogError(s);
     ThrowMsg(PrivilegeDb::Exception::IOError, s);
@@ -129,7 +128,7 @@ void throwDbInitEx(const std::string &errDesc) {
 template <class F>
 void tryCatchDbInit(F &&f) {
     try {
-        f();
+        std::forward<F>(f)();
     } catch (DB::SqlConnection::Exception::Base &e) {
         throwDbInitEx(e.DumpToString());
     }
@@ -190,28 +189,28 @@ PrivilegeDb::StatementWrapper PrivilegeDb::getStatement(StmtType queryType)
 
 void PrivilegeDb::BeginTransaction(void)
 {
-    try_catch<void>([&] {
+    try_catch_db([&] {
         mSqlConnection.BeginTransaction();
     });
 }
 
 void PrivilegeDb::CommitTransaction(void)
 {
-    try_catch<void>([&] {
+    try_catch_db([&] {
         mSqlConnection.CommitTransaction();
     });
 }
 
 void PrivilegeDb::RollbackTransaction(void)
 {
-    try_catch<void>([&] {
+    try_catch_db([&] {
         mSqlConnection.RollbackTransaction();
     });
 }
 
 bool PrivilegeDb::PkgNameExists(const std::string &pkgName)
 {
-    return try_catch<bool>([&] {
+    return try_catch_db([&] {
         auto command = getStatement(StmtType::EPkgNameExists);
         int cnt = 0;
 
@@ -227,7 +226,7 @@ bool PrivilegeDb::PkgNameExists(const std::string &pkgName)
 
 bool PrivilegeDb::AppNameExists(const std::string &appName)
 {
-    return try_catch<bool>([&] {
+    return try_catch_db([&] {
         auto command = getStatement(StmtType::EAppNameExists);
         int cnt = 0;
 
@@ -243,7 +242,7 @@ bool PrivilegeDb::AppNameExists(const std::string &appName)
 
 void PrivilegeDb::GetAppPkgName(const std::string &appName, std::string &pkgName)
 {
-    return try_catch<void>([&] {
+    return try_catch_db([&] {
         pkgName.clear();
 
         auto command = getStatement(StmtType::EGetAppPkgName);
@@ -256,7 +255,7 @@ void PrivilegeDb::GetAppPkgName(const std::string &appName, std::string &pkgName
 
 bool PrivilegeDb::GetAppPkgInfo(const std::string &appName, std::string &pkgName, bool &isHybrid, bool &isSharedRO)
 {
-    return try_catch<bool>([&] {
+    return try_catch_db([&] {
         auto command = getStatement(StmtType::EGetAppPkgInfo);
         command->BindString(1, appName);
 
@@ -272,7 +271,7 @@ bool PrivilegeDb::GetAppPkgInfo(const std::string &appName, std::string &pkgName
 
 void PrivilegeDb::GetAppVersion(const std::string &appName, std::string &tizenVer)
 {
-    return try_catch<void>([&] {
+    return try_catch_db([&] {
         tizenVer.clear();
 
         auto command = getStatement(StmtType::EGetAppVersion);
@@ -310,7 +309,7 @@ void PrivilegeDb::AddApplication(
         const std::string &authorName,
         bool isHybrid)
 {
-    try_catch<void>([&] {
+    try_catch_db([&] {
         auto command = getStatement(StmtType::EAddApplication);
         command->BindString(1, appName);
         command->BindString(2, pkgName);
@@ -335,7 +334,7 @@ void PrivilegeDb::RemoveApplication(
         bool &pkgNameIsNoMore,
         bool &authorNameIsNoMore)
 {
-    try_catch<void>([&] {
+    try_catch_db([&] {
         if (!AppNameExists(appName))
             return;
 
@@ -364,7 +363,7 @@ void PrivilegeDb::RemoveApplication(
 
 void PrivilegeDb::GetPathSharingCount(const std::string &path, int &count)
 {
-    try_catch<void>([&] {
+    try_catch_db([&] {
         auto command = getStatement(StmtType::EGetPathSharedCount);
         command->BindString(1, path);
 
@@ -376,7 +375,7 @@ void PrivilegeDb::GetPathSharingCount(const std::string &path, int &count)
 void PrivilegeDb::GetOwnerTargetSharingCount(const std::string &ownerAppName,
     const std::string &targetAppName, int &count)
 {
-    try_catch<void>([&] {
+    try_catch_db([&] {
         auto command = getStatement(StmtType::EGetOwnerTargetSharedCount);
         command->BindString(1, ownerAppName);
         command->BindString(2, targetAppName);
@@ -389,7 +388,7 @@ void PrivilegeDb::GetOwnerTargetSharingCount(const std::string &ownerAppName,
 void PrivilegeDb::GetTargetPathSharingCount(const std::string &targetAppName,
     const std::string &path, int &count)
 {
-    try_catch<void>([&] {
+    try_catch_db([&] {
         auto command = getStatement(StmtType::EGetTargetPathSharedCount);
         command->BindString(1, targetAppName);
         command->BindString(2, path);
@@ -403,7 +402,7 @@ void PrivilegeDb::ApplyPrivateSharing(const std::string &ownerAppName,
     const std::string &targetAppName, const std::string &path,
     const std::string &pathLabel)
 {
-    try_catch<void>([&] {
+    try_catch_db([&] {
         auto command = getStatement(StmtType::EAddPrivatePathSharing);
         command->BindString(1, ownerAppName);
         command->BindString(2, targetAppName);
@@ -417,7 +416,7 @@ void PrivilegeDb::ApplyPrivateSharing(const std::string &ownerAppName,
 void PrivilegeDb::DropPrivateSharing(const std::string &ownerAppName,
     const std::string &targetAppName, const std::string &path)
 {
-    try_catch<void>([&] {
+    try_catch_db([&] {
         auto command = getStatement(StmtType::ERemovePrivatePathSharing);
         command->BindString(1, ownerAppName);
         command->BindString(2, targetAppName);
@@ -428,7 +427,7 @@ void PrivilegeDb::DropPrivateSharing(const std::string &ownerAppName,
 }
 
 void PrivilegeDb::GetAllPrivateSharing(std::map<std::string, std::vector<std::string>> &appPathMap) {
-    try_catch<void>([&] {
+    try_catch_db([&] {
         auto command = getStatement(StmtType::EGetAllSharedPaths);
         while (command->Step()) {
             std::string appName = command->GetColumnString(0);
@@ -442,7 +441,7 @@ void PrivilegeDb::GetAllPrivateSharing(std::map<std::string, std::vector<std::st
 void PrivilegeDb::GetPrivateSharingForOwner(const std::string &ownerAppName,
                                             std::map<std::string, std::vector<std::string>> &ownerSharing)
 {
-    try_catch<void>([&] {
+    try_catch_db([&] {
         auto command = getStatement(StmtType::EGetSharingForOwner);
         command->BindString(1, ownerAppName);
         while (command->Step()) {
@@ -457,7 +456,7 @@ void PrivilegeDb::GetPrivateSharingForOwner(const std::string &ownerAppName,
 void PrivilegeDb::GetPrivateSharingForTarget(const std::string &targetAppName,
                                              std::map<std::string, std::vector<std::string>> &targetSharing)
 {
-    try_catch<void>([&] {
+    try_catch_db([&] {
         auto command = getStatement(StmtType::EGetSharingForTarget);
         command->BindString(1, targetAppName);
         while (command->Step()) {
@@ -470,7 +469,7 @@ void PrivilegeDb::GetPrivateSharingForTarget(const std::string &targetAppName,
 }
 
 void PrivilegeDb::SquashSharing(const std::string &targetAppName, const std::string &path) {
-    try_catch<void>([&] {
+    try_catch_db([&] {
         auto command = getStatement(StmtType::ESquashSharing);
         command->BindString(1, targetAppName);
         command->BindString(2, path);
@@ -480,7 +479,7 @@ void PrivilegeDb::SquashSharing(const std::string &targetAppName, const std::str
 }
 
 void PrivilegeDb::ClearPrivateSharing() {
-    try_catch<void>([&] {
+    try_catch_db([&] {
         {
             auto command = getStatement(StmtType::EClearSharing);
             command->Step();
@@ -494,7 +493,7 @@ void PrivilegeDb::ClearPrivateSharing() {
 
 void PrivilegeDb::GetUserApps(uid_t uid, std::vector<std::string> &apps)
 {
-   try_catch<void>([&] {
+    try_catch_db([&] {
         auto command = getStatement(StmtType::EGetUserApps);
         command->BindInteger(1, static_cast<unsigned int>(uid));
         apps.clear();
@@ -508,7 +507,7 @@ void PrivilegeDb::GetUserApps(uid_t uid, std::vector<std::string> &apps)
 
 void PrivilegeDb::GetUserAppsFromPkg(uid_t uid, const std::string &pkgName, std::vector<std::string> &apps)
 {
-   try_catch<void>([&] {
+    try_catch_db([&] {
         auto command = getStatement(StmtType::EGetUserAppsFromPkg);
         command->BindInteger(1, static_cast<unsigned int>(uid));
         command->BindString(2, pkgName);
@@ -523,7 +522,7 @@ void PrivilegeDb::GetUserAppsFromPkg(uid_t uid, const std::string &pkgName, std:
 
 void PrivilegeDb::GetUserPkgs(uid_t uid, std::vector<std::string> &pkgs)
 {
-   try_catch<void>([&] {
+    try_catch_db([&] {
         auto command = getStatement(StmtType::EGetUserPkgs);
         command->BindInteger(1, static_cast<unsigned int>(uid));
         pkgs.clear();
@@ -537,7 +536,7 @@ void PrivilegeDb::GetUserPkgs(uid_t uid, std::vector<std::string> &pkgs)
 
 void PrivilegeDb::GetAllPackages(std::vector<std::string> &packages)
 {
-    try_catch<void>([&] {
+    try_catch_db([&] {
         auto command = getStatement(StmtType::EGetAllPackages);
         packages.clear();
         while (command->Step()) {
@@ -551,7 +550,7 @@ void PrivilegeDb::GetAllPackages(std::vector<std::string> &packages)
 void PrivilegeDb::GetPkgApps(const std::string &pkgName,
         std::vector<std::string> &appNames)
 {
-    try_catch<void>([&] {
+    try_catch_db([&] {
         auto command = getStatement(StmtType::EGetAppsInPkg);
 
         command->BindString(1, pkgName);
@@ -567,7 +566,7 @@ void PrivilegeDb::GetPkgApps(const std::string &pkgName,
 
 void PrivilegeDb::GetPkgAuthorHash(const std::string &pkgName, std::string &authorHash)
 {
-    try_catch<void>([&] {
+    try_catch_db([&] {
         auto command = getStatement(StmtType::EGetPkgAuthor);
 
         command->BindString(1, pkgName);
@@ -583,7 +582,7 @@ void PrivilegeDb::GetPkgAuthorHash(const std::string &pkgName, std::string &auth
 
 bool PrivilegeDb::AuthorExists(const std::string &authorHash)
 {
-    return try_catch<bool>([&]() -> bool {
+    return try_catch_db([&]() -> bool {
         auto command = getStatement(StmtType::EAuthorExists);
         int cnt = 0;
 
@@ -599,7 +598,7 @@ bool PrivilegeDb::AuthorExists(const std::string &authorHash)
 
 void PrivilegeDb::GetGroupsRelatedPrivileges(std::vector<std::pair<std::string, std::string>> &privileges)
 {
-    try_catch<void>([&] {
+    try_catch_db([&] {
         auto command = getStatement(StmtType::EGetGroupsRelatedPrivileges);
 
         while (command->Step()) {
@@ -613,7 +612,7 @@ void PrivilegeDb::GetGroupsRelatedPrivileges(std::vector<std::pair<std::string,
 
 void PrivilegeDb::SetSharedROPackage(const std::string &pkgName, bool isSharedRO)
 {
-   try_catch<void>([&] {
+    try_catch_db([&] {
         auto command = getStatement(StmtType::ESetPackageSharedRO);
         command->BindInteger(1, isSharedRO);
         command->BindString(2, pkgName);
@@ -625,7 +624,7 @@ void PrivilegeDb::SetSharedROPackage(const std::string &pkgName, bool isSharedRO
 
 bool PrivilegeDb::IsPackageHybrid(const std::string& pkgName)
 {
-    return try_catch<bool>([&]() -> bool {
+    return try_catch_db([&]() -> bool {
         auto command = getStatement(StmtType::EIsPackageHybrid);
         command->BindString(1, pkgName);
         int isHybrid = 0;
@@ -642,7 +641,7 @@ bool PrivilegeDb::IsPackageHybrid(const std::string& pkgName)
 void PrivilegeDb::AddAppDefinedPrivilege(const std::string &appName, uid_t uid,
                                          const AppDefinedPrivilege &privilege)
 {
-    try_catch<void>([&] {
+    try_catch_db([&] {
         auto command = getStatement(StmtType::EAddAppDefinedPrivilege);
         command->BindString(1, appName);
         command->BindInteger(2, uid);
@@ -670,7 +669,7 @@ void PrivilegeDb::AddAppDefinedPrivileges(const std::string &appName, uid_t uid,
 void PrivilegeDb::AddClientPrivilege(const std::string &appName, uid_t uid, const std::string &privilege,
                                      const std::string &license)
 {
-    try_catch<void>([&] {
+    try_catch_db([&] {
         auto command = getStatement(StmtType::EAddClientPrivilege);
         command->BindString(1, appName);
         command->BindInteger(2, uid);
@@ -689,7 +688,7 @@ void PrivilegeDb::AddClientPrivilege(const std::string &appName, uid_t uid, cons
 
 void PrivilegeDb::RemoveAppDefinedPrivileges(const std::string &appName, uid_t uid)
 {
-    try_catch<void>([&] {
+    try_catch_db([&] {
         auto command = getStatement(StmtType::ERemoveAppDefinedPrivileges);
         command->BindString(1, appName);
         command->BindInteger(2, uid);
@@ -705,7 +704,7 @@ void PrivilegeDb::RemoveAppDefinedPrivileges(const std::string &appName, uid_t u
 
 void PrivilegeDb::RemoveClientPrivileges(const std::string &appName, uid_t uid)
 {
-    try_catch<void>([&] {
+    try_catch_db([&] {
         auto command = getStatement(StmtType::ERemoveClientPrivileges);
         command->BindString(1, appName);
         command->BindInteger(2, uid);
@@ -722,7 +721,7 @@ void PrivilegeDb::RemoveClientPrivileges(const std::string &appName, uid_t uid)
 void PrivilegeDb::GetAppDefinedPrivileges(const std::string &appName, uid_t uid,
                                           AppDefinedPrivilegesVector &privileges)
 {
-    try_catch<void>([&] {
+    try_catch_db([&] {
         privileges.clear();
 
         auto command = getStatement(StmtType::EGetAppDefinedPrivileges);
@@ -745,7 +744,7 @@ bool PrivilegeDb::GetAppPkgLicenseForAppDefinedPrivilege(
         std::string &pkgName,
         std::string &license)
 {
-    return try_catch<bool>([&] {
+    return try_catch_db([&] {
         appName.clear();
         pkgName.clear();
         license.clear();
@@ -773,7 +772,7 @@ bool PrivilegeDb::GetLicenseForClientPrivilegeAndApp(
         const std::string &privilege,
         std::string &license)
 {
-    return try_catch<bool>([&] {
+    return try_catch_db([&] {
         license.clear();
 
         auto command = getStatement(StmtType::EGetLicenseForClientPrivilegeAndApp);
@@ -800,7 +799,7 @@ bool PrivilegeDb::GetLicenseForClientPrivilegeAndPkg(
         const std::string &privilege,
         std::string &license)
 {
-    return try_catch<bool>([&] {
+    return try_catch_db([&] {
         license.clear();
 
         auto command = getStatement(StmtType::EGetLicenseForClientPrivilegeAndPkg);
@@ -823,7 +822,7 @@ bool PrivilegeDb::GetLicenseForClientPrivilegeAndPkg(
 
 bool PrivilegeDb::IsUserPkgInstalled(const std::string& pkgName, uid_t uid)
 {
-    return try_catch<bool>([&]() -> bool {
+    return try_catch_db([&]() -> bool {
         auto command = getStatement(StmtType::EIsUserPkgInstalled);
         command->BindString(1, pkgName);
         command->BindInteger(2, uid);

diff --git a/src/common/utils.cpp b/src/common/utils.cpp
index 071919f..0f263ac 100644 (file)
--- a/src/common/utils.cpp
+++ b/src/common/utils.cpp
@@ -26,23 +26,17 @@
  * @brief       Implementation of utility functions
  */
 
-#include <iostream>
-#include <cxxabi.h>
 #include <fcntl.h>
 #include <grp.h>
-#include <stdexcept>
 #include <system_error>
 #include <utils.h>
 
 #include <config.h>
 #include <config-file.h>
 
-#include <dpl/log/log.h>
 #include <dpl/errno_string.h>
 #include <protocols.h>
 
-#include <security-manager-types.h>
-
 namespace SecurityManager {
 
 int try_catch(const std::function<int()>& func)

diff --git a/test/test_misc.cpp b/test/test_misc.cpp
index ee99bbc..d4879ba 100644 (file)
--- a/test/test_misc.cpp
+++ b/test/test_misc.cpp
@@ -68,6 +68,29 @@ POSITIVE_TEST_CASE(T280_ScopedTimeStamper_try_catch)
     BOOST_REQUIRE_MESSAGE(try_catch(noThrowFunc) == SECURITY_MANAGER_SUCCESS, "Invalid return from try_catch()");
 }
 
+NEGATIVE_TEST_CASE(T281_try_catch_forced_unwind)
+{
+    pthread_t id;
+    int ret = pthread_create(&id, nullptr,
+            [](void*) -> void* {
+                try_catch([]{
+                    while(1) {
+                        sleep(5);
+                    }
+                    return SECURITY_MANAGER_SUCCESS;
+                });
+                return nullptr;
+            },
+            nullptr);
+
+    BOOST_REQUIRE_EQUAL(ret, 0);
+    BOOST_REQUIRE_EQUAL(pthread_cancel(id), 0);
+
+    void *res;
+    BOOST_REQUIRE_EQUAL(pthread_join(id, &res), 0);
+    BOOST_REQUIRE_EQUAL(res, PTHREAD_CANCELED);
+}
+
 NEGATIVE_TEST_CASE(T282_try_catch)
 {
     BOOST_REQUIRE_MESSAGE(try_catch(throwExFunc) == SECURITY_MANAGER_ERROR_UNKNOWN, "Invalid return value from try_catch()");