From 8ade6308013540eb6aee3ffa987e0272aa590396 Mon Sep 17 00:00:00 2001 From: danij Date: Fri, 12 Oct 2012 05:52:18 +0100 Subject: [PATCH] Refactor|FileSys: Use references rather than pointers where appropriate Plus minor cleanup. --- doomsday/engine/api/dfile.h | 4 +- doomsday/engine/portable/include/fs_main.h | 117 +++++++------ doomsday/engine/portable/src/abstractfile.cpp | 2 +- doomsday/engine/portable/src/dd_games.cpp | 2 +- doomsday/engine/portable/src/dfile.cpp | 14 +- doomsday/engine/portable/src/fs_main.cpp | 156 +++++++++--------- doomsday/engine/portable/src/image.cpp | 6 +- 7 files changed, 151 insertions(+), 150 deletions(-) diff --git a/doomsday/engine/api/dfile.h b/doomsday/engine/api/dfile.h index 2c7d8ba623..175774ac2b 100644 --- a/doomsday/engine/api/dfile.h +++ b/doomsday/engine/api/dfile.h @@ -72,8 +72,8 @@ class DFile DFile& setList(struct filelist_s* list); /// @todo Should not be visible outside the engine. - AbstractFile* file(); - AbstractFile* file() const; + AbstractFile& file(); + AbstractFile& file() const; /// @return @c true iff this handle's internal state is valid. bool isValid() const; diff --git a/doomsday/engine/portable/include/fs_main.h b/doomsday/engine/portable/include/fs_main.h index 553d1a208a..310163f0aa 100644 --- a/doomsday/engine/portable/include/fs_main.h +++ b/doomsday/engine/portable/include/fs_main.h @@ -88,7 +88,10 @@ class FS */ void mapPath(char const* source, char const* destination); - /// @note Should be called after WADs have been processed. + /** + * (Re-)Initialize the path => lump mappings. + * @note Should be called after WADs have been processed. + */ void initLumpPathMap(); /** @@ -112,9 +115,6 @@ class FS */ bool checkFileId(char const* path); - /// @return Number of files in the currently active primary LumpIndex. - int lumpCount(); - /** * @return @c true if a file exists at @a path which can be opened for reading. */ @@ -152,36 +152,6 @@ class FS int removeFiles(char const* const* paths, int num, bool permitRequired = false); - /** - * Opens the given file (will be translated) for reading. - * - * @post If @a allowDuplicate = @c false a new file ID for this will have been - * added to the list of known file identifiers if this file hasn't yet been - * opened. It is the responsibility of the caller to release this identifier when done. - * - * @param path Possibly relative or mapped path to the resource being opened. - * @param mode 't' = text mode (with real files, lumps are always binary) - * 'b' = binary - * 'f' = must be a real file in the local file system - * @param baseOffset Offset from the start of the file in bytes to begin. - * @param allowDuplicate @c false = open only if not already opened. - * - * @return Opened file reference/handle else @c NULL. - */ - DFile* openFile(char const* path, char const* mode, size_t baseOffset = 0, - bool allowDuplicate = true); - - /** - * Try to locate the specified lump for reading. - * - * @param absoluteLumpNum Logical lumpnum associated to the file being looked up. - * - * @return Handle to the opened file if found. - */ - DFile* openLump(lumpnum_t absoluteLumpNum); - - bool isValidLumpNum(lumpnum_t absoluteLumpNum); - /** * Find a lump in the Zip LumpIndex. * @@ -191,6 +161,11 @@ class FS */ AbstractFile* findLumpFile(char const* path, int* lumpIdx = 0); + /// @return Number of lumps in the currently active LumpIndex. + int lumpCount(); + + bool isValidLumpNum(lumpnum_t absoluteLumpNum); + lumpnum_t lumpNumForName(char const* name, bool silent = true); char const* lumpName(lumpnum_t absoluteLumpNum); @@ -244,14 +219,42 @@ class FS return 0; } + /** + * Opens the given file (will be translated) for reading. + * + * @post If @a allowDuplicate = @c false a new file ID for this will have been + * added to the list of known file identifiers if this file hasn't yet been + * opened. It is the responsibility of the caller to release this identifier when done. + * + * @param path Possibly relative or mapped path to the resource being opened. + * @param mode 't' = text mode (with real files, lumps are always binary) + * 'b' = binary + * 'f' = must be a real file in the local file system + * @param baseOffset Offset from the start of the file in bytes to begin. + * @param allowDuplicate @c false = open only if not already opened. + * + * @return Opened file reference/handle else @c NULL. + */ + DFile* openFile(char const* path, char const* mode, size_t baseOffset = 0, + bool allowDuplicate = true); + + /** + * Try to locate the specified lump for reading. + * + * @param absoluteLumpNum Logical lumpnum associated to the file being looked up. + * + * @return Handle to the opened file if found. + */ + DFile* openLump(lumpnum_t absoluteLumpNum); + /// Clear all references to this file. - void releaseFile(AbstractFile* file); + void releaseFile(AbstractFile& file); /// Close this file handle. - void closeFile(DFile* hndl); + void closeFile(DFile& hndl); /// Completely destroy this file; close if open, clear references and any acquired identifiers. - void deleteFile(DFile* hndl); + void deleteFile(DFile& hndl); /** * Parm is passed on to the callback, which is called for each file @@ -262,20 +265,6 @@ class FS */ int allResourcePaths(char const* searchPath, int flags, int (*callback) (char const* path, PathDirectoryNodeType type, void* parameters), void* parameters = 0); - /** - * Calculate a CRC for the loaded file list. - */ - uint loadedFilesCRC(); - - /** - * Try to open the specified WAD archive into the auxiliary lump cache. - * - * @return Base index for lumps in this archive. - */ - lumpnum_t openAuxiliary(char const* fileName, size_t baseOffset = 0); - - void closeAuxiliary(); - /** * Finds all files. * @@ -309,16 +298,36 @@ class FS */ void printIndex(); + /** + * Calculate a CRC for the loaded file list. + */ + uint loadedFilesCRC(); + + /** + * Try to open the specified WAD archive into the auxiliary lump index. + * + * @return Base index for lumps in this archive. + */ + lumpnum_t openAuxiliary(char const* fileName, size_t baseOffset = 0); + + /** + * Close the auxiliary lump index if open. + */ + void closeAuxiliary(); + private: struct Instance; Instance* d; - void unlinkFile(AbstractFile* file); + /** + * Removes a file from any lump indexes. + * + * @param file File to remove from the index. + */ + void deindex(AbstractFile& file); bool unloadFile(char const* path, bool permitRequired = false, bool quiet = false); - int unloadListFiles(FileList& list, bool nonStartup); - FILE* findRealFile(char const* path, char const* mymode, ddstring_t** foundPath); /** diff --git a/doomsday/engine/portable/src/abstractfile.cpp b/doomsday/engine/portable/src/abstractfile.cpp index 1cda9346d3..e6917befd0 100644 --- a/doomsday/engine/portable/src/abstractfile.cpp +++ b/doomsday/engine/portable/src/abstractfile.cpp @@ -48,7 +48,7 @@ AbstractFile::AbstractFile(filetype_t _type, char const* _path, DFile& file, Lum AbstractFile::~AbstractFile() { - App_FileSystem()->releaseFile(this); + App_FileSystem()->releaseFile(*this); Str_Free(&path_); if(file) delete file; } diff --git a/doomsday/engine/portable/src/dd_games.cpp b/doomsday/engine/portable/src/dd_games.cpp index 90183ff3a6..a240f7f35c 100644 --- a/doomsday/engine/portable/src/dd_games.cpp +++ b/doomsday/engine/portable/src/dd_games.cpp @@ -200,7 +200,7 @@ static bool recognizeZIP(char const* filePath, void* parameters) { result = de::ZipFile::recognise(*dfile); /// @todo Check files. We should implement an auxiliary zip lump index... - App_FileSystem()->closeFile(dfile); + App_FileSystem()->closeFile(*dfile); } return result; } diff --git a/doomsday/engine/portable/src/dfile.cpp b/doomsday/engine/portable/src/dfile.cpp index f0e7bb3dd4..806b77d6b2 100644 --- a/doomsday/engine/portable/src/dfile.cpp +++ b/doomsday/engine/portable/src/dfile.cpp @@ -172,7 +172,7 @@ DFile* DFileBuilder::dup(de::DFile const& hndl) de::DFile* clone = new de::DFile(); clone->d->flags.open = true; clone->d->flags.reference = true; - clone->d->file = hndl.file(); + clone->d->file = &hndl.file(); return clone; } @@ -254,16 +254,16 @@ DFile& DFile::setList(struct filelist_s* list) return *this; } -AbstractFile* DFile::file() +AbstractFile& DFile::file() { errorIfNotValid(*this, "DFile::file"); - return d->file; + return *d->file; } -AbstractFile* DFile::file() const +AbstractFile& DFile::file() const { errorIfNotValid(*this, "DFile::file const"); - return d->file; + return *d->file; } size_t DFile::baseOffset() const @@ -480,13 +480,13 @@ void DFile_Rewind(struct dfile_s* hndl) struct abstractfile_s* DFile_File(struct dfile_s* hndl) { SELF(hndl); - return reinterpret_cast(self->file()); + return reinterpret_cast(&self->file()); } struct abstractfile_s* DFile_File_const(struct dfile_s const* hndl) { SELF_CONST(hndl); - return reinterpret_cast(self->file()); + return reinterpret_cast(&self->file()); } struct filelist_s* DFile_List(struct dfile_s* hndl) diff --git a/doomsday/engine/portable/src/fs_main.cpp b/doomsday/engine/portable/src/fs_main.cpp index d55d507a3d..0ee0f395de 100644 --- a/doomsday/engine/portable/src/fs_main.cpp +++ b/doomsday/engine/portable/src/fs_main.cpp @@ -158,10 +158,11 @@ struct FS::Instance // Unload in reverse load order. for(int i = loadedFiles.size() - 1; i >= 0; i--) { - DFile* hndl = loadedFiles[i]; - if(!index || index->catalogues(*hndl->file())) + DFile& hndl = *(loadedFiles[i]); + if(!index || index->catalogues(hndl.file())) { - self->unlinkFile(hndl->file()); + releaseFileId(Str_Text(hndl.file().path())); + self->deindex(hndl.file()); loadedFiles.removeAt(i); self->deleteFile(hndl); } @@ -171,7 +172,7 @@ struct FS::Instance void clearOpenFiles() { while(!openFiles.empty()) - { self->deleteFile(openFiles.back()); } + { self->deleteFile(*openFiles.back()); } } int pruneLumpsFromIndexesByFile(AbstractFile& file) @@ -286,8 +287,8 @@ static de::FileList::iterator findListFileByPath(de::FileList& list, char const* de::FileList::iterator i; for(i = list.begin(); i != list.end(); ++i) { - de::AbstractFile* file = (*i)->file(); - if(!Str_CompareIgnoreCase(file->path(), Str_Text(path))) + de::AbstractFile& file = (*i)->file(); + if(!Str_CompareIgnoreCase(file.path(), Str_Text(path))) { break; // This is the node we are looking for. } @@ -295,11 +296,9 @@ static de::FileList::iterator findListFileByPath(de::FileList& list, char const* return i; } -void FS::unlinkFile(de::AbstractFile* file) +void FS::deindex(de::AbstractFile& file) { - if(!file) return; - d->releaseFileId(Str_Text(file->path())); - d->pruneLumpsFromIndexesByFile(*file); + d->pruneLumpsFromIndexesByFile(file); } bool FS::unloadFile(char const* path, bool permitRequired, bool quiet) @@ -323,8 +322,9 @@ bool FS::unloadFile(char const* path, bool permitRequired, bool quiet) Con_Message("Unloading \"%s\"...\n", F_PrettyPath(path)); } - DFile* hndl = *found; - unlinkFile(hndl->file()); + DFile& hndl = *(*found); + d->releaseFileId(Str_Text(hndl.file().path())); + deindex(hndl.file()); d->loadedFiles.erase(found); deleteFile(hndl); return true; @@ -359,26 +359,6 @@ void FS::endStartup() d->usePrimaryWadLumpIndex(); } -int FS::unloadListFiles(de::FileList& list, bool nonStartup) -{ - int unloaded = 0; - // Unload in reverse load order. - for(int i = list.size() - 1; i >= 0; i--) - { - DFile* hndl = list[i]; - AbstractFile* file = hndl->file(); - if(!nonStartup || !file->hasStartup()) - { - if(unloadFile(Str_Text(file->path()), - true/*allow unloading game resources*/, true/*quiet please*/)) - { - ++unloaded; - } - } - } - return unloaded; -} - #if _DEBUG static void printFileIds(FileIds const& fileIds) { @@ -397,11 +377,11 @@ static void printFileList(de::FileList& list) for(int i = 0; i < list.size(); ++i) { de::DFile* hndl = list[i]; - de::AbstractFile* file = hndl->file(); - FileId fileId = FileId::fromPath(Str_Text(file->path())); + de::AbstractFile& file = hndl->file(); + FileId fileId = FileId::fromPath(Str_Text(file.path())); LOG_MSG(" %c%d: %s - \"%s\" [handle: %p]") - << (file->hasStartup()? '*' : ' ') << i - << fileId << F_PrettyPath(Str_Text(file->path())) << (void*)&hndl; + << (file.hasStartup()? '*' : ' ') << i + << fileId << F_PrettyPath(Str_Text(file.path())) << (void*)&hndl; } } #endif @@ -410,15 +390,27 @@ int FS::reset() { #if _DEBUG // List all open files with their identifiers. - VERBOSE( - Con_Printf("Open files at reset:\n"); + if(verbose) + { + LOG_MSG("Open files at reset:"); printFileList(d->openFiles); - Con_Printf("End\n") - ) + LOG_MSG("End\n"); + } #endif - // Perform non-startup file unloading... - int unloaded = unloadListFiles(d->loadedFiles, true/*non-startup*/); + // Perform non-startup file unloading (in reverse load order). + int unloaded = 0; + for(int i = d->loadedFiles.size() - 1; i >= 0; i--) + { + DFile& hndl = *(d->loadedFiles[i]); + AbstractFile& file = hndl.file(); + if(file.hasStartup()) continue; + + if(unloadFile(Str_Text(file.path()), true/*allow unloading game resources*/, true/*quiet please*/)) + { + unloaded += 1; + } + } #if _DEBUG // Sanity check: look for orphaned identifiers. @@ -690,31 +682,28 @@ void FS::closeAuxiliary() d->usePrimaryWadLumpIndex(); } -void FS::releaseFile(de::AbstractFile* file) +void FS::releaseFile(de::AbstractFile& file) { - if(!file) return; for(int i = d->openFiles.size() - 1; i >= 0; i--) { - DFile* hndl = d->openFiles[i]; - if(hndl->file() == file) + DFile& hndl = *(d->openFiles[i]); + if(&hndl.file() == &file) { d->openFiles.removeAt(i); } } } -void FS::closeFile(de::DFile* hndl) +void FS::closeFile(de::DFile& hndl) { - if(!hndl) return; - hndl->close(); + hndl.close(); } -void FS::deleteFile(de::DFile* hndl) +void FS::deleteFile(de::DFile& hndl) { - if(!hndl) return; - hndl->close(); - delete hndl->file(); - delete hndl; + closeFile(hndl); + delete &hndl.file(); + delete &hndl; } /// @return @c NULL= Not found. @@ -723,10 +712,10 @@ static WadFile* findFirstWadFile(de::FileList& list, bool custom) if(list.empty()) return 0; DENG2_FOR_EACH(i, list, de::FileList::iterator) { - de::AbstractFile* file = (*i)->file(); - if(custom != file->hasCustom()) continue; + de::AbstractFile& file = (*i)->file(); + if(custom != file.hasCustom()) continue; - WadFile* wad = dynamic_cast(file); + WadFile* wad = dynamic_cast(&file); if(wad) return wad; } return 0; @@ -1243,7 +1232,7 @@ bool FS::accessFile(char const* path) { DFile* hndl = tryOpenFile(path, "rx", 0, true); if(!hndl) return false; - deleteFile(hndl); + deleteFile(*hndl); return true; } @@ -1254,8 +1243,8 @@ uint FS::lastModified(char const* fileName) uint modified = 0; if(hndl) { - modified = hndl->file()->lastModified(); - deleteFile(hndl); + modified = hndl->file().lastModified(); + deleteFile(*hndl); } return modified; } @@ -1272,24 +1261,24 @@ de::AbstractFile* FS::addFile(char const* path, size_t baseOffset) return 0; } - AbstractFile* file = hndl->file(); - VERBOSE( Con_Message("Loading \"%s\"...\n", F_PrettyPath(Str_Text(file->path()))) ) + AbstractFile& file = hndl->file(); + VERBOSE( Con_Message("Loading \"%s\"...\n", F_PrettyPath(Str_Text(file.path()))) ) if(d->loadingForStartup) { - file->setStartup(true); + file.setStartup(true); } DFile* loadedFilesHndl = DFileBuilder::dup(*hndl); d->loadedFiles.push_back(loadedFilesHndl); loadedFilesHndl->setList(reinterpret_cast(&d->loadedFiles)); // Publish lumps to an index? - LumpIndex* lumpIndex = d->lumpIndexForFileType(file->type()); + LumpIndex* lumpIndex = d->lumpIndexForFileType(file.type()); if(lumpIndex) { - file->publishLumpsToIndex(*lumpIndex); + file.publishLumpsToIndex(*lumpIndex); } - return file; + return &file; } int FS::addFiles(char const* const* paths, int num) @@ -1749,11 +1738,11 @@ D_CMD(ListFiles) DENG2_FOR_EACH(i, foundFiles, de::FileList::const_iterator) { - de::AbstractFile* file = (*i)->file(); + de::AbstractFile& file = (*i)->file(); uint crc; - int fileCount = file->lumpCount(); - switch(file->type()) + int fileCount = file.lumpCount(); + switch(file.type()) { case FT_GENERICFILE: crc = 0; @@ -1762,19 +1751,19 @@ D_CMD(ListFiles) crc = 0; break; case FT_WADFILE: - crc = (!file->hasCustom()? reinterpret_cast(file)->calculateCRC() : 0); + crc = (!file.hasCustom()? reinterpret_cast(file).calculateCRC() : 0); break; case FT_LUMPFILE: crc = 0; break; default: - Con_Error("CCmdListLumps: Invalid file type %i.", file->type()); + Con_Error("CCmdListLumps: Invalid file type %i.", file.type()); exit(1); // Unreachable. } - Con_Printf("\"%s\" (%i %s%s)", F_PrettyPath(Str_Text(file->path())), + Con_Printf("\"%s\" (%i %s%s)", F_PrettyPath(Str_Text(file.path())), fileCount, fileCount != 1 ? "files" : "file", - (file->hasStartup()? ", startup" : "")); + (file.hasStartup()? ", startup" : "")); if(0 != crc) Con_Printf(" [%08x]", crc); Con_Printf("\n"); @@ -1906,7 +1895,8 @@ int F_RemoveFiles(char const* const* filenames, int num) void F_ReleaseFile(struct abstractfile_s* file) { - App_FileSystem()->releaseFile(reinterpret_cast(file)); + if(!file) return; + App_FileSystem()->releaseFile(*reinterpret_cast(file)); } struct dfile_s* F_Open3(char const* path, char const* mode, size_t baseOffset, boolean allowDuplicate) @@ -1956,12 +1946,14 @@ uint F_LumpLastModified(lumpnum_t absoluteLumpNum) void F_Close(struct dfile_s* hndl) { - App_FileSystem()->closeFile(reinterpret_cast(hndl)); + if(!hndl) return; + App_FileSystem()->closeFile(*reinterpret_cast(hndl)); } void F_Delete(struct dfile_s* hndl) { - App_FileSystem()->deleteFile(reinterpret_cast(hndl)); + if(!hndl) return; + App_FileSystem()->deleteFile(*reinterpret_cast(hndl)); } ddstring_t const* F_Path(struct abstractfile_s const* file) @@ -2073,7 +2065,7 @@ static ddstring_t* composeFilePathString(de::FileList& files, int flags = DEFAUL maxLength = 0; DENG2_FOR_EACH(i, files, de::FileList::const_iterator) { - ddstring_t const* path = (*i)->file()->path(); + ddstring_t const* path = (*i)->file().path(); if(!(flags & (PTSF_TRANSFORM_EXCLUDE_DIR|PTSF_TRANSFORM_EXCLUDE_EXT))) { @@ -2127,7 +2119,7 @@ static ddstring_t* composeFilePathString(de::FileList& files, int flags = DEFAUL n = 0; DENG2_FOR_EACH(i, files, de::FileList::const_iterator) { - ddstring_t const* path = (*i)->file()->path(); + ddstring_t const* path = (*i)->file().path(); if(flags & PTSF_QUOTED) Str_AppendChar(str, '"'); @@ -2196,11 +2188,11 @@ static bool findFilesPredicate(de::DFile* hndl, void* parameters) { DENG_ASSERT(parameters); findfilespredicate_params_t* p = (findfilespredicate_params_t*)parameters; - de::AbstractFile* file = hndl->file(); - if((!VALID_FILETYPE(p->type) || p->type == file->type()) && - ((p->markedCustom == file->hasCustom()))) + de::AbstractFile& file = hndl->file(); + if((!VALID_FILETYPE(p->type) || p->type == file.type()) && + ((p->markedCustom == file.hasCustom()))) { - ddstring_t const* path = file->path(); + ddstring_t const* path = file.path(); if(stricmp(Str_Text(path) + Str_Length(path) - 3, "lmp")) return true; // Include this. } diff --git a/doomsday/engine/portable/src/image.cpp b/doomsday/engine/portable/src/image.cpp index 721028049b..badc83047a 100644 --- a/doomsday/engine/portable/src/image.cpp +++ b/doomsday/engine/portable/src/image.cpp @@ -140,8 +140,8 @@ boolean Image_LoadFromFileWithFormat(image_t* img, const char* format, DFile* _h /// @todo There are too many copies made here. It would be best if image_t /// contained an instance of QImage. -jk - assert(img); - assert(_hndl); + DENG_ASSERT(img); + DENG_ASSERT(_hndl); de::DFile& hndl = *reinterpret_cast(_hndl); // It is assumed that file's position stays the same (could be trying multiple loaders). @@ -162,7 +162,7 @@ boolean Image_LoadFromFileWithFormat(image_t* img, const char* format, DFile* _h return false; } - //Con_Message("Loading %s\n", Str_Text(file->file()->Path()); + //Con_Message("Loading %s\n", Str_Text(hndl->file().path()); // Convert paletted images to RGB. if(image.colorCount())