libhwcomposer: Rework error paths in DRMFramebuffer

Returning early in a constructor does not signal to the instantiator an
issue has happened and this can lead to problems like leaked resources
and running into errors later when this object is used. To fix this
we should throw exceptions that can be caught by higher layers.

While we are here add error code reporting to the error messages. The
passed in handle can not be null at this point, remove this extra check.
Also failing to close a handle should not stop us from trying to close
the rest, print a warning and continue.

Signed-off-by: Andrew F. Davis <afd@ti.com>

1 files changed, 14 insertions, 15 deletions

diff --git a/libhwcomposer/drmfb.cpp b/libhwcomposer/drmfb.cpp
index 0c193d7..a368c10 100644
--- a/libhwcomposer/drmfb.cpp
+++ b/libhwcomposer/drmfb.cpp
@@ -14,6 +14,9 @@
  * limitations under the License.
  */
 
+#include <cerrno>
+#include <cstring>
+
 #include <log/log.h>
 #include <drm/drm_fourcc.h>
 #include <hardware/hwcomposer.h>
@@ -28,15 +31,12 @@
 DRMFramebuffer::DRMFramebuffer(int drm_fd, buffer_handle_t handle) :
     bo(), pitches(), offsets()
 {
-    if (!handle)
-        return;
-
     uint32_t gem_handle;
     IMG_native_handle_t* img_hnd = (IMG_native_handle_t*)handle;
     int ret = drmPrimeFDToHandle(drm_fd, img_hnd->fd[0], &gem_handle);
     if (ret) {
-        ALOGE("Failed to get DRM buffer object from handle");
-        return;
+        ALOGE("Failed to get DRM buffer object from handle: %d (%s)", ret, strerror(errno));
+        throw std::exception();
     }
 
     this->width = img_hnd->iWidth;
@@ -58,8 +58,8 @@ DRMFramebuffer::DRMFramebuffer(int drm_fd, buffer_handle_t handle) :
     case DRM_FORMAT_RGB565:
         break;
     default:
-        ALOGE("Bad format for overlay");
-        return;
+        ALOGE("Unsupported pixel format for display");
+        throw std::exception();
     }
 
     ret = drmModeAddFB2(drm_fd, this->width, this->height,
@@ -67,16 +67,17 @@ DRMFramebuffer::DRMFramebuffer(int drm_fd, buffer_handle_t handle) :
                         this->pitches, this->offsets,
                         &this->fb_id, 0);
     if (ret) {
-        ALOGE("Could not create DRM frame buffer %d", ret);
-        return;
+        ALOGE("Could not create DRM frame buffer: %d (%s)", ret, strerror(errno));
+        throw std::exception();
     }
 }
 
 DRMFramebuffer::~DRMFramebuffer()
 {
     if (this->fb_id) {
-        if (drmModeRmFB(this->drm_fd, this->fb_id))
-            ALOGE("Failed to remove DRM frame buffer");
+        int ret = drmModeRmFB(this->drm_fd, this->fb_id);
+        if (ret)
+            ALOGW("Failed to remove DRM frame buffer: %d (%s)", ret, strerror(errno));
     }
 
     for (size_t i = 0; i < 4; i++) {
@@ -86,10 +87,8 @@ DRMFramebuffer::~DRMFramebuffer()
                 close_args.pad = 0,
             };
             int ret = drmIoctl(this->drm_fd, DRM_IOCTL_GEM_CLOSE, &close_args);
-            if (ret) {
-                ALOGE("Failed to close GEM handle");
-                return;
-            }
+            if (ret)
+                ALOGW("Failed to close GEM handle: %d (%s)", ret, strerror(errno));
         }
     }
 }