[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