commit 0fade37afd60c3f0e387172e03759e055f831d30
author Roman Stratiienko <r.stratiienko@gmail.com> Sat Feb 20 13:59:55 2021 +0200
committer Roman Stratiienko <r.stratiienko@gmail.com> Sat Jul 24 17:59:09 2021 +0300
tree 247fb23bfac5a18ce8dc8d90efffa3166845b36d
parent 5128715b42e6713b98244bd05f218b8800a39b74

drm_hwcomposer: Rework autofd

Motivation:

Current implementation of UniqueFd can be used in a different ways,
making analytical tracking of FD lifecycle much harder than it may be.
Keep this part clean is very important, since any wrong code may open
a hard-to-detect runtime bugs and fd leaks, which may accidentally slip
into the production.

Implementation:

1. Combine UniqueFd anf OutputFd into single class.
2. Reduce the API to be minimal and sufficient.
3. Document the API and use cases.
4. Move to utils/UniqueFd.h.
5. dup(fd) was replaced with fcntl(fd, F_DUPFD_CLOEXEC)) to
   address clang-tidy findings. Find more information at [1]

[1]: https://clang.llvm.org/extra/clang-tidy/checks/android-cloexec-dup.html

Signed-off-by: Roman Stratiienko <r.stratiienko@gmail.com>

DrmHwcTwo.cpp

diff --git a/DrmHwcTwo.cpp b/DrmHwcTwo.cpp
index 4a7a416..4db60cc 100644
--- a/DrmHwcTwo.cpp
+++ b/DrmHwcTwo.cpp
@@ -19,9 +19,11 @@
 
 #include "DrmHwcTwo.h"
 
+#include <fcntl.h>
 #include <hardware/hardware.h>
 #include <hardware/hwcomposer2.h>
 #include <sync/sync.h>
+#include <unistd.h>
 
 #include <cinttypes>
 #include <iostream>
@@ -597,22 +599,22 @@
     }
 
     layers[num_layers - 1] = l.first;
-    fences[num_layers - 1] = l.second.take_release_fence();
+    fences[num_layers - 1] = l.second.release_fence_.Release();
   }
   *num_elements = num_layers;
   return HWC2::Error::None;
 }
 
-void DrmHwcTwo::HwcDisplay::AddFenceToPresentFence(int fd) {
-  if (fd < 0)
+void DrmHwcTwo::HwcDisplay::AddFenceToPresentFence(UniqueFd fd) {
+  if (!fd) {
     return;
+  }
 
-  if (present_fence_.get() >= 0) {
-    int old_fence = present_fence_.get();
-    present_fence_.Set(sync_merge("dc_present", old_fence, fd));
-    close(fd);
+  if (present_fence_) {
+    present_fence_ = UniqueFd(
+        sync_merge("dc_present", present_fence_.Get(), fd.Get()));
   } else {
-    present_fence_.Set(fd);
+    present_fence_ = std::move(fd);
   }
 }
 
@@ -763,10 +765,10 @@
                                                    int32_t dataspace,
                                                    hwc_region_t /*damage*/) {
   supported(__func__);
-  UniqueFd uf(acquire_fence);
 
   client_layer_.set_buffer(target);
-  client_layer_.set_acquire_fence(uf.get());
+  client_layer_.acquire_fence_ = UniqueFd(
+      fcntl(acquire_fence, F_DUPFD_CLOEXEC));
   client_layer_.SetLayerDataspace(dataspace);
 
   /* TODO: Do not update source_crop every call.
@@ -1047,10 +1049,9 @@
 HWC2::Error DrmHwcTwo::HwcLayer::SetLayerBuffer(buffer_handle_t buffer,
                                                 int32_t acquire_fence) {
   supported(__func__);
-  UniqueFd uf(acquire_fence);
 
   set_buffer(buffer);
-  set_acquire_fence(uf.get());
+  acquire_fence_ = UniqueFd(fcntl(acquire_fence, F_DUPFD_CLOEXEC));
   return HWC2::Error::None;
 }
 
@@ -1141,11 +1142,9 @@
       break;
   }
 
-  OutputFd release_fence = release_fence_output();
-
   layer->sf_handle = buffer_;
-  layer->acquire_fence = acquire_fence_.Release();
-  layer->release_fence = std::move(release_fence);
+  // TODO(rsglobal): Avoid extra fd duplication
+  layer->acquire_fence = UniqueFd(fcntl(acquire_fence_.Get(), F_DUPFD_CLOEXEC));
   layer->display_frame = display_frame_;
   layer->alpha = lround(65535.0F * alpha_);
   layer->source_crop = source_crop_;