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
diff --git a/uncrypt/Android.mk b/uncrypt/Android.mk
index e73c8f1..f31db42 100644
--- a/uncrypt/Android.mk
+++ b/uncrypt/Android.mk
@@ -20,6 +20,8 @@
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 8785b29..aef4800 100644
--- a/uncrypt/uncrypt.cpp
+++ b/uncrypt/uncrypt.cpp
@@ -51,6 +51,8 @@
#include <sys/types.h>
#include <unistd.h>
+#include <memory>
+
#include <base/file.h>
#include <base/strings.h>
#include <cutils/android_reboot.h>
@@ -60,6 +62,8 @@
#define LOG_TAG "uncrypt"
#include <log/log.h>
+#include "unique_fd.h"
+
#define WINDOW_SIZE 5
static const std::string cache_block_map = "/cache/recovery/block.map";
@@ -183,6 +187,7 @@
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 @@
}
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
}
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 @@
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 @@
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 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 0000000..98a7c7b
--- /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 <stdio.h>
+
+#include <memory>
+
+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<FILE, safe_fclose>;
+
+#endif // UNIQUE_FD_H