Simplify declaration and generation of unique_ptrs

The unique_ptr-based RAII pattern is used in several places in
security-manager. Declaration of unique pointer variables can be awkward
and hard to read.

This patch hides the nasty details of unique_ptr types declaration behind
a template function. It is loosely inspired by std::make_unique from C++14.

Change-Id: Ifbd8b5ab409fd8646d149d6294cb60bd2ac873a8

diff --git a/src/client/check-proper-drop.cpp b/src/client/check-proper-drop.cpp
index 4d136c5..6c6180c 100644 (file)
--- a/src/client/check-proper-drop.cpp
+++ b/src/client/check-proper-drop.cpp
@@ -24,7 +24,8 @@
 
 #include "check-proper-drop.h"
 #include "smack-labels.h"
-#include <dpl/log/log.h>
+#include "dpl/log/log.h"
+#include "utils.h"
 
 #include <sys/capability.h>
 
@@ -33,10 +34,6 @@
 
 namespace SecurityManager {
 
-using proctabPtr = std::unique_ptr<PROCTAB, void (*)(PROCTAB*)>;
-using capPtr = std::unique_ptr<_cap_struct, int (*)(void*)>;
-using capStrPtr = std::unique_ptr<char, int(*)(void*)>;
-
 CheckProperDrop::~CheckProperDrop()
 {
     for (const auto &thread : m_threads)
@@ -47,18 +44,17 @@ CheckProperDrop::~CheckProperDrop()
 void CheckProperDrop::getThreads()
 {
     pid_t pid[2] = {m_pid, 0};
-    proctabPtr PT(openproc(PROC_FILLSTATUS | PROC_PID, pid), closeproc);
-    if (!PT)
-        ThrowMsg(Exception::ProcError,
-            "Unable to open proc interface");
+    auto proctabPtr = makeUnique(openproc(PROC_FILLSTATUS | PROC_PID, pid), closeproc);
+    if (!proctabPtr)
+        ThrowMsg(Exception::ProcError, "Unable to open proc interface");
 
-    m_proc = readproc(PT.get(), nullptr);
+    m_proc = readproc(proctabPtr.get(), nullptr);
     if (!m_proc)
         ThrowMsg(Exception::ProcError,
             "Unable read process information for " << pid);
 
     proc_t *thread;
-    while ((thread = readtask(PT.get(), m_proc, nullptr)))
+    while ((thread = readtask(proctabPtr.get(), m_proc, nullptr)))
         if (thread->tid != m_pid)
             m_threads.push_back(thread);
 }
@@ -79,29 +75,29 @@ bool CheckProperDrop::checkThreads()
 
     std::string smackProc = SmackLabels::getSmackLabelFromPid(m_pid);
 
-    capPtr capProc(cap_get_pid(m_pid), cap_free);
-    if (!capProc)
+    auto capProcPtr = makeUnique(cap_get_pid(m_pid), cap_free);
+    if (!capProcPtr)
         ThrowMsg(Exception::CapError,
             "Unable to get capabilities for " << m_pid);
 
-    capStrPtr capStrProc(cap_to_text(capProc.get(), nullptr), cap_free);
-    if (!capStrProc)
+    auto capProcStrPtr = makeUnique(cap_to_text(capProcPtr.get(), nullptr), cap_free);
+    if (!capProcStrPtr)
         ThrowMsg(Exception::CapError,
             "Unable to get capabilities for " << m_pid);
 
     for (const auto &thread : m_threads) {
-        capPtr capThread(cap_get_pid(thread->tid), cap_free);
-        if (!capThread)
+        auto capThreadPtr = makeUnique(cap_get_pid(thread->tid), cap_free);
+        if (!capThreadPtr)
             ThrowMsg(Exception::CapError,
                 "Unable to get capabilities for " << thread->tid);
 
-        if (cap_compare(capProc.get(), capThread.get())) {
-            capStrPtr capStrThread(cap_to_text(capThread.get(), nullptr), cap_free);
-            if (!capStrThread)
+        if (cap_compare(capProcPtr.get(), capThreadPtr.get())) {
+            auto capStrThreadPtr = makeUnique(cap_to_text(capThreadPtr.get(), nullptr), cap_free);
+            if (!capStrThreadPtr)
                 ThrowMsg(Exception::CapError, "Unable to get capabilities for " << thread->tid);
 
             REPORT_THREAD_ERROR(thread->tid, "capabilities",
-                capStrProc.get(), capStrThread.get());
+                capProcStrPtr.get(), capStrThreadPtr.get());
         }
 
         std::string smackThread = SmackLabels::getSmackLabelFromPid(thread->tid);

diff --git a/src/client/client-label-monitor.cpp b/src/client/client-label-monitor.cpp
index 675ff1a..c06a06c 100644 (file)
--- a/src/client/client-label-monitor.cpp
+++ b/src/client/client-label-monitor.cpp
@@ -48,6 +48,7 @@
 #include <permissible-set.h>
 #include <protocols.h>
 #include <smack-labels.h>
+#include <utils.h>
 
 struct app_labels_monitor {
     int inotify;
@@ -99,7 +100,6 @@ static lib_retcode inotify_add_watch_full(int fd, const char* pathname, uint32_t
 SECURITY_MANAGER_API
 int security_manager_app_labels_monitor_init(app_labels_monitor **monitor)
 {
-    typedef std::unique_ptr<app_labels_monitor, void (*)(app_labels_monitor *)> monitorPtr;
     return try_catch([&] {
         LogDebug("security_manager_app_labels_monitor_init() called");
         if (monitor == nullptr) {
@@ -111,8 +111,8 @@ int security_manager_app_labels_monitor_init(app_labels_monitor **monitor)
 
         *monitor = nullptr;
 
-        monitorPtr m(new app_labels_monitor, security_manager_app_labels_monitor_finish);
-        if (!m) {
+        auto monitorPtr = makeUnique(new app_labels_monitor, security_manager_app_labels_monitor_finish);
+        if (!monitorPtr) {
             LogError("Bad memory allocation for app_labels_monitor");
             return SECURITY_MANAGER_ERROR_MEMORY;
         }
@@ -129,20 +129,20 @@ int security_manager_app_labels_monitor_init(app_labels_monitor **monitor)
             LogError("Inotify init failed: " << GetErrnoString(errno));
             return SECURITY_MANAGER_ERROR_WATCH_ADD_TO_FILE_FAILED;
         }
-        m.get()->inotify = ret;
-        ret_lib = inotify_add_watch_full(m.get()->inotify, globalFile.c_str(),
-                IN_CLOSE_WRITE, &(m.get()->global_labels_file_watch));
+        monitorPtr.get()->inotify = ret;
+        ret_lib = inotify_add_watch_full(monitorPtr->inotify, globalFile.c_str(),
+                IN_CLOSE_WRITE, &(monitorPtr->global_labels_file_watch));
         if (ret_lib != SECURITY_MANAGER_SUCCESS) {
             return ret_lib;
         }
-        ret_lib = inotify_add_watch_full(m.get()->inotify,
-            userFile.c_str(), IN_CLOSE_WRITE, &(m.get()->user_labels_file_watch));
+        ret_lib = inotify_add_watch_full(monitorPtr->inotify, userFile.c_str(),
+                IN_CLOSE_WRITE, &(monitorPtr->user_labels_file_watch));
         if (ret_lib != SECURITY_MANAGER_SUCCESS) {
             return ret_lib;
         }
-        m->user_label_file_path = userFile;
-        m->global_label_file_path = globalFile;
-        *monitor = m.release();
+        monitorPtr->user_label_file_path = userFile;
+        monitorPtr->global_label_file_path = globalFile;
+        *monitor = monitorPtr.release();
         return SECURITY_MANAGER_SUCCESS;
     });
 }
@@ -156,25 +156,23 @@ void security_manager_app_labels_monitor_finish(app_labels_monitor *monitor)
             LogDebug("input param \"monitor\" is nullptr");
             return 0;
         }
-        std::unique_ptr<app_labels_monitor> m(monitor);
-        if (!m)
-            LogError("Bad memory allocation for app_labels_monitor");
-        if (monitor->inotify != -1) {
-            if (monitor->global_labels_file_watch != -1) {
-                int ret = inotify_rm_watch(monitor->inotify, monitor->global_labels_file_watch);
+        auto monitorPtr = makeUnique(monitor);
+        if (monitorPtr->inotify != -1) {
+            if (monitorPtr->global_labels_file_watch != -1) {
+                int ret = inotify_rm_watch(monitorPtr->inotify, monitorPtr->global_labels_file_watch);
                 if (ret == -1) {
                     LogError("Inotify watch removal failed on file " <<
                             Config::APPS_NAME_FILE << ": " << GetErrnoString(errno));
                 }
             }
-            if (monitor->user_labels_file_watch != -1) {
-                int ret = inotify_rm_watch(monitor->inotify, monitor->user_labels_file_watch);
+            if (monitorPtr->user_labels_file_watch != -1) {
+                int ret = inotify_rm_watch(monitorPtr->inotify, monitorPtr->user_labels_file_watch);
                 if (ret == -1) {
                     LogError("Inotify watch removal failed on file "
                             << monitor->user_label_file_path << ": " << GetErrnoString(errno));
                 }
             }
-            close(monitor->inotify);
+            close(monitorPtr->inotify);
         }
         return 0;
     });
@@ -210,7 +208,6 @@ int security_manager_app_labels_monitor_get_fd(app_labels_monitor const *monitor
 SECURITY_MANAGER_API
 int security_manager_app_labels_monitor_process(app_labels_monitor *monitor)
 {
-    typedef std::unique_ptr<char, void (*)(void *)> bufPtr;
     return try_catch([&] {
         LogDebug("security_manager_app_labels_process() called");
         if (monitor == nullptr) {
@@ -237,9 +234,9 @@ int security_manager_app_labels_monitor_process(app_labels_monitor *monitor)
             return SECURITY_MANAGER_ERROR_UNKNOWN;
         }
 
-        bufPtr buffer(static_cast<char *>(malloc(avail)), free);
+        auto bufPtr = makeUnique<char[]>(avail);
         for (int pos = 0; pos < avail;) {
-            int ret = TEMP_FAILURE_RETRY(read(monitor->inotify, buffer.get() + pos, avail - pos));
+            int ret = TEMP_FAILURE_RETRY(read(monitor->inotify, bufPtr.get() + pos, avail - pos));
             if (ret == -1) {
                 LogError("Inotify read failed: " << GetErrnoString(errno));
                 return SECURITY_MANAGER_ERROR_UNKNOWN;
@@ -251,7 +248,7 @@ int security_manager_app_labels_monitor_process(app_labels_monitor *monitor)
             struct inotify_event event;
 
             /* Event must be copied to avoid memory alignment issues */
-            memcpy(&event, buffer.get() + pos, sizeof(struct inotify_event));
+            memcpy(&event, bufPtr.get() + pos, sizeof(struct inotify_event));
             pos += sizeof(struct inotify_event) + event.len;
             if ((event.mask & IN_CLOSE_WRITE) &&
                 ((event.wd == monitor->global_labels_file_watch) ||

diff --git a/src/client/client-security-manager.cpp b/src/client/client-security-manager.cpp
index 51f1b88..e9dc7d7 100644 (file)
--- a/src/client/client-security-manager.cpp
+++ b/src/client/client-security-manager.cpp
@@ -54,6 +54,7 @@
 #include <service_impl.h>
 #include <connection.h>
 #include <check-proper-drop.h>
+#include <utils.h>
 
 #include <security-manager.h>
 #include <client-offline.h>
@@ -397,7 +398,7 @@ static int getProcessGroups(std::vector<gid_t> &groups)
     }
     int cnt = ret;
 
-    std::unique_ptr<gid_t[]> groupsPtr(new gid_t[cnt]);
+    auto groupsPtr = makeUnique<gid_t[]>(cnt);
     if (!groupsPtr) {
         LogError("Memory allocation failed.");
         return SECURITY_MANAGER_ERROR_MEMORY;
@@ -1179,11 +1180,10 @@ int security_manager_groups_get(char ***groups, size_t *groups_count)
         const auto vgroups_size = vgroups.size();
         LogInfo("Number of groups: " << vgroups_size);
 
-        std::unique_ptr<char *, std::function<void(char **)>> array(
-            static_cast<char **>(calloc(vgroups_size, sizeof(char *))),
+        auto array = makeUnique(static_cast<char **>(calloc(vgroups_size, sizeof(char *))),
             std::bind(security_manager_groups_free, std::placeholders::_1, vgroups_size));
 
-        if (array == nullptr)
+        if (!array)
             return SECURITY_MANAGER_ERROR_MEMORY;
 
         for (size_t i = 0; i < vgroups_size; ++i) {

diff --git a/src/common/filesystem.cpp b/src/common/filesystem.cpp
index 6582104..0993a6a 100644 (file)
--- a/src/common/filesystem.cpp
+++ b/src/common/filesystem.cpp
@@ -38,6 +38,7 @@
 
 #include <filesystem.h>
 #include <filesystem-exception.h>
+#include <utils.h>
 
 namespace SecurityManager {
 namespace FS {
@@ -57,9 +58,9 @@ FileNameVector getDirContents(const std::string &path, const mode_t &mode)
     FileNameVector result;
     dirent tmp, *ptr;
     int err;
-    std::unique_ptr<DIR, decltype(closedir)*> dir(opendir(path.c_str()), closedir);
+    auto dir = makeUnique(opendir(path.c_str()), closedir);
 
-    if (!dir.get()) {
+    if (!dir) {
         err = errno;
         ThrowMsg(FS::Exception::FileError, "Error opening directory: " << GetErrnoString(err));
     }

diff --git a/src/common/include/utils.h b/src/common/include/utils.h
new file mode 100644 (file)
index 0000000..9367d86
--- /dev/null
+++ b/src/common/include/utils.h
@@ -0,0 +1,53 @@
+/*
+ *  Copyright (c) 2016 Samsung Electronics Co., Ltd All Rights Reserved
+ *
+ *  Contact: Rafal Krypa <r.krypa@samsung.com>
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License
+ */
+/*
+ * @file        utils.h
+ * @author      Rafal Krypa <r.krypa@samsung.com>
+ * @version     1.0
+ * @brief       Utility macros and templates
+ */
+
+#pragma once
+
+#include <functional>
+#include <memory>
+
+namespace SecurityManager {
+
+// Pointer
+template<typename T>
+std::unique_ptr<T> makeUnique(T *ptr)
+{
+    return std::unique_ptr<T>(ptr);
+}
+
+// Pointer & deleter func
+template<typename T, typename F>
+std::unique_ptr<T, F> makeUnique(T *ptr, F func)
+{
+    return std::unique_ptr<T, F>(ptr, func);
+}
+
+// Array - borrowed from C++14
+template<typename T>
+std::unique_ptr<T> makeUnique(size_t size)
+{
+    return std::unique_ptr<T>(new typename std::remove_extent<T>::type[size]);
+}
+
+} /* namespace SecurityManager */

diff --git a/src/common/service_impl.cpp b/src/common/service_impl.cpp
index e88ccd6..9ce8555 100644 (file)
--- a/src/common/service_impl.cpp
+++ b/src/common/service_impl.cpp
@@ -47,6 +47,7 @@
 #include "smack-labels.h"
 #include "security-manager.h"
 #include "tzplatform-config.h"
+#include "utils.h"
 
 #include "service_impl.h"
 
@@ -218,7 +219,7 @@ bool ServiceImpl::isSubDir(const std::string &parent, const std::string &subdir)
 
 std::string ServiceImpl::realPath(const std::string &path)
 {
-    std::unique_ptr<char, decltype(free)*> real_pathPtr(realpath(path.c_str(), nullptr), free);
+    auto real_pathPtr = makeUnique(realpath(path.c_str(), nullptr), free);
     if (!real_pathPtr) {
         LogError("Error in realpath(): " << GetErrnoString(errno) << " for: " << path);
         return std::string();

diff --git a/src/common/smack-labels.cpp b/src/common/smack-labels.cpp
index 07394b8..16710a2 100644 (file)
--- a/src/common/smack-labels.cpp
+++ b/src/common/smack-labels.cpp
@@ -43,6 +43,7 @@
 
 #include "security-manager.h"
 #include "smack-labels.h"
+#include "utils.h"
 
 
 namespace SecurityManager {
@@ -86,10 +87,7 @@ static void dirSetSmack(const std::string &path, const std::string &label,
     char *const path_argv[] = {const_cast<char *>(path.c_str()), NULL};
     FTSENT *ftsent;
 
-    std::unique_ptr<FTS, std::function<void(FTS*)> > fts(
-            fts_open(path_argv, FTS_PHYSICAL | FTS_NOCHDIR, NULL),
-            fts_close);
-
+    auto fts = makeUnique(fts_open(path_argv, FTS_PHYSICAL | FTS_NOCHDIR, NULL), fts_close);
     if (!fts) {
         LogError("fts_open failed.");
         ThrowMsg(SmackException::FileError, "fts_open failed.");
@@ -265,7 +263,7 @@ static std::string getSmackLabel(FuncType func, ArgsType... args)
     ssize_t labelLen = func(args..., &label);
     if (labelLen <= 0)
         ThrowMsg(SmackException::Base, "Error while getting Smack label");
-    std::unique_ptr<char, decltype(free)*> labelPtr(label, free);
+    auto labelPtr = makeUnique(label, free);
     return std::string(labelPtr.get(), labelLen);
 }