From e43893e95c48774cff59ce1f442809abbdc440b8 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Mon, 9 Mar 2020 12:05:32 -0700 Subject: [PATCH] base: tag unique_fd::reset as reinitializing for clang-tidy. Appease clang-tidy by marking reset() as a method that reinitializes after moving out of a unique_fd. Unfortunately, there isn't an attribute that let us mark the type as being safe to use after move in general, which means that moving out of a unique_fd and then calling get() on it will still be frowned upon. clang-tidy has a hard-coded list of standard container types that are safe to use after move, but doesn't provide a way to mark custom types as satisfying this condition. Bug: http://b/150959261 Test: reverted the change to unique_fd.h and the test failed Change-Id: Ide73d7caa4cd2b192018f111059d696dca4de987 --- base/Android.bp | 17 ++++++++++++++ base/include/android-base/unique_fd.h | 2 +- base/tidy/unique_fd_test.cpp | 32 +++++++++++++++++++++++++++ base/tidy/unique_fd_test2.cpp | 19 ++++++++++++++++ 4 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 base/tidy/unique_fd_test.cpp create mode 100644 base/tidy/unique_fd_test2.cpp diff --git a/base/Android.bp b/base/Android.bp index 25c74f219..f2e412237 100644 --- a/base/Android.bp +++ b/base/Android.bp @@ -203,6 +203,23 @@ cc_test { test_suites: ["device-tests"], } +cc_test { + name: "libbase_tidy_test", + defaults: ["libbase_cflags_defaults"], + host_supported: true, + + tidy: true, + tidy_checks_as_errors: ["bugprone-use-after-move"], + + srcs: [ + "tidy/unique_fd_test.cpp", + "tidy/unique_fd_test2.cpp", + ], + + shared_libs: ["libbase"], + test_suites: ["device_tests"], +} + cc_benchmark { name: "libbase_benchmark", defaults: ["libbase_cflags_defaults"], diff --git a/base/include/android-base/unique_fd.h b/base/include/android-base/unique_fd.h index c4a0aad0f..9ceb5dbdc 100644 --- a/base/include/android-base/unique_fd.h +++ b/base/include/android-base/unique_fd.h @@ -102,7 +102,7 @@ class unique_fd_impl final { return *this; } - void reset(int new_value = -1) { reset(new_value, nullptr); } + [[clang::reinitializes]] void reset(int new_value = -1) { reset(new_value, nullptr); } int get() const { return fd_; } diff --git a/base/tidy/unique_fd_test.cpp b/base/tidy/unique_fd_test.cpp new file mode 100644 index 000000000..b3a99fcbb --- /dev/null +++ b/base/tidy/unique_fd_test.cpp @@ -0,0 +1,32 @@ +/* + * Copyright (C) 2020 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 + +extern void consume_unique_fd(android::base::unique_fd fd); + +TEST(unique_fd, bugprone_use_after_move) { + // Compile time test for clang-tidy's bugprone-use-after-move check. + android::base::unique_fd ufd(open("/dev/null", O_RDONLY | O_CLOEXEC)); + consume_unique_fd(std::move(ufd)); + ufd.reset(open("/dev/null", O_RDONLY | O_CLOEXEC)); + ufd.get(); + consume_unique_fd(std::move(ufd)); +} diff --git a/base/tidy/unique_fd_test2.cpp b/base/tidy/unique_fd_test2.cpp new file mode 100644 index 000000000..b0c71e239 --- /dev/null +++ b/base/tidy/unique_fd_test2.cpp @@ -0,0 +1,19 @@ +/* + * Copyright (C) 2020 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" + +void consume_unique_fd(android::base::unique_fd) {}