[poppler] Cached files, reading stdin, http streams and PDFDocBuilder
Albert Astals Cid
aacid at kde.org
Sat Mar 27 08:20:02 PDT 2010
A Dijous, 25 de febrer de 2010, Hib Eris va escriure:
> Hi all,
>
> After some sleep, I noticed that my patches regarding the
> PDFDocBuilders could be simplified. So here are all patches updated
> with this new insight.
>
> Hib Eris
Sorry it took too much, some comments:
* We try not to use the C++ standard library (yeah idiotic decision, but we
try to go on with it), do you really need <map> ?
* Please move "enum CachedFileChunkState" inside CachedFile class, and drop
the CachedFile from the name
* The same for CachedFileChunk
* Make CachedFile::getFileName return a GooString, it's not a hot path and
will help people forgetting to delete it and causing leaks
* If you really need to do manual reference counting (man this is C++ doing
manual reference counting is bad) do it like it's done in GfxFont, that is,
the destructor is private and inc/decRef return void and recRef automatically
deletes when counter reaches 0
* Also convert the "if (foo) delete foo;" statements to "delete foo;"
* In CurlCacheLoader::load you probably leak the range GooString
* The way builders are created/loaded/chained feels a bit weirdish, i think i
would prefer something like this (not well though names not the ones i would
probably use)
class PDFDocLoader
{
PDFDoc *load(...) = 0;
GooStringList supportedProtocols() const = 0;
};
class PDFDocLoaderFactory
{
bool init(); // loads default builders
bool add(Builder *); // adds a non default builder (so that for example
the Qt4 frontend can "inject" a builder based in QNetworkAccessManager
PDFDoc *load(...); // uses supportedProtocols to see which PDFDocLoader
has to use
};
What do you think?
Albert
More information about the poppler
mailing list