drm_hwcomposer: CI: Upgrade clang-* to v12

- Enabling readability-ientifier-naming tidy check does require to specify
MacroDefinitionIgnoredRegexp key, which is available only in clang-tidy-12.

- Clang-12 isn't available on ubuntu 20.10, therefore upgrade to 21.04.

- "DEBIAN_FRONTEND: noninteractive" is required to prevent ubuntu 21.04
from hanging, presumably due to waiting for the user input.

- A positive side effect of upgrading to clang-12 is new clang-tidy-12,
which exposed new issues in the code which is also fixed by this commit,
e.g:

    Failed cppcoreguidelines-narrowing-conversions check with error:
    error: narrowing conversion from 'uint32_t' (aka 'unsigned int') to 'float'

require explicit casting to pass the check, while some of such fails are caused
by incorrect variable type and fixed by changing the type to correct one.

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

diff --git a/.ci/.common.sh b/.ci/.common.sh
index 0183aba..21c2b28 100644 (file)
--- a/.ci/.common.sh
+++ b/.ci/.common.sh
@@ -1,7 +1,7 @@
 INCLUDE_DIRS="-I. -I../libdrm/include/drm -Iinclude -I/usr/include/libdrm -I./.ci/android_headers -I./tests/test_include"
 
-CLANG="clang++-11"
-CLANG_TIDY="clang-tidy-11"
+CLANG="clang++-12"
+CLANG_TIDY="clang-tidy-12"
 
 CXXARGS="-fPIC -Wall -Werror -DPLATFORM_SDK_VERSION=30 -D__ANDROID_API__=30 -Wsign-promo -Wimplicit-fallthrough"
 CXXARGS+=" -D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS -Wno-gnu-include-next "

diff --git a/.ci/.gitlab-ci-clang-tidy-coarse.sh b/.ci/.gitlab-ci-clang-tidy-coarse.sh
index de0c024..3f98150 100755 (executable)
--- a/.ci/.gitlab-ci-clang-tidy-coarse.sh
+++ b/.ci/.gitlab-ci-clang-tidy-coarse.sh
@@ -17,6 +17,7 @@ TIDY_COARSE_CHECKS+="google-*,"
 TIDY_COARSE_CHECKS+="-google-readability-braces-around-statements,"
 TIDY_COARSE_CHECKS+="-google-readability-casting,"
 TIDY_COARSE_CHECKS+="misc-*,"
+TIDY_COARSE_CHECKS+="-misc-non-private-member-variables-in-classes",
 TIDY_COARSE_CHECKS+="modernize-*,"
 TIDY_COARSE_CHECKS+="-modernize-avoid-c-arrays,"
 TIDY_COARSE_CHECKS+="-modernize-use-trailing-return-type,"
@@ -24,6 +25,7 @@ TIDY_COARSE_CHECKS+="performance-*,"
 TIDY_COARSE_CHECKS+="portability-*,"
 TIDY_COARSE_CHECKS+="readability-*,"
 TIDY_COARSE_CHECKS+="-readability-braces-around-statements,"
+TIDY_COARSE_CHECKS+="-readability-function-cognitive-complexity,"
 TIDY_COARSE_CHECKS+="-readability-convert-member-functions-to-static,"
 TIDY_COARSE_CHECKS+="-readability-implicit-bool-conversion,"
 TIDY_COARSE_CHECKS+="-readability-magic-numbers,"

diff --git a/.clang-tidy b/.clang-tidy
index 0bb7bf0..6cdbdc0 100644 (file)
--- a/.clang-tidy
+++ b/.clang-tidy
@@ -6,6 +6,7 @@ Checks: >
     *,
     -fuchsia*,
     -llvm*,
+    -concurrency-mt-unsafe,
     -cppcoreguidelines-pro-type-vararg, -hicpp-vararg,
     -hicpp-signed-bitwise,
 

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 04eb6c5..41d38ba 100644 (file)
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -1,8 +1,11 @@
-image: ubuntu:20.10
+image: ubuntu:21.04
+
+variables:
+  DEBIAN_FRONTEND: noninteractive
 
 before_script:
   - apt-get --quiet update --yes >/dev/null
-  - apt-get --quiet install --yes clang-11 clang-tidy-11 clang-format-11 git libdrm-dev blueprint-tools libgtest-dev >/dev/null
+  - apt-get --quiet install --yes clang-12 clang-tidy-12 clang-format-12 git libdrm-dev blueprint-tools libgtest-dev >/dev/null
 
 stages:
   - build

diff --git a/DrmHwcTwo.cpp b/DrmHwcTwo.cpp
index 02667cf..d9ba4f3 100644 (file)
--- a/DrmHwcTwo.cpp
+++ b/DrmHwcTwo.cpp
@@ -48,7 +48,7 @@ DrmHwcTwo::DrmHwcTwo() : hwc2_device() {
 
 HWC2::Error DrmHwcTwo::CreateDisplay(hwc2_display_t displ,
                                      HWC2::DisplayType type) {
-  DrmDevice *drm = resource_manager_.GetDrmDevice(displ);
+  DrmDevice *drm = resource_manager_.GetDrmDevice(static_cast<int>(displ));
   if (!drm) {
     ALOGE("Failed to get a valid drmresource");
     return HWC2::Error::NoResources;
@@ -413,22 +413,26 @@ HWC2::Error DrmHwcTwo::HwcDisplay::GetDisplayAttribute(hwc2_config_t config,
   auto attribute = static_cast<HWC2::Attribute>(attribute_in);
   switch (attribute) {
     case HWC2::Attribute::Width:
-      *value = mode->h_display();
+      *value = static_cast<int>(mode->h_display());
       break;
     case HWC2::Attribute::Height:
-      *value = mode->v_display();
+      *value = static_cast<int>(mode->v_display());
       break;
     case HWC2::Attribute::VsyncPeriod:
       // in nanoseconds
-      *value = 1000.0 * 1000.0 * 1000.0 / mode->v_refresh();
+      *value = static_cast<int>(1E9 / mode->v_refresh());
       break;
     case HWC2::Attribute::DpiX:
       // Dots per 1000 inches
-      *value = mm_width ? (mode->h_display() * kUmPerInch) / mm_width : -1;
+      *value = mm_width
+                   ? static_cast<int>(mode->h_display() * kUmPerInch / mm_width)
+                   : -1;
       break;
     case HWC2::Attribute::DpiY:
       // Dots per 1000 inches
-      *value = mm_height ? (mode->v_display() * kUmPerInch) / mm_height : -1;
+      *value = mm_height ? static_cast<int>(mode->v_display() * kUmPerInch /
+                                            mm_height)
+                         : -1;
       break;
 #if PLATFORM_SDK_VERSION > 29
     case HWC2::Attribute::ConfigGroup:
@@ -786,8 +790,8 @@ HWC2::Error DrmHwcTwo::HwcDisplay::SetClientTarget(buffer_handle_t target,
 
   hwc_frect_t source_crop = {.left = 0.0F,
                              .top = 0.0F,
-                             .right = bo.width + 0.0F,
-                             .bottom = bo.height + 0.0F};
+                             .right = static_cast<float>(bo.width),
+                             .bottom = static_cast<float>(bo.height)};
   client_layer_.SetLayerSourceCrop(source_crop);
 
   return HWC2::Error::None;
@@ -909,7 +913,7 @@ HWC2::Error DrmHwcTwo::HwcDisplay::GetDisplayVsyncPeriod(
   if (mode.id() == 0)
     return HWC2::Error::BadConfig;
 
-  *outVsyncPeriod = 1E9 / mode.v_refresh();
+  *outVsyncPeriod = static_cast<hwc2_vsync_period_t>(1E9 / mode.v_refresh());
   return HWC2::Error::None;
 }
 

diff --git a/README.md b/README.md
index 728bc76..c142266 100644 (file)
--- a/README.md
+++ b/README.md
@@ -16,7 +16,7 @@ A short list of contribution guidelines:
   you with formatting of your patches:
 
     ```
-    git diff | clang-format-diff-11 -p 1 -style=file
+    git diff | clang-format-diff-12 -p 1 -style=file
     ```
 
 * Hardware specific changes should be tested on relevant platforms before

diff --git a/bufferinfo/legacy/BufferInfoLibdrm.cpp b/bufferinfo/legacy/BufferInfoLibdrm.cpp
index 52f792f..da89eb5 100644 (file)
--- a/bufferinfo/legacy/BufferInfoLibdrm.cpp
+++ b/bufferinfo/legacy/BufferInfoLibdrm.cpp
@@ -37,9 +37,9 @@ enum chroma_order {
 
 struct DroidYuvFormat {
   /* Lookup keys */
-  int native;                     /* HAL_PIXEL_FORMAT_ */
+  uint32_t native;                /* HAL_PIXEL_FORMAT_ */
   enum chroma_order chroma_order; /* chroma order is {Cb, Cr} or {Cr, Cb} */
-  int chroma_step; /* Distance in bytes between subsequent chroma pixels. */
+  size_t chroma_step; /* Distance in bytes between subsequent chroma pixels. */
 
   /* Result */
   int fourcc; /* DRM_FORMAT_ */
@@ -64,8 +64,8 @@ static const struct DroidYuvFormat kDroidYuvFormats[] = {
 
 #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
 
-static int get_fourcc_yuv(int native, enum chroma_order chroma_order,
-                          int chroma_step) {
+static int get_fourcc_yuv(uint32_t native, enum chroma_order chroma_order,
+                          size_t chroma_step) {
   for (auto droid_yuv_format : kDroidYuvFormats)
     if (droid_yuv_format.native == native &&
         droid_yuv_format.chroma_order == chroma_order &&
@@ -75,7 +75,7 @@ static int get_fourcc_yuv(int native, enum chroma_order chroma_order,
   return -1;
 }
 
-static bool is_yuv(int native) {
+static bool is_yuv(uint32_t native) {
   for (auto droid_yuv_format : kDroidYuvFormats)
     if (droid_yuv_format.native == native)
       return true;

diff --git a/compositor/DrmDisplayComposition.cpp b/compositor/DrmDisplayComposition.cpp
index 49dff0e..47f669d 100644 (file)
--- a/compositor/DrmDisplayComposition.cpp
+++ b/compositor/DrmDisplayComposition.cpp
@@ -145,7 +145,7 @@ static const char *DrmCompositionTypeToString(DrmCompositionType type) {
   }
 }
 
-static const char *DPMSModeToString(int dpms_mode) {
+static const char *DPMSModeToString(uint32_t dpms_mode) {
   switch (dpms_mode) {
     case DRM_MODE_DPMS_ON:
       return "ON";

diff --git a/compositor/DrmDisplayCompositor.cpp b/compositor/DrmDisplayCompositor.cpp
index a1fe50f..ff9f6ad 100644 (file)
--- a/compositor/DrmDisplayCompositor.cpp
+++ b/compositor/DrmDisplayCompositor.cpp
@@ -263,7 +263,7 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
     DrmPlane *plane = comp_plane.plane();
     std::vector<size_t> &source_layers = comp_plane.source_layers();
 
-    int fb_id = -1;
+    uint32_t fb_id = UINT32_MAX;
     int fence_fd = -1;
     hwc_rect_t display_frame;
     hwc_frect_t source_crop;
@@ -346,7 +346,7 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
         rotation |= DRM_MODE_ROTATE_0;
 
       if (fence_fd >= 0) {
-        int prop_id = plane->in_fence_fd_property().id();
+        uint32_t prop_id = plane->in_fence_fd_property().id();
         if (prop_id == 0) {
           ALOGE("Failed to get IN_FENCE_FD property id");
           break;
@@ -399,7 +399,7 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
     }
 
     // Disable the plane if there's no framebuffer
-    if (fb_id < 0) {
+    if (fb_id == UINT32_MAX) {
       ret = drmModeAtomicAddProperty(pset, plane->id(),
                                      plane->crtc_property().id(), 0) < 0 ||
             drmModeAtomicAddProperty(pset, plane->id(),
@@ -754,7 +754,9 @@ void DrmDisplayCompositor::Dump(std::ostringstream *out) const {
 
   uint64_t cur_ts = ts.tv_sec * 1000 * 1000 * 1000 + ts.tv_nsec;
   uint64_t num_ms = (cur_ts - dump_last_timestamp_ns_) / (1000 * 1000);
-  float fps = num_ms ? (num_frames * 1000.0F) / (num_ms) : 0.0F;
+  float fps = num_ms ? static_cast<float>(num_frames) * 1000.0F /
+                           static_cast<float>(num_ms)
+                     : 0.0F;
 
   *out << "--DrmDisplayCompositor[" << display_
        << "]: num_frames=" << num_frames << " num_ms=" << num_ms

diff --git a/drm/DrmDevice.cpp b/drm/DrmDevice.cpp
index abc8edc..570b676 100644 (file)
--- a/drm/DrmDevice.cpp
+++ b/drm/DrmDevice.cpp
@@ -195,7 +195,7 @@ std::tuple<int, int> DrmDevice::Init(const char *path, int num_displays) {
     crtcs_.emplace_back(std::move(crtc));
   }
 
-  std::vector<int> possible_clones;
+  std::vector<uint32_t> possible_clones;
   for (int i = 0; !ret && i < res->count_encoders; ++i) {
     drmModeEncoderPtr e = drmModeGetEncoder(fd(), res->encoders[i]);
     if (!e) {

diff --git a/drm/DrmEventListener.cpp b/drm/DrmEventListener.cpp
index b303653..53e7032 100644 (file)
--- a/drm/DrmEventListener.cpp
+++ b/drm/DrmEventListener.cpp
@@ -84,7 +84,7 @@ void DrmEventListener::FlipHandler(int /* fd */, unsigned int /* sequence */,
 
 void DrmEventListener::UEventHandler() {
   char buffer[1024];
-  int ret = 0;
+  ssize_t ret = 0;
 
   struct timespec ts {};
 
@@ -93,7 +93,7 @@ void DrmEventListener::UEventHandler() {
   if (!ret)
     timestamp = ts.tv_sec * 1000 * 1000 * 1000 + ts.tv_nsec;
   else
-    ALOGE("Failed to get monotonic clock on hotplug %d", ret);
+    ALOGE("Failed to get monotonic clock on hotplug %zd", ret);
 
   while (true) {
     ret = read(uevent_fd_.Get(), &buffer, sizeof(buffer));
@@ -101,7 +101,7 @@ void DrmEventListener::UEventHandler() {
       return;
 
     if (ret < 0) {
-      ALOGE("Got error reading uevent %d", ret);
+      ALOGE("Got error reading uevent %zd", ret);
       return;
     }
 

diff --git a/drm/DrmMode.cpp b/drm/DrmMode.cpp
index dd25758..c714458 100644 (file)
--- a/drm/DrmMode.cpp
+++ b/drm/DrmMode.cpp
@@ -121,7 +121,7 @@ uint32_t DrmMode::v_scan() const {
 
 float DrmMode::v_refresh() const {
   // Always recalculate refresh to report correct float rate
-  return clock_ / (float)(v_total_ * h_total_) * 1000.0F;
+  return static_cast<float>(clock_) / (float)(v_total_ * h_total_) * 1000.0F;
 }
 
 uint32_t DrmMode::flags() const {

diff --git a/drm/VSyncWorker.cpp b/drm/VSyncWorker.cpp
index 25eeeab..1c0de21 100644 (file)
--- a/drm/VSyncWorker.cpp
+++ b/drm/VSyncWorker.cpp
@@ -104,7 +104,8 @@ int VSyncWorker::SyntheticWaitVBlank(int64_t *timestamp) {
     ALOGW("Vsync worker active with conn=%p refresh=%f\n", conn,
           conn ? conn->active_mode().v_refresh() : 0.0F);
 
-  int64_t phased_timestamp = GetPhasedVSync(kOneSecondNs / refresh,
+  int64_t phased_timestamp = GetPhasedVSync(kOneSecondNs /
+                                                static_cast<int>(refresh),
                                             vsync.tv_sec * kOneSecondNs +
                                                 vsync.tv_nsec);
   vsync.tv_sec = phased_timestamp / kOneSecondNs;

diff --git a/include/drmhwcgralloc.h b/include/drmhwcgralloc.h
index db54802..57685d1 100644 (file)
--- a/include/drmhwcgralloc.h
+++ b/include/drmhwcgralloc.h
@@ -28,7 +28,7 @@ typedef struct hwc_drm_bo {
   uint32_t usage;
   uint32_t pitches[HWC_DRM_BO_MAX_PLANES];
   uint32_t offsets[HWC_DRM_BO_MAX_PLANES];
-  uint32_t prime_fds[HWC_DRM_BO_MAX_PLANES];
+  int prime_fds[HWC_DRM_BO_MAX_PLANES];
   uint64_t modifiers[HWC_DRM_BO_MAX_PLANES];
   int acquire_fence_fd;
 } hwc_drm_bo_t;

diff --git a/utils/properties.h b/utils/properties.h
index 607cbc5..38a2762 100644 (file)
--- a/utils/properties.h
+++ b/utils/properties.h
@@ -20,7 +20,7 @@ auto inline property_get(const char *name, char *value,
   if (prop == nullptr) {
     snprintf(value, PROPERTY_VALUE_MAX, "%s", default_value);
   }
-  return strlen(value);
+  return static_cast<int>(strlen(value));
 }
 
 #endif