QQOpenGLProgramBinaryCache: split FdWrapper to fix a Coverity UNINIT warning

Coverity complained that mapSize was used in ~FdWrapper uninitialized
when the object was destroyed before map() had been called on it. This
is a False Positive, because in that case, ptr == MAP_FAILED, and we
wouldn't be reading mapSize.

But be nice and split the FdWrapper class into two, by creating an
object managing only the mmap() part of equation and returning it from
map(). The most-natural choice for such an object would be a
unique_ptr with a custom deleter, but that has built-in knowledge of
nullptr (ie. doesn't call the deleter on nullptr), which, at least
theoretically, is a valid return value of mmap() (the error case is
(void*)-1), so write a small struct ourselves.

Coverity-Id: 390668
Pick-to: 6.9 6.8 6.5
Change-Id: I43d635617bd26120cf402241bf59680fe63d071a
Reviewed-by: Thiago Macieira <[email protected]>
Reviewed-by: Laszlo Agocs <[email protected]>

1 files changed, 27 insertions, 12 deletions

diff --git a/src/gui/opengl/qopenglprogrambinarycache.cpp b/src/gui/opengl/qopenglprogrambinarycache.cpp
index 28b09b553aa..7e1120f2eb4 100644
--- a/src/gui/opengl/qopenglprogrambinarycache.cpp
+++ b/src/gui/opengl/qopenglprogrambinarycache.cpp
@@ -192,34 +192,48 @@ bool QOpenGLProgramBinaryCache::setProgramBinary(uint programId, uint blobFormat
 #ifdef Q_OS_UNIX
 class FdWrapper
 {
+    Q_DISABLE_COPY_MOVE(FdWrapper)
 public:
     FdWrapper(const QString &fn)
-        : ptr(MAP_FAILED)
     {
         fd = qt_safe_open(QFile::encodeName(fn).constData(), O_RDONLY);
     }
     ~FdWrapper()
     {
-        if (ptr != MAP_FAILED)
-            munmap(ptr, mapSize);
         if (fd != -1)
             qt_safe_close(fd);
     }
-    bool map()
+    auto map()
     {
+        struct R {
+            size_t mapSize;
+            void *ptr;
+
+            Q_DISABLE_COPY_MOVE(R)
+            explicit R(size_t sz, void *p)
+                : mapSize{sz}, ptr{p} {}
+            ~R()
+            {
+                if (ptr != MAP_FAILED)
+                    munmap(ptr, mapSize);
+            }
+
+            explicit operator bool() const noexcept { return ptr != MAP_FAILED; }
+        };
+
         off_t offs = lseek(fd, 0, SEEK_END);
         if (offs == (off_t) -1) {
             qErrnoWarning(errno, "lseek failed for program binary");
-            return false;
+            return R{0, MAP_FAILED};
         }
-        mapSize = static_cast<size_t>(offs);
-        ptr = mmap(nullptr, mapSize, PROT_READ, MAP_SHARED, fd, 0);
-        return ptr != MAP_FAILED;
+        auto mapSize = static_cast<size_t>(offs);
+        return R{
+            mapSize,
+            mmap(nullptr, mapSize, PROT_READ, MAP_SHARED, fd, 0),
+        };
     }
 
     int fd;
-    void *ptr;
-    size_t mapSize;
 };
 #endif
 
@@ -276,11 +290,12 @@ bool QOpenGLProgramBinaryCache::load(const QByteArray &cacheKey, uint programId)
 
     const uchar *p;
 #ifdef Q_OS_UNIX
-    if (!fdw.map()) {
+    const auto map = fdw.map();
+    if (!map) {
         undertaker.setActive();
         return false;
     }
-    p = static_cast<const uchar *>(fdw.ptr) + BASE_HEADER_SIZE;
+    p = static_cast<const uchar *>(map.ptr) + BASE_HEADER_SIZE;
 #else
     buf = f.readAll();
     p = reinterpret_cast<const uchar *>(buf.constData());