From c754792a07d340c30128b2fd064a58b1f15623da Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Thu, 6 Aug 2015 18:35:05 -0700 Subject: Use unique_ptr and unique_fd to manager FDs. Clean up leaky file descriptors in uncrypt/uncrypt.cpp. Add unique_fd for open() and unique_file for fopen() to close FDs on destruction. Bug: 21496020 Change-Id: I0174db0de9d5f59cd43b44757b8ef0f5912c91a2 --- uncrypt/Android.mk | 2 ++ uncrypt/uncrypt.cpp | 32 +++++++++++------------ unique_fd.h | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 16 deletions(-) create mode 100644 unique_fd.h diff --git a/uncrypt/Android.mk b/uncrypt/Android.mk index e73c8f1b6..f31db4243 100644 --- a/uncrypt/Android.mk +++ b/uncrypt/Android.mk @@ -20,6 +20,8 @@ LOCAL_CLANG := true LOCAL_SRC_FILES := uncrypt.cpp +LOCAL_C_INCLUDES := $(LOCAL_PATH)/.. + LOCAL_MODULE := uncrypt LOCAL_STATIC_LIBRARIES := libbase liblog libfs_mgr libcutils diff --git a/uncrypt/uncrypt.cpp b/uncrypt/uncrypt.cpp index 8785b29af..aef480035 100644 --- a/uncrypt/uncrypt.cpp +++ b/uncrypt/uncrypt.cpp @@ -51,6 +51,8 @@ #include #include +#include + #include #include #include @@ -60,6 +62,8 @@ #define LOG_TAG "uncrypt" #include +#include "unique_fd.h" + #define WINDOW_SIZE 5 static const std::string cache_block_map = "/cache/recovery/block.map"; @@ -183,6 +187,7 @@ static int produce_block_map(const char* path, const char* map_file, const char* return -1; } FILE* mapf = fdopen(mapfd, "w"); + unique_file mapf_holder(mapf); // Make sure we can write to the status_file. if (!android::base::WriteStringToFd("0\n", status_fd)) { @@ -191,8 +196,7 @@ static int produce_block_map(const char* path, const char* map_file, const char* } struct stat sb; - int ret = stat(path, &sb); - if (ret != 0) { + if (stat(path, &sb) != 0) { ALOGE("failed to stat %s\n", path); return -1; } @@ -221,15 +225,18 @@ static int produce_block_map(const char* path, const char* map_file, const char* size_t pos = 0; int fd = open(path, O_RDONLY); - if (fd < 0) { + unique_fd fd_holder(fd); + if (fd == -1) { ALOGE("failed to open fd for reading: %s\n", strerror(errno)); return -1; } int wfd = -1; + unique_fd wfd_holder(wfd); if (encrypted) { wfd = open(blk_dev, O_WRONLY | O_SYNC); - if (wfd < 0) { + wfd_holder = unique_fd(wfd); + if (wfd == -1) { ALOGE("failed to open fd for writing: %s\n", strerror(errno)); return -1; } @@ -247,8 +254,7 @@ static int produce_block_map(const char* path, const char* map_file, const char* if ((tail+1) % WINDOW_SIZE == head) { // write out head buffer int block = head_block; - ret = ioctl(fd, FIBMAP, &block); - if (ret != 0) { + if (ioctl(fd, FIBMAP, &block) != 0) { ALOGE("failed to find block %d\n", head_block); return -1; } @@ -288,8 +294,7 @@ static int produce_block_map(const char* path, const char* map_file, const char* while (head != tail) { // write out head buffer int block = head_block; - ret = ioctl(fd, FIBMAP, &block); - if (ret != 0) { + if (ioctl(fd, FIBMAP, &block) != 0) { ALOGE("failed to find block %d\n", head_block); return -1; } @@ -313,14 +318,11 @@ static int produce_block_map(const char* path, const char* map_file, const char* ALOGE("failed to fsync \"%s\": %s\n", map_file, strerror(errno)); return -1; } - fclose(mapf); - close(fd); if (encrypted) { if (fsync(wfd) == -1) { ALOGE("failed to fsync \"%s\": %s\n", blk_dev, strerror(errno)); return -1; } - close(wfd); } return 0; @@ -333,6 +335,8 @@ static void wipe_misc() { if (!v->mount_point) continue; if (strcmp(v->mount_point, "/misc") == 0) { int fd = open(v->blk_device, O_WRONLY | O_SYNC); + unique_fd fd_holder(fd); + uint8_t zeroes[1088]; // sizeof(bootloader_message) from recovery memset(zeroes, 0, sizeof(zeroes)); @@ -349,10 +353,8 @@ static void wipe_misc() { } if (fsync(fd) == -1) { ALOGE("failed to fsync \"%s\": %s\n", v->blk_device, strerror(errno)); - close(fd); return; } - close(fd); } } } @@ -437,6 +439,7 @@ int main(int argc, char** argv) { ALOGE("failed to open pipe \"%s\": %s\n", status_file.c_str(), strerror(errno)); return 1; } + unique_fd status_fd_holder(status_fd); if (argc == 3) { // when command-line args are given this binary is being used @@ -447,7 +450,6 @@ int main(int argc, char** argv) { std::string package; if (!find_uncrypt_package(package)) { android::base::WriteStringToFd("-1\n", status_fd); - close(status_fd); return 1; } input_path = package.c_str(); @@ -457,12 +459,10 @@ int main(int argc, char** argv) { int status = uncrypt(input_path, map_file, status_fd); if (status != 0) { android::base::WriteStringToFd("-1\n", status_fd); - close(status_fd); return 1; } android::base::WriteStringToFd("100\n", status_fd); - close(status_fd); } return 0; diff --git a/unique_fd.h b/unique_fd.h new file mode 100644 index 000000000..98a7c7b67 --- /dev/null +++ b/unique_fd.h @@ -0,0 +1,73 @@ +/* + * Copyright (C) 2015 The Android Open Source Project + * + * 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. + */ + +#ifndef UNIQUE_FD_H +#define UNIQUE_FD_H + +#include + +#include + +class unique_fd { + public: + unique_fd(int fd) : fd_(fd) { } + + unique_fd(unique_fd&& uf) { + fd_ = uf.fd_; + uf.fd_ = -1; + } + + ~unique_fd() { + if (fd_ != -1) { + close(fd_); + } + } + + int get() { + return fd_; + } + + // Movable. + unique_fd& operator=(unique_fd&& uf) { + fd_ = uf.fd_; + uf.fd_ = -1; + return *this; + } + + explicit operator bool() const { + return fd_ != -1; + } + + private: + int fd_; + + // Non-copyable. + unique_fd(const unique_fd&) = delete; + unique_fd& operator=(const unique_fd&) = delete; +}; + +// Custom deleter for unique_file to avoid fclose(NULL). +struct safe_fclose { + void operator()(FILE *fp) const { + if (fp) { + fclose(fp); + }; + } +}; + +using unique_file = std::unique_ptr; + +#endif // UNIQUE_FD_H -- cgit v1.2.3