From 358c2ec1dca24d48a503b441d38545ace021ea8e Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Mon, 28 Nov 2016 11:48:43 -0800 Subject: Remove ota_close(int) and ota_fclose(FILE*). We should always use unique_fd or unique_file to hold the FD or FILE* pointer when opening via ota_(f)open functions. This CL avoids accidentally closing raw FDs or FILE* pointers that are managed by unique_fd/unique_file. Test: recovery_component_test passes. Change-Id: If58eb8b5c5da507563f85efd5d56276472a1c957 --- updater/install.cpp | 142 +++++++++++++++++++++++++--------------------------- 1 file changed, 69 insertions(+), 73 deletions(-) (limited to 'updater/install.cpp') diff --git a/updater/install.cpp b/updater/install.cpp index b9bc19e2c..8db5c1fe0 100644 --- a/updater/install.cpp +++ b/updater/install.cpp @@ -509,8 +509,8 @@ Value* PackageExtractFileFn(const char* name, State* state, int argc, Expr* argv return StringValue(""); } - int fd = TEMP_FAILURE_RETRY( - ota_open(dest_path.c_str(), O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR)); + unique_fd fd(TEMP_FAILURE_RETRY( + ota_open(dest_path.c_str(), O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR))); if (fd == -1) { printf("%s: can't open %s for write: %s\n", name, dest_path.c_str(), strerror(errno)); return StringValue(""); @@ -872,68 +872,67 @@ Value* GetPropFn(const char* name, State* state, int argc, Expr* argv[]) { // per line. # comment lines, blank lines, lines without '=' ignored), // and returns the value for 'key' (or "" if it isn't defined). Value* FileGetPropFn(const char* name, State* state, int argc, Expr* argv[]) { - if (argc != 2) { - return ErrorAbort(state, kArgsParsingFailure, "%s() expects 2 args, got %d", name, argc); - } + if (argc != 2) { + return ErrorAbort(state, kArgsParsingFailure, "%s() expects 2 args, got %d", name, argc); + } - std::vector args; - if (!ReadArgs(state, 2, argv, &args)) { - return ErrorAbort(state, kArgsParsingFailure, "%s() Failed to parse the argument(s)", name); - } - const std::string& filename = args[0]; - const std::string& key = args[1]; + std::vector args; + if (!ReadArgs(state, 2, argv, &args)) { + return ErrorAbort(state, kArgsParsingFailure, "%s() Failed to parse the argument(s)", name); + } + const std::string& filename = args[0]; + const std::string& key = args[1]; - struct stat st; - if (stat(filename.c_str(), &st) < 0) { - return ErrorAbort(state, kFileGetPropFailure, "%s: failed to stat \"%s\": %s", name, - filename.c_str(), strerror(errno)); - } + struct stat st; + if (stat(filename.c_str(), &st) < 0) { + return ErrorAbort(state, kFileGetPropFailure, "%s: failed to stat \"%s\": %s", name, + filename.c_str(), strerror(errno)); + } - constexpr off_t MAX_FILE_GETPROP_SIZE = 65536; - if (st.st_size > MAX_FILE_GETPROP_SIZE) { - return ErrorAbort(state, kFileGetPropFailure, "%s too large for %s (max %lld)", - filename.c_str(), name, static_cast(MAX_FILE_GETPROP_SIZE)); - } + constexpr off_t MAX_FILE_GETPROP_SIZE = 65536; + if (st.st_size > MAX_FILE_GETPROP_SIZE) { + return ErrorAbort(state, kFileGetPropFailure, "%s too large for %s (max %lld)", + filename.c_str(), name, static_cast(MAX_FILE_GETPROP_SIZE)); + } - std::string buffer(st.st_size, '\0'); - FILE* f = ota_fopen(filename.c_str(), "rb"); - if (f == nullptr) { - return ErrorAbort(state, kFileOpenFailure, "%s: failed to open %s: %s", name, - filename.c_str(), strerror(errno)); - } + std::string buffer(st.st_size, '\0'); + unique_file f(ota_fopen(filename.c_str(), "rb")); + if (f == nullptr) { + return ErrorAbort(state, kFileOpenFailure, "%s: failed to open %s: %s", name, filename.c_str(), + strerror(errno)); + } - if (ota_fread(&buffer[0], 1, st.st_size, f) != static_cast(st.st_size)) { - ErrorAbort(state, kFreadFailure, "%s: failed to read %zu bytes from %s", - name, static_cast(st.st_size), filename.c_str()); - ota_fclose(f); - return nullptr; - } + if (ota_fread(&buffer[0], 1, st.st_size, f.get()) != static_cast(st.st_size)) { + ErrorAbort(state, kFreadFailure, "%s: failed to read %zu bytes from %s", name, + static_cast(st.st_size), filename.c_str()); + return nullptr; + } - ota_fclose(f); + ota_fclose(f); - std::vector lines = android::base::Split(buffer, "\n"); - for (size_t i = 0; i < lines.size(); i++) { - std::string line = android::base::Trim(lines[i]); + std::vector lines = android::base::Split(buffer, "\n"); + for (size_t i = 0; i < lines.size(); i++) { + std::string line = android::base::Trim(lines[i]); - // comment or blank line: skip to next line - if (line.empty() || line[0] == '#') { - continue; - } - size_t equal_pos = line.find('='); - if (equal_pos == std::string::npos) { - continue; - } + // comment or blank line: skip to next line + if (line.empty() || line[0] == '#') { + continue; + } + size_t equal_pos = line.find('='); + if (equal_pos == std::string::npos) { + continue; + } - // trim whitespace between key and '=' - std::string str = android::base::Trim(line.substr(0, equal_pos)); + // trim whitespace between key and '=' + std::string str = android::base::Trim(line.substr(0, equal_pos)); - // not the key we're looking for - if (key != str) continue; + // not the key we're looking for + if (key != str) continue; - return StringValue(android::base::Trim(line.substr(equal_pos + 1))); - } + return StringValue(android::base::Trim(line.substr(equal_pos + 1))); + } - return StringValue(""); + return StringValue(""); } // apply_patch_space(bytes) @@ -1292,29 +1291,26 @@ Value* GetStageFn(const char* name, State* state, int argc, Expr* argv[]) { } Value* WipeBlockDeviceFn(const char* name, State* state, int argc, Expr* argv[]) { - if (argc != 2) { - return ErrorAbort(state, kArgsParsingFailure, "%s() expects 2 args, got %d", name, argc); - } - - std::vector args; - if (!ReadArgs(state, 2, argv, &args)) { - return ErrorAbort(state, kArgsParsingFailure, "%s() Failed to parse the argument(s)", name); - } - const std::string& filename = args[0]; - const std::string& len_str = args[1]; - - size_t len; - if (!android::base::ParseUint(len_str.c_str(), &len)) { - return nullptr; - } - int fd = ota_open(filename.c_str(), O_WRONLY, 0644); - // The wipe_block_device function in ext4_utils returns 0 on success and 1 - // for failure. - int status = wipe_block_device(fd, len); + if (argc != 2) { + return ErrorAbort(state, kArgsParsingFailure, "%s() expects 2 args, got %d", name, argc); + } - ota_close(fd); + std::vector args; + if (!ReadArgs(state, 2, argv, &args)) { + return ErrorAbort(state, kArgsParsingFailure, "%s() Failed to parse the argument(s)", name); + } + const std::string& filename = args[0]; + const std::string& len_str = args[1]; - return StringValue((status == 0) ? "t" : ""); + size_t len; + if (!android::base::ParseUint(len_str.c_str(), &len)) { + return nullptr; + } + unique_fd fd(ota_open(filename.c_str(), O_WRONLY, 0644)); + // The wipe_block_device function in ext4_utils returns 0 on success and 1 + // for failure. + int status = wipe_block_device(fd, len); + return StringValue((status == 0) ? "t" : ""); } Value* EnableRebootFn(const char* name, State* state, int argc, Expr* argv[]) { -- cgit v1.2.3