From 30dd7d4dff9b503f30130d5d4f50d82bf4fcf164 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Thu, 19 Jul 2018 18:00:03 -0700 Subject: [PATCH] Revert "base: add support for tagged fd closure to unique_fd." This reverts commit fcf2c01b5599a96b968afb1528c996d9486937b0. Commit broke full-eng, because libbase is being compiled against the NDK. Test: treehugger Change-Id: I8447b6a7fc33e6aa34cec0f037727322fa824446 --- base/Android.bp | 1 - base/include/android-base/unique_fd.h | 82 +++++---------------------- base/unique_fd_test.cpp | 54 ------------------ 3 files changed, 14 insertions(+), 123 deletions(-) delete mode 100644 base/unique_fd_test.cpp diff --git a/base/Android.bp b/base/Android.bp index 46a023342..3d80d9719 100644 --- a/base/Android.bp +++ b/base/Android.bp @@ -135,7 +135,6 @@ cc_test { "strings_test.cpp", "test_main.cpp", "test_utils_test.cpp", - "unique_fd_test.cpp", ], target: { android: { diff --git a/base/include/android-base/unique_fd.h b/base/include/android-base/unique_fd.h index 019d337c4..c7330817a 100644 --- a/base/include/android-base/unique_fd.h +++ b/base/include/android-base/unique_fd.h @@ -42,29 +42,10 @@ // // unique_fd is also known as ScopedFd/ScopedFD/scoped_fd; mentioned here to help // you find this class if you're searching for one of those names. - -#if defined(__BIONIC__) -#include -#endif - namespace android { namespace base { struct DefaultCloser { -#if defined(__BIONIC__) - static void Tag(int fd, void* old_addr, void* new_addr) { - uint64_t old_tag = android_fdsan_create_owner_tag(ANDROID_FDSAN_OWNER_TYPE_UNIQUE_FD, - reinterpret_cast(old_addr)); - uint64_t new_tag = android_fdsan_create_owner_tag(ANDROID_FDSAN_OWNER_TYPE_UNIQUE_FD, - reinterpret_cast(new_addr)); - android_fdsan_exchange_owner_tag(fd, old_tag, new_tag); - } - static void Close(int fd, void* addr) { - uint64_t tag = android_fdsan_create_owner_tag(ANDROID_FDSAN_OWNER_TYPE_UNIQUE_FD, - reinterpret_cast(addr)); - android_fdsan_close_with_tag(fd, tag); - } -#else static void Close(int fd) { // Even if close(2) fails with EINTR, the fd will have been closed. // Using TEMP_FAILURE_RETRY will either lead to EBADF or closing someone @@ -72,75 +53,40 @@ struct DefaultCloser { // http://lkml.indiana.edu/hypermail/linux/kernel/0509.1/0877.html ::close(fd); } -#endif }; template class unique_fd_impl final { public: - unique_fd_impl() {} + unique_fd_impl() : value_(-1) {} - explicit unique_fd_impl(int fd) { reset(fd); } + explicit unique_fd_impl(int value) : value_(value) {} ~unique_fd_impl() { reset(); } - unique_fd_impl(unique_fd_impl&& other) { reset(other.release()); } + unique_fd_impl(unique_fd_impl&& other) : value_(other.release()) {} unique_fd_impl& operator=(unique_fd_impl&& s) { - int fd = s.fd_; - s.fd_ = -1; - reset(fd, &s); + reset(s.release()); return *this; } - void reset(int new_value = -1) { reset(new_value, nullptr); } + void reset(int new_value = -1) { + if (value_ != -1) { + Closer::Close(value_); + } + value_ = new_value; + } - int get() const { return fd_; } + int get() const { return value_; } operator int() const { return get(); } int release() __attribute__((warn_unused_result)) { - tag(fd_, this, nullptr); - int ret = fd_; - fd_ = -1; + int ret = value_; + value_ = -1; return ret; } private: - void reset(int new_value, void* previous_tag) { - if (fd_ != -1) { - close(fd_, this); - } - - fd_ = new_value; - if (new_value != -1) { - tag(new_value, previous_tag, this); - } - } - - int fd_ = -1; - - // Template magic to use Closer::Tag if available, and do nothing if not. - // If Closer::Tag exists, this implementation is preferred, because int is a better match. - // If not, this implementation is SFINAEd away, and the no-op below is the only one that exists. - template - static auto tag(int fd, void* old_tag, void* new_tag) - -> decltype(T::Tag(fd, old_tag, new_tag), void()) { - T::Tag(fd, old_tag, new_tag); - } - - template - static void tag(long, void*, void*) { - // No-op. - } - - // Same as above, to select between Closer::Close(int) and Closer::Close(int, void*). - template - static auto close(int fd, void* tag_value) -> decltype(T::Close(fd, tag_value), void()) { - T::Close(fd, tag_value); - } - - template - static auto close(int fd, void*) -> decltype(T::Close(fd), void()) { - T::Close(fd); - } + int value_; unique_fd_impl(const unique_fd_impl&); void operator=(const unique_fd_impl&); diff --git a/base/unique_fd_test.cpp b/base/unique_fd_test.cpp deleted file mode 100644 index 3fdf12a30..000000000 --- a/base/unique_fd_test.cpp +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Copyright (C) 2018 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. - */ - -#include "android-base/unique_fd.h" - -#include - -#include -#include -#include - -using android::base::unique_fd; - -TEST(unique_fd, unowned_close) { -#if defined(__BIONIC__) - unique_fd fd(open("/dev/null", O_RDONLY)); - EXPECT_DEATH(close(fd.get()), "incorrect tag"); -#endif -} - -TEST(unique_fd, untag_on_release) { - unique_fd fd(open("/dev/null", O_RDONLY)); - close(fd.release()); -} - -TEST(unique_fd, move) { - unique_fd fd(open("/dev/null", O_RDONLY)); - unique_fd fd_moved = std::move(fd); - ASSERT_EQ(-1, fd.get()); - ASSERT_GT(fd_moved.get(), -1); -} - -TEST(unique_fd, unowned_close_after_move) { -#if defined(__BIONIC__) - unique_fd fd(open("/dev/null", O_RDONLY)); - unique_fd fd_moved = std::move(fd); - ASSERT_EQ(-1, fd.get()); - ASSERT_GT(fd_moved.get(), -1); - EXPECT_DEATH(close(fd_moved.get()), "incorrect tag"); -#endif -}