From: Rafal Krypa Date: Wed, 1 Jun 2016 08:24:29 +0000 (+0200) Subject: Simplify declaration and generation of unique_ptrs X-Git-Tag: accepted/tizen/common/20160816.130002~2 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=555bb3b8b9bd276cb1c09cc8f700b7b12339b0f1;p=platform%2Fcore%2Fsecurity%2Fsecurity-manager.git 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 --- 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 +#include "dpl/log/log.h" +#include "utils.h" #include @@ -33,10 +34,6 @@ namespace SecurityManager { -using proctabPtr = std::unique_ptr; -using capPtr = std::unique_ptr<_cap_struct, int (*)(void*)>; -using capStrPtr = std::unique_ptr; - 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 --- a/src/client/client-label-monitor.cpp +++ b/src/client/client-label-monitor.cpp @@ -48,6 +48,7 @@ #include #include #include +#include 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 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 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 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(malloc(avail)), free); + auto bufPtr = makeUnique(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 --- a/src/client/client-security-manager.cpp +++ b/src/client/client-security-manager.cpp @@ -54,6 +54,7 @@ #include #include #include +#include #include #include @@ -397,7 +398,7 @@ static int getProcessGroups(std::vector &groups) } int cnt = ret; - std::unique_ptr groupsPtr(new gid_t[cnt]); + auto groupsPtr = makeUnique(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> array( - static_cast(calloc(vgroups_size, sizeof(char *))), + auto array = makeUnique(static_cast(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 --- a/src/common/filesystem.cpp +++ b/src/common/filesystem.cpp @@ -38,6 +38,7 @@ #include #include +#include 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(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 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 + * + * 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 + * @version 1.0 + * @brief Utility macros and templates + */ + +#pragma once + +#include +#include + +namespace SecurityManager { + +// Pointer +template +std::unique_ptr makeUnique(T *ptr) +{ + return std::unique_ptr(ptr); +} + +// Pointer & deleter func +template +std::unique_ptr makeUnique(T *ptr, F func) +{ + return std::unique_ptr(ptr, func); +} + +// Array - borrowed from C++14 +template +std::unique_ptr makeUnique(size_t size) +{ + return std::unique_ptr(new typename std::remove_extent::type[size]); +} + +} /* namespace SecurityManager */ diff --git a/src/common/service_impl.cpp b/src/common/service_impl.cpp index e88ccd6..9ce8555 100644 --- 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 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 --- 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(path.c_str()), NULL}; FTSENT *ftsent; - std::unique_ptr > 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 labelPtr(label, free); + auto labelPtr = makeUnique(label, free); return std::string(labelPtr.get(), labelLen); }