From 5c158e5c701b1a8ae0d0e8a0e4efb61d7662c90b Mon Sep 17 00:00:00 2001 From: shenleban tongying Date: Fri, 14 Apr 2023 06:50:00 -0400 Subject: [PATCH] fix: remove the WriteBuffer from File::Class * This buffer was originally deisgned with FILE in mind * After migrating from FILE to QFile, the buffer is duplicated with QFile's buffer --- .clang-format | 4 +- file.cc | 304 ++++++++++++-------------------------------------- file.hh | 146 ++++++++++++------------ 3 files changed, 145 insertions(+), 309 deletions(-) diff --git a/.clang-format b/.clang-format index e9510fe4..6467a4cc 100644 --- a/.clang-format +++ b/.clang-format @@ -15,7 +15,7 @@ AllowAllArgumentsOnNextLine: false AllowAllParametersOfDeclarationOnNextLine: true AllowShortBlocksOnASingleLine: Empty AllowShortCaseLabelsOnASingleLine: false -AllowShortFunctionsOnASingleLine: All +AllowShortFunctionsOnASingleLine: Empty AllowShortLambdasOnASingleLine: Empty AllowShortIfStatementsOnASingleLine: Never AllowShortLoopsOnASingleLine: false @@ -23,7 +23,7 @@ AlwaysBreakAfterDefinitionReturnType: None AlwaysBreakAfterReturnType: None AllowShortEnumsOnASingleLine: false AlwaysBreakBeforeMultilineStrings: true -AlwaysBreakTemplateDeclarations: MultiLine +AlwaysBreakTemplateDeclarations: Yes BinPackArguments: false BinPackParameters: false BraceWrapping: diff --git a/file.cc b/file.cc index 1d2f66b3..a6ca8fa6 100644 --- a/file.cc +++ b/file.cc @@ -2,38 +2,20 @@ * Part of GoldenDict. Licensed under GPLv3 or later, see the LICENSE file */ #include "file.hh" - -#include -#include -#include - -#include -#include -#ifndef _MSC_VER -#include -#endif - -#ifdef __WIN32 -#include -#endif - -#include "ufile.hh" #include "fsencoding.hh" #include "zipfile.hh" -namespace File { +#include +#include +#ifdef __WIN32 + #include +#endif -enum -{ - // We employ a writing buffer to considerably speed up file operations when - // they consists of many small writes. The default size for the buffer is 64k - WriteBufferSize = 65536 -}; +namespace File { bool tryPossibleName( std::string const & name, std::string & copyTo ) { - if ( File::exists( name ) ) - { + if ( File::exists( name ) ) { copyTo = name; return true; } @@ -51,179 +33,96 @@ bool tryPossibleZipName( std::string const & name, std::string & copyTo ) return false; } -void loadFromFile( std::string const & n, std::vector< char > & data ) +void loadFromFile( std::string const & filename, std::vector< char > & data ) { - File::Class f( n, "rb" ); - - f.seekEnd(); - - data.resize( f.tell() ); - - f.rewind(); - - f.read( &data.front(), data.size() ); + File::Class f( filename, "rb" ); + QByteArray byteArray{ f.readall() }; + data.reserve( byteArray.size() ); + data = std::vector< char >( byteArray.cbegin(), byteArray.cend() ); } -bool exists( char const * filename ) noexcept -{ -#ifdef __WIN32 -#if defined(__WIN64) || defined(_MSC_VER) - struct _stat64 buf; -#else - struct _stat buf; -#endif - wchar_t wname[16384]; - MultiByteToWideChar( CP_UTF8, 0, filename, -1, wname, 16384 ); -#if defined(__WIN64) || defined(_MSC_VER) - return _wstat64( wname, &buf ) == 0; -#else - return _wstat( wname, &buf ) == 0; -#endif -#else - struct stat buf; - - // EOVERFLOW rationale: if the file is too large, it still does exist - return stat( filename, &buf ) == 0 || errno == EOVERFLOW; -#endif -} - -void Class::open( char const * filename, char const * mode ) +void Class::open( char const * mode ) { QFile::OpenMode openMode = QIODevice::Text; + const char * pch = mode; - while( *pch ) - { - switch( *pch ) - { - case 'r': openMode |= QIODevice::ReadOnly; - break; - case 'w': openMode |= QIODevice::WriteOnly; - break; - case '+': openMode &= ~( QIODevice::ReadOnly | QIODevice::WriteOnly ); - openMode |= QIODevice::ReadWrite; - break; - case 'a': openMode |= QIODevice::Append; - break; - case 'b': openMode &= ~QIODevice::Text; - break; - default: break; + while ( *pch ) { + switch ( *pch ) { + case 'r': + openMode |= QIODevice::ReadOnly; + break; + case 'w': + openMode |= QIODevice::WriteOnly; + break; + case '+': + openMode &= ~( QIODevice::ReadOnly | QIODevice::WriteOnly ); + openMode |= QIODevice::ReadWrite; + break; + case 'a': + openMode |= QIODevice::Append; + break; + case 'b': + openMode &= ~QIODevice::Text; + break; + default: + break; } ++pch; } - f.setFileName( filename ); - if ( !f.open( openMode ) ) - throw exCantOpen( std::string( filename ) + ": " + f.errorString().toUtf8().data() ); + throw exCantOpen( f.fileName().toStdString() + ": " + f.errorString().toUtf8().data() ); } -Class::Class( char const * filename, char const * mode ) : - writeBuffer( 0 ) +Class::Class( std::string_view filename, char const * mode ) { - open( filename, mode ); + f.setFileName( QString::fromUtf8( filename.data(), filename.size() ) ); + open( mode ); } -Class::Class( std::string const & filename, char const * mode ) - : writeBuffer( 0 ) +void Class::read( void * buf, qint64 size ) { - open( filename.c_str(), mode ); -} - -void Class::read( void * buf, qint64 size ) -{ - if ( !size ) - return; - - if ( writeBuffer ) - flushWriteBuffer(); - - qint64 result = f.read( reinterpret_cast( buf ), size ); + qint64 result = f.read( static_cast< char * >( buf ), size ); if ( result != size ) throw exReadError(); } -size_t Class::readRecords( void * buf, qint64 size, size_t count ) +size_t Class::readRecords( void * buf, qint64 size, qint64 count ) { - if ( writeBuffer ) - flushWriteBuffer(); - - qint64 result = f.read( reinterpret_cast( buf ), size * count ); + qint64 result = f.read( static_cast< char * >( buf ), size * count ); return result < 0 ? result : result / size; } -void Class::write( void const * buf, qint64 size ) +void Class::write( void const * buf, qint64 size ) { - if ( !size ) - return; - - if ( size >= WriteBufferSize ) - { - // If the write is large, there's not much point in buffering - flushWriteBuffer(); - - qint64 result = f.write( reinterpret_cast( buf ), size ); - - if ( result != size ) - throw exWriteError(); - + if ( 0 == size ) { return; } - if ( !writeBuffer ) - { - // Allocate the writing buffer since we don't have any yet - writeBuffer = new char[ WriteBufferSize ]; - if( !writeBuffer ) - throw exAllocation(); - writeBufferLeft = WriteBufferSize; + if ( size < 0 ) { + throw exWriteError(); } - size_t toAdd = size < writeBufferLeft ? size : writeBufferLeft; - - memcpy( writeBuffer + ( WriteBufferSize - writeBufferLeft ), - buf, toAdd ); - - size -= toAdd; - writeBufferLeft -= toAdd; - - if ( !writeBufferLeft ) // Out of buffer? Flush it. - { - flushWriteBuffer(); - - if ( size ) // Something's still left? Add to buffer. - { - memcpy( writeBuffer, (char const *)buf + toAdd, size ); - writeBufferLeft -= size; - } - } + f.write( static_cast< char const * >( buf ), size ); } -size_t Class::writeRecords( void const * buf, qint64 size, size_t count ) - +size_t Class::writeRecords( void const * buf, qint64 size, qint64 count ) { - flushWriteBuffer(); - - qint64 result = f.write( reinterpret_cast( buf ), size * count ); + qint64 result = f.write( static_cast< const char * >( buf ), size * count ); return result < 0 ? result : result / size; } char * Class::gets( char * s, int size, bool stripNl ) - { - if ( writeBuffer ) - flushWriteBuffer(); + qint64 len = f.readLine( s, size ); + char * result = len > 0 ? s : nullptr; - qint64 len = f.readLine( s, size ); - char * result = len > 0 ? s : NULL; + if ( result && stripNl ) { - if ( result && stripNl ) - { - char * last = result + len; - while( len-- ) - { + while ( len-- ) { --last; if ( *last == '\n' || *last == '\r' ) @@ -236,30 +135,30 @@ char * Class::gets( char * s, int size, bool stripNl ) return result; } -std::string Class::gets( bool stripNl ) +std::string Class::gets( bool stripNl ) { char buf[ 1024 ]; if ( !gets( buf, sizeof( buf ), stripNl ) ) throw exReadError(); - return std::string( buf ); + return { buf }; } -void Class::seek( qint64 offset ) +QByteArray Class::readall() { - if ( writeBuffer ) - flushWriteBuffer(); + return f.readAll(); +}; + +void Class::seek( qint64 offset ) +{ if ( !f.seek( offset ) ) throw exSeekError(); } uchar * Class::map( qint64 offset, qint64 size ) { - if( writeBuffer ) - flushWriteBuffer(); - return f.map( offset, size ); } @@ -269,102 +168,41 @@ bool Class::unmap( uchar * address ) } -void Class::seekCur( qint64 offset ) +void Class::seekEnd() { - if ( writeBuffer ) - flushWriteBuffer(); - - if( !f.seek( f.pos() + offset ) ) + if ( !f.seek( f.size() ) ) throw exSeekError(); } -void Class::seekEnd( qint64 offset ) -{ - if ( writeBuffer ) - flushWriteBuffer(); - - if( !f.seek( f.size() + offset ) ) - throw exSeekError(); -} - -void Class::rewind() +void Class::rewind() { seek( 0 ); } -qint64 Class::tell() +qint64 Class::tell() { - qint64 result = f.pos(); - - if ( result == -1 ) - throw exSeekError(); - - if ( writeBuffer ) - result += ( WriteBufferSize - writeBufferLeft ); - - return result; + return f.pos(); } -bool Class::eof() +bool Class::eof() const { - if ( writeBuffer ) - flushWriteBuffer(); - return f.atEnd(); } -QFile & Class::file() +QFile & Class::file() { - flushWriteBuffer(); - return f; } -void Class::close() +void Class::close() { - releaseWriteBuffer(); f.close(); } Class::~Class() noexcept { - if ( f.isOpen() ) - { - try - { - releaseWriteBuffer(); - } - catch( exWriteError & ) - { - } - f.close(); - } -} - -void Class::flushWriteBuffer() -{ - if ( writeBuffer && writeBufferLeft != WriteBufferSize ) - { - qint64 result = f.write( writeBuffer, WriteBufferSize - writeBufferLeft ); - - if ( result != WriteBufferSize - writeBufferLeft ) - throw exWriteError(); - - writeBufferLeft = WriteBufferSize; - } -} - -void Class::releaseWriteBuffer() -{ - flushWriteBuffer(); - - if ( writeBuffer ) - { - delete [] writeBuffer; - - writeBuffer = 0; - } + f.close(); } -} +} // namespace File diff --git a/file.hh b/file.hh index fa819306..045a115a 100644 --- a/file.hh +++ b/file.hh @@ -1,19 +1,22 @@ /* This file is (c) 2008-2012 Konstantin Isakov * Part of GoldenDict. Licensed under GPLv3 or later, see the LICENSE file */ -#ifndef __FILE_HH_INCLUDED__ -#define __FILE_HH_INCLUDED__ +#ifndef GOLDENDICT_FILE_HH +#define GOLDENDICT_FILE_HH +#include "ex.hh" +#include "mutex.hh" +#include +#include #include #include #include -#include -#include "ex.hh" -#include "mutex.hh" -/// A simple wrapper over FILE * operations with added write-buffering, -/// used for non-Qt parts of code. -/// It is possible to ifdef implementation details for some platforms. +/// A simple wrapper over QFile with some convenient GD specific functions +/// Consider the wrapped QFile as private implementation in the `Pimpl Idiom` +/// +/// Note: this is used *only* in code related to `Dictionary::CLass` to manage dict files. +/// In other places, just use QFile directly. namespace File { DEF_EX( Ex, "File exception", std::exception ) @@ -23,111 +26,106 @@ DEF_EX( exWriteError, "Error writing to the file", Ex ) DEF_EX( exSeekError, "File seek error", Ex ) DEF_EX( exAllocation, "Memory allocation error", Ex ) -/// Checks if the file exists or not. - bool tryPossibleName( std::string const & name, std::string & copyTo ); bool tryPossibleZipName( std::string const & name, std::string & copyTo ); -void loadFromFile( std::string const & n, std::vector< char > & data ); +void loadFromFile( std::string const & filename, std::vector< char > & data ); -bool exists( char const * filename ) noexcept; - -inline bool exists( std::string const & filename ) noexcept -{ return exists( filename.c_str() ); } +// QFileInfo::exists but used for std::string and char* +inline bool exists( std::string_view filename ) noexcept +{ + return QFileInfo::exists( QString::fromUtf8( filename.data(), filename.size() ) ); +}; class Class { QFile f; - char * writeBuffer; - qint64 writeBufferLeft; - - void open( char const * filename, char const * mode ) ; public: QMutex lock; - Class( char const * filename, char const * mode ) ; + // Create QFile Object and open() it. + Class( std::string_view filename, char const * mode ); - Class( std::string const & filename, char const * mode ) ; + /// QFile::read & QFile::write , but with exception throwing + void read( void * buf, qint64 size ); + void write( void const * buf, qint64 size ); - /// Reads the number of bytes to the buffer, throws an error if it - /// failed to fill the whole buffer (short read, i/o error etc). - void read( void * buf, qint64 size ) ; + // Read sizeof(T) bytes and convert the read data to T + template< typename T > + T read() + { + T value; + readType( value ); + return value; + } template< typename T > - void read( T & value ) - { read( &value, sizeof( value ) ); } - - template< typename T > - T read() - { T value; read( value ); return value; } + void write( T const & value ) + { + write( &value, sizeof( value ) ); + } /// Attempts reading at most 'count' records sized 'size'. Returns /// the number of records it managed to read, up to 'count'. - size_t readRecords( void * buf, qint64 size, size_t count ) ; - - /// Writes the number of bytes from the buffer, throws an error if it - /// failed to write the whole buffer (short write, i/o error etc). - /// This function employs write buffering, and as such, writes may not - /// end up on disk immediately, or a short write may occur later - /// than it really did. If you don't want write buffering, use - /// writeRecords() function instead. - void write( void const * buf, qint64 size ) ; - - template< typename T > - void write( T const & value ) - { write( &value, sizeof( value ) ); } + size_t readRecords( void * buf, qint64 size, qint64 count ); /// Attempts writing at most 'count' records sized 'size'. Returns /// the number of records it managed to write, up to 'count'. /// This function does not employ buffering, but flushes the buffer if it /// was used before. - size_t writeRecords( void const * buf, qint64 size, size_t count ) - ; + size_t writeRecords( void const * buf, qint64 size, qint64 count ); - /// Reads a string from the file. Unlike the normal fgets(), this one - /// can strip the trailing newline character, if this was requested. + /// Read a line with option to strip the trailing newline character /// Returns either s or 0 if no characters were read. - char * gets( char * s, int size, bool stripNl = false ) ; + char * gets( char * s, int size, bool stripNl = false ); - /// Like the above, but uses its own local internal buffer (1024 bytes - /// currently), and strips newlines by default. - std::string gets( bool stripNl = true ) ; + /// Like the above, but uses its own local internal buffer and strips newlines by default. + std::string gets( bool stripNl = true ); - /// Seeks in the file, relative to its beginning. - void seek( qint64 offset ) ; - uchar * map( qint64 offset, qint64 size ); - /// Seeks in the file, relative to the current position. - void seekCur( qint64 offset ) ; - /// Seeks in the file, relative to the end of file. - void seekEnd( qint64 offset = 0 ) ; + /// export QFile::readall + QByteArray readall(); + + /// export QFile::seek + void seek( qint64 offset ); + + /// Seeks to the end of file. + void seekEnd(); /// Seeks to the beginning of file - void rewind() ; + void rewind(); /// Tells the current position within the file, relative to its beginning. - qint64 tell() ; + qint64 tell(); - /// Returns true if end-of-file condition is set. - bool eof() ; + /// QFile::atEnd() const + bool eof() const; - /// Returns the underlying FILE * record, so other operations can be - /// performed on it. - QFile & file() ; - - /// Closes the file. No further operations are valid. - void close() ; - - ~Class() noexcept; + uchar * map( qint64 offset, qint64 size ); bool unmap( uchar * address ); -private: - void flushWriteBuffer() ; - void releaseWriteBuffer() ; + /// Returns the underlying QFile* , so other operations can be + /// performed on it. + QFile & file(); + + /// Closes the file. No further operations are valid. + void close(); + + ~Class() noexcept; + +private: + // QFile::open but with fopen-like mode settings. + void open( char const * mode ); + + template< typename T > + void readType( T & value ) + { + read( &value, sizeof( value ) ); + } }; -} +} // namespace File #endif