PIM/MS Windows/SQLite Folder Indices/merge

From KDE Community Wiki

Validation of the SQLite folder indices merge into kdepim trunk (805075), to see whether the mmap mode is affected.

Status: finished; see results.

jstaniek 11:38, 8 May 2008 (CEST)

Notes

  • "OK" means files or methods inspected in every detail.
  • We do not mention changes that only add/remove kDebug(), etc.

Inspection

  • compactionjob.cpp - OK
    • void MboxCompactionJob::done( int rc ): the only addition: result of KDE_rename() is checked, return immediately on failure - OK
  • folderstorage.cpp, folderstorage.h - OK
    • QString FolderStorage::indexLocation() const - implemented here to return "*.index" name for mmap mode ("*.index.db" for sqlite mode). Previously it was abstract and implemented in KMFolderIndex. KMFolderSearch reimplements it to return "*.index.search" name - see kmfoldersearch.cpp for details.
    • added virtual QString FolderStorage::location(const QString& suffix) const helper, used in various *location() methods of in KMail, to avoid code duplication; returns full path to .index, .index.ids or .index.sorted file (depending on suffix); the former path (in case of mmap-based implementation) is returned when the suffix is empty.
    • QString FolderStorage::sortedLocation() const - added to avoid constructing paths using indexLocation() + ".sorted" code
    • QString FolderStorage::idsLocation() const - added to avoid constructing paths using indexLocation() + ".ids" code - OK
    • int FolderStorage::rename( const QString &newName, KMFolderDir *newParent ):
      • sortedLocation() is used instead of adding indexLocation()+".sorted" - OK
      • result of KDE_rename( oldIndexLoc, newIndexLoc ) is checked and we return immediately on failure - OK
      • result of KDE_rename( oldSortedLoc, newSortedLoc ) is checked and we return immediately on failure - OK
      • result of KDE_rename( oldIdsLoc, newIdsLoc ) is checked and we return immediately on failure - OK
    • void FolderStorage::remove() - sortedLocation() is used instead of adding indexLocation()+".sorted" - OK
    • void FolderStorage::invalidateFolder():
      • sortedLocation() is used instead of adding indexLocation()+".sorted" - OK
      • idsLocation() is used instead of adding indexLocation()+".ids" - OK
    • int FolderStorage::writeFolderIdsFile() const, int FolderStorage::touchFolderIdsFile(), int FolderStorage::appendToFolderIdsFile( int idx ) all return 0 (success) instead of -1 immediately if mExportsSernums == 0 - OK
  • kmailicalifaceimpl.cpp - OK

void KMailICalIfaceImpl::readConfig() - added sanity check:

if ( !mCalendar || !mTasks || !mJournals || !mContacts || !mNotes )
  return;

KMFolder* KMailICalIfaceImpl::initFolder( KMail::FolderContentsType contentsType ) - added check for result of open():

if ( 0 != folder->open( "ifacefolder" ) ) { ....
  • kmfolder.cpp, kmfolder.h - OK
    • QString KMFolder::indexLocation() const - made virtual but implementation is unaffected.
    • QString KMFolder::sortedLocation() const - calls FolderStorage::idsLocation() if storage is presents, else returns empty string; added as a helper for KMHeaders so we don't need to call mFolder->indexLocation() + ".sorted"; gives effectively the same result - OK
    • QString KMFolder::idsLocation() const - like above but for ".ids" names - OK
  • kmfolderdir.cpp - OK
    • bool KMFolderDir::reload(void): no changes, as .index.db files are only ignored for sqlite mode - OK
  • kmfolderindex.cpp, kmfolderindex.h - OK
    • uchar *indexStreamBasePtr(), size_t indexStreamLength() const, indexSwapByteOrder(), indexSizeOfLong(), bool updateIndexStreamPtr(bool just_close=false) methods and FILE* mIndexStream, off_t mHeaderOffset, uchar *mIndexStreamPtr, size_t mIndexStreamPtrLength, bool mIndexSwapByteOrder, int mIndexSizeOfLong members are unused in sqlite mode - mmap mode is unaffected - OK
    • bool openDatabase( int mode ), int writeMessages( KMMsgBase* msg, WriteMessagesMode mode ) and sqlite3 *mIndexDb member - unused in mmap mode - OK
    • implementation of virtual QString indexLocation() const is moved to FolderStorage (superclass), implementation itself remains unchanged - OK
    • added "kmfolderindex_common.cpp", included from "kmfolderindex.cpp" and "kmfolderindex_sqlite.cpp" - contains common includes and defines for these two files, and the following shared methods:
      • int KMFolderIndex::openInternal() added, common code for int KMFolderMaildir::open( const char * ) and int KMFolderMbox::open( const char *owner ) - see results of inspection of these methods for details - OK
      • int KMFolderIndex::createInternal() added, common code for KMFolderMaildir::create() and KMFolderMbox::create() - see results of inspection of these methods for details - OK
    • int KMFolderIndex::updateIndex() - minor optimization - OK
    • int KMFolderIndex::writeIndex( bool createEmptyIndex ) - OK
      • returns on unlink( QFile::encodeName( tempName ) ) failure
      • uses KMFolderIndex::writeMessages( KMMsgBase* msg, bool flush, FILE* indexStream ) to write the messages; writeMessages() provides the same code as that of writeIndex() except on failure of fwrite( buffer, len, 1, indexStream ), writing is stopped, the stream is closed by writeIndex(), and failure result is returned. - OK
      • on failure of updateIndexStreamPtr() or writeFolderIdsFile(), failure result is returned. - OK
    • int KMFolderIndex::writeMessages( KMMsgBase* msg, bool flush = true ) added, common code for KMFolderIndex::writeIndex(), KMFolderMaildir::addMsgInternal() and KMFolderMbox::addMsg(), see results of inspection of these methods for details - OK
    • bool KMFolderIndex::updateIndexStreamPtr(bool just_close):
      • result of munmap((char *)mIndexStreamPtr, mIndexStreamPtrLength) is checked (twice), false returned on failure - OK
    • void KMFolderIndex::fillMessageDict() - minor optimization - OK
  • kmfoldermaildir.cpp - OK
    • int KMFolderMaildir::open( const char * ) - OK
      • mIndexStream-dependent code ifdef'd for sqlite mode only - OK
      • main code moved to int common KMFolderIndex::openInternal(false/*!checkIfIndexTooOld*/) - OK
        • result of updateIndexStreamPtr() is checked, and return on failure - OK
        • if (!mIndexStream) condition replaced by if ( shouldCreateIndexFromContents ); it's equivalent because regardless of mIndexStream initial value:
          • if KMFolderIndex::IndexOk != indexStatus(), it's set to 0, and shouldCreateIndexFromContents to true - OK
          • else, mIndexStream is set to result of KDE_fopen(), and then shouldCreateIndexFromContents is equivalent to !mIndexStream. - OK
        • KMFolderIndex::IndexTooOld == index_status condition is not checked and message box is not displayed because bool checkIfIndexTooOld argument is set to false - OK
    • int KMFolderMaildir::create()
      • main code moved to int common int KMFolderIndex::createInternal(), the code is 100% equivalent - OK
    • void KMFolderMaildir::close( const char *, bool aForced ) - mIndexStream-dependent code ifdef'd for sqlite mode only - OK
    • void KMFolderMaildir::sync() - empty for sqlite mode, no changes for mmap mode - OK
    • int KMFolderMaildir::addMsgInternal( KMMessage* aMsg, int* index_return, bool stripUid ) - OK
      • mIndexStream-dependent code ifdef'd for sqlite mode only - OK
      • code for data writing replaced by writeMessages( mb, true /*flush*/ ) call - OK
    • int KMFolderMaildir::createIndexFromContents() -mHeaderOffset ifdef'd for sqlite mode only - OK
      • added return with failure on fwrite(buffer, len, 1, mIndexStream) failure (provided by writeMessages()) - OK
  • kmfoldermbox.cpp - OK
    • int KMFolderMbox::open( const char * ) - exactly like KMFolderMaildir::open( const char * ) except the KMFolderIndex::openInternal(true/*checkIfIndexTooOld*/) call takes true value, instead of false - OK
    • int KMFolderMbox::create()
      • main code moved to int common int KMFolderIndex::createInternal(), the code is 100% equivalent - OK
    • void KMFolderMbox::close( const char *, bool aForced ) - mIndexStream-dependent code ifdef'd for sqlite mode only - OK
    • void KMFolderMbox::sync() - empty for sqlite mode, no changes for mmap mode - OK
    • void KMFolderMbox::lock(), KMFolderMbox::unlock() - mIndexStream-dependent code ifdef'd for sqlite mode only - OK
    • int KMFolderMbox::createIndexFromContents() - ifdef'd one line using mHeaderOffset for sqlite mode only - OK
    • int KMFolderMbox::addMsg( KMMessage *aMsg, int *aIndex_ret )
      • mIndexStream-dependent code ifdef'd for sqlite mode only - OK
      • code for data writing replaced by writeMessages( mb, true /*flush*/ ) call
  • kmfoldersearch.cpp - OK
    • QString KMFolderSearch::indexLocation() const now just calls FolderStorage::location( "search" ) utility function what has the same effect
    • int KMFolderSearch::writeIndex( bool ): at the very end final, result of KDE_rename() from temp name to the indexLocation() name is checked; on faulure the method fails, what cause that "mDirty = false; mUnlinked = false;" code is not executed. - OK
  • kmheaders.cpp, kmheaders.h - OK
    • #define KMAIL_SORT_FILE(x) x->indexLocation() + ".sorted" replaced by #define KMAIL_SORT_FILE(x) x->sortedLocation() - uses a new utility FolderStorage::sortedLocation() method having basically the same source code - OK
    • void KMHeaders::setNestedOverride( bool override ) - like above - OK
    • void KMHeaders::setSubjectThreading( bool aSubjThreading ) - like above - OK
    • void KMHeaders::writeFolderConfig (void): minor optimization: mFolder->getMsgBase( current->msgId() ) is called once instead of twice - OK
    • bool KMHeaders::writeSortOrder() - the only addition: result of KDE_rename() is checked, return immediately on failure
    • QList< Q_UINT32 > KMHeaders::selectedSernums() - sanity check added in case when 
      KMMsgBase *msgBase = mFolder->getMsgBase( item->msgId() );
      returns null result; this shouldn't happen - OK
    • QList< Q_UINT32 > KMHeaders::selectedVisibleSernums() - like above
  • kmkernel.cpp - OK
    • void KMKernel::slotRunBackgroundTasks(): added sanity checks for null pointers: the_folderMgr, the_imapFolderMgr, the_dimapFolderMgr; this is neutral change - OK
  • kmmsgbase.cpp, kmmsgbase.h - OK
    • added ctor
      • KMMsgBase(KMFolder* aParentFolder, char* data, short len, sqlite_int64 id) for sqlite mode
      • KMMsgBase(KMFolder* aParentFolder, off_t off, short len) for mmap mode
      • In both cases the ctor is effectively the same as KMMsgBase(KMFolder* aParentFolder), but also initializes KMMsgBase members - OK
    • KMMsgBase::~KMMsgBase() - frees mData for sqlite mode - no changes for mmap mode - OK
    • void KMMsgBase::assign(const KMMsgBase* other) - for sqlite makes a deep copy of mData, copies mDbId value; no changes for mmap mode - OK
    • setData(char* data), setDbId(sqlite_int64) and sqlite_int64 dbId() const, and sqlite_int64 mDbId member: added only for sqlite mode, won't affect mmap mode - OK
    • off_t mIndexOffset and bool syncIndexString() const unused (ifdef'd) in sqlite mode - OK
      • syncIndexString(): added check: returns false on fwrite( buffer, len, 1, storage()->mIndexStream) failure - OK
    • QString KMMsgBase::getStringPart(MsgPartType t) const:
      • QString ret is moved up above 'retry:' label, nothing is affected because after ret modified (set to not empty), there is no path of return to the 'retry:' label - OK
      • plain rename from quint16 l to quint16 len no changes - OK
      • g_chunk_offset = 0 moved down to an initializer of the for loop, nothing is affected because g_chunk_offset is not used before the loop
      • the while(g_chunk_offset < mIndexLength) loop is replaced by for ( g_chunk_offset = 0; g_chunk_offset < mIndexLength; g_chunk_offset += len ):
        • g_chunk_offset = 0 precondition is unaffected - OK
        • the test expression g_chunk_offset < mIndexLength is unchanged - OK
        • the count command g_chunk_offset += len originally appeared at the end of while loop, this is equivalent as there are no continue command in the loop - OK
    • off_t KMMsgBase::getLongPart(MsgPartType t) const: changes are is similar to those in QString KMMsgBase::getStringPart(MsgPartType t) const:
      • the while loop is replaced by for - OK
      • quint16 len moved outside of the loop, nothing is affected by this, as the value is set at the beginning of each step of the loop - OK
  • kmmsgdict.cpp - OK
    • QString KMMsgDict::getFolderIdsLocation( const FolderStorage &storage ): the new FolderStorage::idsLocation() is called which provides the same code as the original impl. of KMMsgDict::getFolderIdsLocation(). - OK
  • kmmsginfo.cpp, kmmsginfo.h - OK
    • sqlite mode has different ctor: explicit KMMsgInfo(KMFolder* p, char* data=0, short len=0, sqlite_int64 dbId=0) - mmap mode is unaffected - OK
    • init(): mData is initialized in sqlite mode - mmap mode is unaffected - OK
  • messageproperty.cpp - OK
    • minor optimizations only added, for example 1. instead of 2.:
      1. QMap<quint32, QPointer<KMFolder> >::ConstIterator it = sFolders.constFind( serNum );
        return it == sFolders.constEnd() ? 0 : (*it).operator->(); - OK
      2. if (sFolders.contains(serNum))
          return sFolders[serNum].operator->();
        return 0; - OK
    • Affected methods:
      • KMFolder* MessageProperty::filterFolder( quint32 serNum ) - OK
      • ActionScheduler* MessageProperty::filterHandler( quint32 serNum ) - OK
      • bool MessageProperty::transferInProgress( quint32 serNum ) - OK
      • void MessageProperty::setTransferInProgress( quint32 serNum, bool transfer, bool force ) - OK
      • quint32 MessageProperty::serialCache( const KMMsgBase *msgBase ) - OK

Results

  • 805511, 805447 - added CreateIndexFromContentsWhenReadIndexFailed flag for openInternal(), used for mbox storage; added CheckIfIndexTooOld flag (also used for mbox) to skip message "The index of folder .. seems to be out of date" for maildir folders; this can probbaly fix recently reported issues with displaying the message for maildir folders.
Retrieved from "https://community.kde.org/index.php?title=PIM/MS_Windows/SQLite_Folder_Indices/merge&oldid=54083"