From 522aed9b73f340ccaaa87c883ba50e4a4d5b6494 Mon Sep 17 00:00:00 2001 From: James Turner Date: Tue, 16 Mar 2021 15:49:58 +0000 Subject: [PATCH] MMap : improved error reporting, Win32 compat Use strutils::sterror to print errno; convert to pimpl idiom to avoid pulling Windows.h into public view. --- simgear/io/sg_mmap.cxx | 292 ++++++++++++++++++++++++----------------- simgear/io/sg_mmap.hxx | 83 ++++-------- 2 files changed, 196 insertions(+), 179 deletions(-) mode change 100644 => 100755 simgear/io/sg_mmap.cxx diff --git a/simgear/io/sg_mmap.cxx b/simgear/io/sg_mmap.cxx old mode 100644 new mode 100755 index 72b6787a..119a2369 --- a/simgear/io/sg_mmap.cxx +++ b/simgear/io/sg_mmap.cxx @@ -24,27 +24,51 @@ #include #include - -#ifdef _WIN32 -# include -#else -# include -#endif - #include - #include #include #include -#if !defined(_MSC_VER) +#include +#include +#include +#include + +#include "sg_mmap.hxx" + +#if defined(SG_WINDOWS) +# define WIN32_LEAN_AND_MEAN +# include +# include +#else +# include # include #endif -#include -#include +class SGMMapFile::SGMMapFilePrivate +{ +public: + SGPath file_name; + int fp = -1; + bool eof_flag = true; + // Number of repetitions to play. -1 means loop infinitely. + int repeat = 1; + int iteration = 0; // number of current repetition, + // starting at 0 + int extraoflags = 0; -#include "sg_mmap.hxx" + char *buffer = nullptr; + off_t offset = 0; + size_t size = 0; +#if defined(SG_WINDOWS) + HANDLE _nativeHandle = NULL; +#endif + + ssize_t mmap_read(void *buf, size_t count); + ssize_t mmap_write(const void *buf, size_t count); + + off_t forward(off_t amount); +}; SGMMapFile::SGMMapFile( ) { @@ -52,51 +76,58 @@ SGMMapFile::SGMMapFile( ) } SGMMapFile::SGMMapFile(const SGPath &file, int repeat_, int extraoflags_ ) - : file_name(file), repeat(repeat_), extraoflags(extraoflags_) + : d(new SGMMapFilePrivate) { + d->file_name = file; + d->repeat = repeat_; + d->extraoflags = extraoflags_; set_type( sgFileType ); } -SGMMapFile::SGMMapFile( int existingFd ) : - fp(existingFd) +SGMMapFile::SGMMapFile( int existingFd ) + : d(new SGMMapFilePrivate) { + d->fp = existingFd; set_type( sgFileType ); } -SGMMapFile::~SGMMapFile() { +SGMMapFile::~SGMMapFile() +{ close(); } // open the file based on specified direction -bool SGMMapFile::open( const SGPath& file, const SGProtocolDir d ) { - file_name = file; - return open(d); +bool SGMMapFile::open( const SGPath& file, const SGProtocolDir dir ) +{ + d->file_name = file; + return open(dir); } // open the file based on specified direction -bool SGMMapFile::open( const SGProtocolDir d ) { - set_dir( d ); +bool SGMMapFile::open( const SGProtocolDir dir ) +{ + set_dir( dir ); #if defined(SG_WINDOWS) - std::wstring n = file_name.wstr(); + const std::wstring n = d->file_name.wstr(); #else - std::string n = file_name.utf8Str(); + const std::string n = d->file_name.utf8Str(); #endif if ( get_dir() == SG_IO_OUT ) { #if defined(SG_WINDOWS) int mode = _S_IREAD | _S_IWRITE; - fp = ::_wopen(n.c_str(), O_WRONLY | O_CREAT | O_TRUNC | extraoflags, mode); + d->fp = ::_wopen(n.c_str(), O_WRONLY | O_CREAT | O_TRUNC | d->extraoflags, mode); #else mode_t mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH; - fp = ::open( n.c_str(), O_WRONLY | O_CREAT | O_TRUNC | extraoflags, mode ); + d->fp = ::open( n.c_str(), O_WRONLY | O_CREAT | O_TRUNC | d->extraoflags, mode ); #endif } else if ( get_dir() == SG_IO_IN ) { #if defined(SG_WINDOWS) - fp = ::_wopen( n.c_str(), O_RDONLY | extraoflags ); + d->fp = ::_wopen( n.c_str(), O_RDONLY | d->extraoflags ); #else - fp = ::open( n.c_str(), O_RDONLY | extraoflags ); + d->fp = ::open( n.c_str(), O_RDONLY | d->extraoflags ); #endif } else { SG_LOG( SG_IO, SG_ALERT, @@ -104,30 +135,54 @@ bool SGMMapFile::open( const SGProtocolDir d ) { return false; } - if ( fp == -1 ) { - SG_LOG( SG_IO, SG_ALERT, "Error opening file: " << file_name ); + if ( d->fp == -1 ) { + SG_LOG( SG_IO, SG_ALERT, "Error opening file: " << d->file_name + << "\n\t" << simgear::strutils::error_string(errno)); return false; } // mmap struct stat statbuf; - fstat(fp, &statbuf); + fstat(d->fp, &statbuf); - size = (size_t)statbuf.st_size; - buffer = (char*)simple_mmap(fp, size, &un); - if (buffer == (char*)-1) - { - SG_LOG( SG_IO, SG_ALERT, "Error mmapping file: " << file_name ); + d->size = (size_t) statbuf.st_size; +#if defined(SG_WINDOWS) + HANDLE osfHandle = (HANDLE)_get_osfhandle(d->fp); + if (!osfHandle) { + SG_LOG( SG_IO, SG_ALERT, "Error mmapping file: _get_osfhandle failed:" << d->file_name + << "\n\t" << simgear::strutils::error_string(errno)); return false; } - eof_flag = false; + d->_nativeHandle = CreateFileMapping(osfHandle, NULL, PAGE_READONLY, 0, 0, NULL); + if (!d->_nativeHandle) { + SG_LOG( SG_IO, SG_ALERT, "Error mmapping file: CreateFileMapping failed:" << d->file_name + << "\n\t" << simgear::strutils::error_string(errno)); + return false; + } + d->buffer = reinterpret_cast(MapViewOfFile(d->_nativeHandle, FILE_MAP_READ, 0, 0, 0)); + if (!d->buffer) { + CloseHandle(d->_nativeHandle); + SG_LOG( SG_IO, SG_ALERT, "Error mmapping file: MapViewOfFile failed:" << d->file_name + << "\n\t" << simgear::strutils::error_string(errno)); + return false; + } +#else + d->buffer = (char*) mmap(0, d->size, PROT_READ, MAP_PRIVATE, d->fp, 0L); + if (d->buffer == MAP_FAILED) { + SG_LOG( SG_IO, SG_ALERT, "Error mmapping file: " << d->file_name + << "\n\t" << simgear::strutils::error_string(errno)); + return false; + } +#endif + d-> eof_flag = false; return true; } -off_t SGMMapFile::forward(off_t amount) { - if ((size_t)amount > size - offset) { +off_t SGMMapFile::SGMMapFilePrivate::forward(off_t amount) +{ + if ((size_t) amount > size - offset) { amount = size - offset; } offset += amount; @@ -138,78 +193,80 @@ off_t SGMMapFile::forward(off_t amount) { return amount; } -const char* SGMMapFile::advance(off_t amount) { - const char *ptr = buffer + offset; +const char* SGMMapFile::advance(off_t amount) +{ + const char *ptr = d->buffer + d->offset; - off_t advanced = forward(amount); + off_t advanced = d->forward(amount); if (advanced != amount) { - eof_flag = true; + d->eof_flag = true; return nullptr; } return ptr; } -ssize_t SGMMapFile::mmap_read(void *buf, size_t count) { +ssize_t SGMMapFile::SGMMapFilePrivate::mmap_read(void *buf, size_t count) +{ const char *ptr = buffer + offset; size_t result = forward(count); - memcpy(buf, ptr, result); - return result; } -ssize_t SGMMapFile::mmap_write(const void *buf, size_t count) { +ssize_t SGMMapFile::SGMMapFilePrivate::mmap_write(const void *buf, size_t count) +{ char *ptr = buffer + offset; size_t result = forward(count); - memcpy(ptr, buf, result); - return result; } // read a block of data of specified size -int SGMMapFile::read( char *buf, int length ) { +int SGMMapFile::read( char *buf, int length ) +{ // read a chunk - ssize_t result = mmap_read(buf, length); + ssize_t result = d->mmap_read(buf, length); if ( length > 0 && result == 0 ) { - if (repeat < 0 || iteration < repeat - 1) { - iteration++; + if (d->repeat < 0 || d->iteration < d->repeat - 1) { + d->iteration++; // loop reading the file, unless it is empty - off_t fileLen = offset; // lseek(0, SEEK_CUR) + off_t fileLen = d->offset; // lseek(0, SEEK_CUR) if (fileLen == 0) { - eof_flag = true; + d->eof_flag = true; return 0; } else { - offset = 0; // lseek(0, SEEK_SET) - return mmap_read(buf, length); + d->offset = 0; // lseek(0, SEEK_SET) + return d->mmap_read(buf, length); } } else { - eof_flag = true; + d->eof_flag = true; } } return result; } -int SGMMapFile::read( char *buf, int length, int num ) { - return mmap_read(buf, num*length)/length; +int SGMMapFile::read( char *buf, int length, int num ) +{ + return d->mmap_read(buf, num*length)/length; } // read a line of data, length is max size of input buffer -int SGMMapFile::readline( char *buf, int length ) { - int pos = offset; // pos = lseek(0, SEEK_CUR) +int SGMMapFile::readline( char *buf, int length ) +{ + int pos = d->offset; // pos = lseek(0, SEEK_CUR) // read a chunk - ssize_t result = mmap_read(buf, length); + ssize_t result = d->mmap_read(buf, length); if ( length > 0 && result == 0 ) { - if ((repeat < 0 || iteration < repeat - 1) && pos != 0) { - iteration++; + if ((d->repeat < 0 || d->iteration < d->repeat - 1) && pos != 0) { + d->iteration++; - pos = offset = 0; // pos = lseek(0, SEEK_SET) - result = mmap_read(buf, length); + pos = d->offset = 0; // pos = lseek(0, SEEK_SET) + result = d->mmap_read(buf, length); } else { - eof_flag = true; + d->eof_flag = true; } } @@ -217,28 +274,30 @@ int SGMMapFile::readline( char *buf, int length ) { int i; for ( i = 0; i < result && buf[i] != '\n'; ++i ); if ( buf[i] == '\n' ) { - result = i + 1; + result = i + 1; } else { - result = i; + result = i; } - offset = pos + result; // lseek(pos+result, SEEK_SET) + d->offset = pos + result; // lseek(pos+result, SEEK_SET) // just in case ... buf[ result ] = '\0'; - return result; } -std::string SGMMapFile::read_all() { - return std::string(buffer, size); +std::string SGMMapFile::read_all() +{ + return std::string(d->buffer, d->size); } // write data to a file -int SGMMapFile::write( const char *buf, const int length ) { - int result = mmap_write(buf, length); +int SGMMapFile::write( const char *buf, const int length ) +{ + int result = d->mmap_write(buf, length); if ( result != length ) { - SG_LOG( SG_IO, SG_ALERT, "Error writing data: " << file_name ); + SG_LOG( SG_IO, SG_ALERT, "Error writing data to mmaped-: " << d->file_name + << "\n\t" << simgear::strutils::error_string(errno)); } return result; @@ -246,65 +305,60 @@ int SGMMapFile::write( const char *buf, const int length ) { // write null terminated string to a file -int SGMMapFile::writestring( const char *str ) { +int SGMMapFile::writestring( const char *str ) +{ int length = std::strlen( str ); return write( str, length ); } - // close the port -bool SGMMapFile::close() { - if (fp != -1 ) { - simple_unmmap(buffer, size, &un); - if ( ::close( fp ) == -1 ) { +bool SGMMapFile::close() +{ + if (d->fp != -1 ) { +#if defined(SG_WINDOWS) + UnmapViewOfFile(d->buffer); + CloseHandle(d->_nativeHandle); + d->_nativeHandle = NULL; +#else + if (munmap(d->buffer, d->size) != 0) { + SG_LOG( SG_IO, SG_ALERT, "Error un-mapping mmaped-file: " << d->file_name + << "\n\t" << simgear::strutils::error_string(errno)); + } +#endif + if ( ::close( d->fp ) == -1 ) { + SG_LOG( SG_IO, SG_ALERT, "Error clossing mmaped-file: " << d->file_name + << "\n\t" << simgear::strutils::error_string(errno)); return false; } - eof_flag = true; + d->eof_flag = true; + d->fp = -1; return true; } return false; } -#ifdef WIN32 -/* Source: - * https://mollyrocket.com/forums/viewtopic.php?p=2529 - */ - -void * -SGMMapFile::simple_mmap(int fd, size_t length, SIMPLE_UNMMAP *un) +const char* SGMMapFile::get() const { - HANDLE f; - HANDLE m; - void *p; - - f = (HANDLE)_get_osfhandle(fd); - if (!f) return (void *)-1; - - m = CreateFileMapping(f, NULL, PAGE_READONLY, 0, 0, NULL); - if (!m) return (void *)-1; - - p = MapViewOfFile(m, FILE_MAP_READ, 0, 0, 0); - if (!p) - { - CloseHandle(m); - return (void *)-1; - } - - if (un) - { - un->m = m; - un->p = p; - } - - return p; + return d->buffer; } -void -SGMMapFile::simple_unmmap(void *addr, size_t len, SIMPLE_UNMMAP *un) +const char* SGMMapFile::ptr() const { - UnmapViewOfFile(un->p); - CloseHandle(un->m); + return d->buffer + d->offset; } -#endif +size_t SGMMapFile::get_size() const +{ + return d->size; +} + +bool SGMMapFile::eof() const +{ + return d->eof_flag; +} + +off_t SGMMapFile::forward(off_t amount) +{ + return d->forward(amount); +} diff --git a/simgear/io/sg_mmap.hxx b/simgear/io/sg_mmap.hxx index 673d2a55..b33c525e 100644 --- a/simgear/io/sg_mmap.hxx +++ b/simgear/io/sg_mmap.hxx @@ -19,60 +19,21 @@ // License along with this library; if not, write to the Free Software // Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA. -#ifndef _SG_MMAP_HXX -#define _SG_MMAP_HXX +#pragma once -#include - -#include +#include +#include #include "iochannel.hxx" -#include - -#ifdef WIN32 -# define WIN32_LEAN_AND_MEAN -# include -#endif +// forward decls +class SGPath; /** * A file I/O class based on SGIOChannel. */ -class SGMMapFile : public SGIOChannel { - - SGPath file_name; - int fp = -1; - bool eof_flag = true; - // Number of repetitions to play. -1 means loop infinitely. - const int repeat = 1; - int iteration = 0; // number of current repetition, - // starting at 0 - int extraoflags = 0; - - char *buffer = nullptr; - off_t offset = 0; - size_t size = 0; -#ifdef WIN32 - typedef struct - { - HANDLE m; - void *p; - } SIMPLE_UNMMAP; - SIMPLE_UNMMAP un; - - // map the file descriptor and return a pointer to the buffer. - static void *simple_mmap(int, size_t, SIMPLE_UNMMAP *); - static void simple_unmmap(void*, size_t, SIMPLE_UNMMAP *); -#else - int un; // referenced but not used - -# define simple_mmap(a, b, c) mmap(0, (b), PROT_READ, MAP_PRIVATE, (a), 0L) -# define simple_unmmap(a, b, c) munmap((a), (b)) -#endif - - ssize_t mmap_read(void *buf, size_t count); - ssize_t mmap_write(const void *buf, size_t count); - +class SGMMapFile : public SGIOChannel +{ public: SGMMapFile(); @@ -97,26 +58,26 @@ public: // open the file based on specified direction bool open( const SGPath& file, const SGProtocolDir dir ); - bool open( const SGProtocolDir dir ); + bool open(const SGProtocolDir dir) override; // read a block of data of specified size - int read( char *buf, int length ); + int read(char* buf, int length) override; // read a block of data of specified size int read( char *buf, int length, int num ); // read a line of data, length is max size of input buffer - int readline( char *buf, int length ); + int readline(char* buf, int length) override; // reads the whole file into a buffer // note: this really defeats the purpose of mmapping a file std::string read_all(); // get the pointer to the start of the buffer - inline const char *get() { return buffer; } + const char* get() const; // get the pointer to the current offset of the buffer - inline const char *ptr() { return buffer + offset; } + const char* ptr() const; // get the pointer at the current offset and increase the offset by amount // returns nullptr if the offset pointer would end up beyond the mmap @@ -124,7 +85,7 @@ public: const char* advance(off_t amount); // return the size of the mmaped area - inline size_t get_size() { return size; } + size_t get_size() const; // forward by amount, returns the amount which could be forwarded // without the offset getting beyond the mmap buffer size. @@ -132,22 +93,24 @@ public: off_t forward(off_t amount); // rewind the offset pointer - inline void rewind() { offset = 0; } + void rewind(); // write data to a file - int write( const char *buf, const int length ); + int write(const char* buf, const int length) override; // write null terminated string to a file - int writestring( const char *str ); + int writestring(const char* str) override; // close file - bool close(); + bool close() override; /** @return the name of the file being manipulated. */ - std::string get_file_name() const { return file_name.utf8Str(); } + std::string get_file_name() const; /** @return true of eof conditions exists */ - virtual bool eof() const { return eof_flag; }; -}; + bool eof() const override; -#endif // _SG_MMAP_HXX +private: + class SGMMapFilePrivate; + std::unique_ptr d; +};