[poppler] glib/poppler-document.cc goo/gfile.cc goo/gfile.h poppler/GlobalParamsWin.cc poppler/PDFDoc.cc poppler/PDFDoc.h

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Fri Mar 11 17:57:38 UTC 2022


 glib/poppler-document.cc   |    5 +++--
 goo/gfile.cc               |   14 +++++++-------
 goo/gfile.h                |   10 ++++++----
 poppler/GlobalParamsWin.cc |    8 +++-----
 poppler/PDFDoc.cc          |    7 +++----
 poppler/PDFDoc.h           |    2 +-
 6 files changed, 23 insertions(+), 23 deletions(-)

New commits:
commit 451acd97906681da4b385c8bb52a5dc971abb7fc
Author: Albert Astals Cid <aacid at kde.org>
Date:   Fri Mar 4 19:45:56 2022 +0100

    Make GooFile::open return an unique_ptr
    
    Sadly uncovers a memory leak on poppler_document_new_from_fd

diff --git a/glib/poppler-document.cc b/glib/poppler-document.cc
index 540e81a6..c00689a2 100644
--- a/glib/poppler-document.cc
+++ b/glib/poppler-document.cc
@@ -496,8 +496,9 @@ PopplerDocument *poppler_document_new_from_fd(int fd, const char *password, GErr
         CachedFile *cachedFile = new CachedFile(new FILECacheLoader(file), nullptr);
         stream = new CachedFileStream(cachedFile, 0, false, cachedFile->getLength(), Object(objNull));
     } else {
-        GooFile *file = GooFile::open(fd);
-        stream = new FileStream(file, 0, false, file->size(), Object(objNull));
+        std::unique_ptr<GooFile> file = GooFile::open(fd);
+        // FIXME file is getting leak here
+        stream = new FileStream(file.release(), 0, false, file->size(), Object(objNull));
     }
 
     const std::optional<GooString> password_g = poppler_password_to_latin1(password);
diff --git a/goo/gfile.cc b/goo/gfile.cc
index 327301df..218ad882 100644
--- a/goo/gfile.cc
+++ b/goo/gfile.cc
@@ -358,18 +358,18 @@ Goffset GooFile::size() const
     return size.QuadPart;
 }
 
-GooFile *GooFile::open(const std::string &fileName)
+std::unique_ptr<GooFile> GooFile::open(const std::string &fileName)
 {
     HANDLE handle = CreateFileA(fileName.c_str(), GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr);
 
-    return handle == INVALID_HANDLE_VALUE ? nullptr : new GooFile(handle);
+    return handle == INVALID_HANDLE_VALUE ? std::unique_ptr<GooFile>() : std::unique_ptr<GooFile>(new GooFile(handle));
 }
 
-GooFile *GooFile::open(const wchar_t *fileName)
+std::unique_ptr<GooFile> GooFile::open(const wchar_t *fileName)
 {
     HANDLE handle = CreateFileW(fileName, GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr);
 
-    return handle == INVALID_HANDLE_VALUE ? nullptr : new GooFile(handle);
+    return handle == INVALID_HANDLE_VALUE ? std::unique_ptr<GooFile>() : std::unique_ptr<GooFile>(new GooFile(handle));
 }
 
 bool GooFile::modificationTimeChangedSinceOpen() const
@@ -400,16 +400,16 @@ Goffset GooFile::size() const
 #    endif
 }
 
-GooFile *GooFile::open(const std::string &fileName)
+std::unique_ptr<GooFile> GooFile::open(const std::string &fileName)
 {
     int fd = openFileDescriptor(fileName.c_str(), O_RDONLY);
 
     return GooFile::open(fd);
 }
 
-GooFile *GooFile::open(int fdA)
+std::unique_ptr<GooFile> GooFile::open(int fdA)
 {
-    return fdA < 0 ? nullptr : new GooFile(fdA);
+    return fdA < 0 ? std::unique_ptr<GooFile>() : std::unique_ptr<GooFile>(new GooFile(fdA));
 }
 
 GooFile::GooFile(int fdA) : fd(fdA)
diff --git a/goo/gfile.h b/goo/gfile.h
index f913bb44..516c2fc2 100644
--- a/goo/gfile.h
+++ b/goo/gfile.h
@@ -16,7 +16,7 @@
 // under GPL version 2 or later
 //
 // Copyright (C) 2006 Kristian Høgsberg <krh at redhat.com>
-// Copyright (C) 2009, 2011, 2012, 2017, 2018, 2021 Albert Astals Cid <aacid at kde.org>
+// Copyright (C) 2009, 2011, 2012, 2017, 2018, 2021, 2022 Albert Astals Cid <aacid at kde.org>
 // Copyright (C) 2009 Kovid Goyal <kovid at kovidgoyal.net>
 // Copyright (C) 2013 Adam Reichold <adamreichold at myopera.com>
 // Copyright (C) 2013, 2017 Adrian Johnson <ajohnson at redneon.com>
@@ -75,6 +75,8 @@ extern "C" {
 #endif
 }
 
+#include <memory>
+
 class GooString;
 
 /* Integer type for all file offsets and file sizes */
@@ -122,13 +124,13 @@ public:
     int read(char *buf, int n, Goffset offset) const;
     Goffset size() const;
 
-    static GooFile *open(const std::string &fileName);
+    static std::unique_ptr<GooFile> open(const std::string &fileName);
 #ifndef _WIN32
-    static GooFile *open(int fdA);
+    static std::unique_ptr<GooFile> open(int fdA);
 #endif
 
 #ifdef _WIN32
-    static GooFile *open(const wchar_t *fileName);
+    static std::unique_ptr<GooFile> open(const wchar_t *fileName);
 
     ~GooFile() { CloseHandle(handle); }
 
diff --git a/poppler/GlobalParamsWin.cc b/poppler/GlobalParamsWin.cc
index 6e06ad18..f686cb62 100644
--- a/poppler/GlobalParamsWin.cc
+++ b/poppler/GlobalParamsWin.cc
@@ -369,7 +369,6 @@ void GlobalParams::setupBaseFonts(const char *dir)
 {
     const char *dataRoot = popplerDataDir ? popplerDataDir : POPPLER_DATADIR;
     GooString *fileName = nullptr;
-    GooFile *file;
 
     if (baseFontsInitialized)
         return;
@@ -416,11 +415,11 @@ void GlobalParams::setupBaseFonts(const char *dir)
     fileName->append("/cidfmap");
 
     // try to open file
-    file = GooFile::open(fileName->toStr());
+    const std::unique_ptr<GooFile> file = GooFile::open(fileName->toStr());
 
-    if (file != nullptr) {
+    if (file) {
         Parser *parser;
-        parser = new Parser(nullptr, new FileStream(file, 0, false, file->size(), Object(objNull)), true);
+        parser = new Parser(nullptr, new FileStream(file.get(), 0, false, file->size(), Object(objNull)), true);
         Object obj1 = parser->getObj();
         while (!obj1.isEOF()) {
             Object obj2 = parser->getObj();
@@ -441,7 +440,6 @@ void GlobalParams::setupBaseFonts(const char *dir)
                 obj1 = parser->getObj();
             }
         }
-        delete file;
         delete parser;
     } else {
         delete fileName;
diff --git a/poppler/PDFDoc.cc b/poppler/PDFDoc.cc
index a2d36ac1..b04d937f 100644
--- a/poppler/PDFDoc.cc
+++ b/poppler/PDFDoc.cc
@@ -139,7 +139,7 @@ PDFDoc::PDFDoc(std::unique_ptr<GooString> &&fileNameA, const std::optional<GooSt
     file = GooFile::open(fileName->toStr());
 #endif
 
-    if (file == nullptr) {
+    if (!file) {
         // fopen() has failed.
         // Keep a copy of the errno returned by fopen so that it can be
         // referred to later.
@@ -150,7 +150,7 @@ PDFDoc::PDFDoc(std::unique_ptr<GooString> &&fileNameA, const std::optional<GooSt
     }
 
     // create stream
-    str = new FileStream(file, 0, false, file->size(), Object(objNull));
+    str = new FileStream(file.get(), 0, false, file->size(), Object(objNull));
 
     ok = setup(ownerPassword, userPassword, xrefReconstructedCallback);
 }
@@ -186,7 +186,7 @@ PDFDoc::PDFDoc(wchar_t *fileNameA, int fileNameLen, const std::optional<GooStrin
     }
 
     // create stream
-    str = new FileStream(file, 0, false, file->size(), Object(objNull));
+    str = new FileStream(file.get(), 0, false, file->size(), Object(objNull));
 
     ok = setup(ownerPassword, userPassword, xrefReconstructedCallback);
 }
@@ -300,7 +300,6 @@ PDFDoc::~PDFDoc()
     delete hints;
     delete linearization;
     delete str;
-    delete file;
 #ifdef _WIN32
     gfree(fileNameU);
 #endif
diff --git a/poppler/PDFDoc.h b/poppler/PDFDoc.h
index 5c9635d6..040878cd 100644
--- a/poppler/PDFDoc.h
+++ b/poppler/PDFDoc.h
@@ -386,7 +386,7 @@ private:
 #ifdef _WIN32
     wchar_t *fileNameU = nullptr;
 #endif
-    GooFile *file = nullptr;
+    std::unique_ptr<GooFile> file;
     BaseStream *str = nullptr;
     void *guiData = nullptr;
     int headerPdfMajorVersion;


More information about the poppler mailing list