From 2e1822c9ed231b25fd474117a01a1492d4209fa4 Mon Sep 17 00:00:00 2001 From: derek Date: Fri, 7 Oct 2011 18:19:09 -0400 Subject: [PATCH] Cleaned up output/exception/asserts, etc after last merge --- src/api/BamIndex.h | 4 +- src/api/IBamIODevice.h | 39 +++++--- src/api/internal/BamFile_p.cpp | 13 ++- src/api/internal/BamPipe_p.cpp | 27 ++---- src/api/internal/BamReader_p.cpp | 2 +- src/api/internal/BamWriter_p.cpp | 17 +--- src/api/internal/BgzfStream_p.cpp | 127 ++++++-------------------- src/api/internal/BgzfStream_p.h | 7 +- src/api/internal/ILocalIODevice_p.cpp | 16 ++-- src/shared/bamtools_global.h | 56 +++++++----- src/utils/bamtools_utilities.h | 2 +- 11 files changed, 118 insertions(+), 192 deletions(-) diff --git a/src/api/BamIndex.h b/src/api/BamIndex.h index b202e92..067244e 100644 --- a/src/api/BamIndex.h +++ b/src/api/BamIndex.h @@ -78,14 +78,14 @@ class API_EXPORT BamIndex { // internal methods protected: - void SetErrorString(const std::string& where, const std::string& what) { + void SetErrorString(const std::string& where, const std::string& what) const { m_errorString = where + ": " + what; } // data members protected: Internal::BamReaderPrivate* m_reader; // copy, not owned - std::string m_errorString; + mutable std::string m_errorString; }; } // namespace BamTools diff --git a/src/api/IBamIODevice.h b/src/api/IBamIODevice.h index 99454b2..5c1856e 100644 --- a/src/api/IBamIODevice.h +++ b/src/api/IBamIODevice.h @@ -1,3 +1,20 @@ +// *************************************************************************** +// IBamIODevice.h (c) 2011 Derek Barnett +// Marth Lab, Department of Biology, Boston College +// --------------------------------------------------------------------------- +// Last modified: 7 October 2011 (DB) +// --------------------------------------------------------------------------- +// Base class for all BAM I/O devices (e.g. local file, pipe, HTTP, FTP, etc.) +// +// Derived classes should provide protocol-specific implementations for +// reading/writing plain bytes, as well as other I/O-related behaviors. +// +// Since IBamIODevices may be defined in client code, the internal +// BamExceptions are NOT allowed to be thrown from devices, including the +// built-in ones. This keeps a consistent interface at the BgzfStream for +// handling any device type. Use the error string for relaying error messages. +// *************************************************************************** + #ifndef IBAMIODEVICE_H #define IBAMIODEVICE_H @@ -16,8 +33,7 @@ class API_EXPORT IBamIODevice { // ctor & dtor public: - IBamIODevice(void); - virtual ~IBamIODevice(void); + virtual ~IBamIODevice(void) { } // IBamIODevice interface public: @@ -32,13 +48,14 @@ class API_EXPORT IBamIODevice { virtual size_t Write(const char* data, const unsigned int numBytes) =0; // default implementation provided - virtual std::string ErrorString(void); + virtual std::string GetErrorString(void); virtual bool IsOpen(void) const; virtual OpenMode Mode(void) const; // internal methods protected: - void SetErrorString(const std::string& errorString); + IBamIODevice(void); // hidden ctor + void SetErrorString(const std::string& where, const std::string& what); // data members protected: @@ -52,13 +69,8 @@ IBamIODevice::IBamIODevice(void) { } inline -IBamIODevice::~IBamIODevice(void) { } - -inline -std::string IBamIODevice::ErrorString(void) { - std::string e = m_errorString; - m_errorString.clear(); - return e; +std::string IBamIODevice::GetErrorString(void) { + return m_errorString; } inline @@ -72,8 +84,9 @@ IBamIODevice::OpenMode IBamIODevice::Mode(void) const { } inline -void IBamIODevice::SetErrorString(const std::string& errorString) { - m_errorString = errorString; +void IBamIODevice::SetErrorString(const std::string& where, const std::string& what) { + static const std::string SEPARATOR = ": "; + m_errorString = where + SEPARATOR + what; } } // namespace BamTools diff --git a/src/api/internal/BamFile_p.cpp b/src/api/internal/BamFile_p.cpp index 6cb5be4..3927d30 100644 --- a/src/api/internal/BamFile_p.cpp +++ b/src/api/internal/BamFile_p.cpp @@ -2,7 +2,7 @@ // BamFile_p.cpp (c) 2011 Derek Barnett // Marth Lab, Department of Biology, Boston College // --------------------------------------------------------------------------- -// Last modified: 9 September 2011 (DB) +// Last modified: 7 October 2011 (DB) // --------------------------------------------------------------------------- // Provides BAM file-specific IO behavior // *************************************************************************** @@ -44,15 +44,15 @@ bool BamFile::Open(const IBamIODevice::OpenMode mode) { else if ( mode == IBamIODevice::WriteOnly ) m_stream = fopen(m_filename.c_str(), "wb"); else { - SetErrorString("BamFile ERROR - unknown device open mode"); + SetErrorString("BamFile::Open", "unknown open mode requested"); return false; } // check that we obtained a valid FILE* if ( m_stream == 0 ) { - string error = "BamFile ERROR - could not open handle on "; - error += ( (m_filename.empty()) ? "empty filename" : m_filename ); - SetErrorString(error); + const string message_base = string("could not open file handle for "); + const string message = message_base + ( (m_filename.empty()) ? "empty filename" : m_filename ); + SetErrorString("BamFile::Open", message); return false; } @@ -63,6 +63,5 @@ bool BamFile::Open(const IBamIODevice::OpenMode mode) { bool BamFile::Seek(const int64_t& position) { BT_ASSERT_X( m_stream, "BamFile::Seek() - null stream" ); - cerr << "BamFile::Seek() - about to attempt seek" << endl; - return ( fseek64(m_stream, position, SEEK_SET) == 0); + return ( fseek64(m_stream, position, SEEK_SET) == 0 ); } diff --git a/src/api/internal/BamPipe_p.cpp b/src/api/internal/BamPipe_p.cpp index 62fd789..1d57ac3 100644 --- a/src/api/internal/BamPipe_p.cpp +++ b/src/api/internal/BamPipe_p.cpp @@ -2,7 +2,7 @@ // BamPipe_p.cpp (c) 2011 Derek Barnett // Marth Lab, Department of Biology, Boston College // --------------------------------------------------------------------------- -// Last modified: 9 September 2011 (DB) +// Last modified: 7 October 2011 (DB) // --------------------------------------------------------------------------- // Provides BAM pipe-specific IO behavior // *************************************************************************** @@ -34,15 +34,15 @@ bool BamPipe::Open(const IBamIODevice::OpenMode mode) { else if ( mode == IBamIODevice::WriteOnly ) m_stream = freopen(0, "wb", stdout); else { - SetErrorString("BamPipe ERROR - unsupported device mode"); + SetErrorString("BamPipe::Open", "unknown open mode requested"); return false; } // check that we obtained a valid FILE* if ( m_stream == 0 ) { - string error = "BamPipe ERROR - could not open handle on "; - error += ( (mode == IBamIODevice::ReadOnly) ? "stdin" : "stdout" ); - SetErrorString(error); + const string message_base = string("could not open handle on "); + const string message = message_base + ( (mode == IBamIODevice::ReadOnly) ? "stdin" : "stdout" ); + SetErrorString("BamPipe::Open", message); return false; } @@ -51,18 +51,7 @@ bool BamPipe::Open(const IBamIODevice::OpenMode mode) { return true; } -bool BamPipe::Seek(const int64_t& position) { -// (void)position; // suppress compiler warning about unused variable -// return false; // seeking not allowed in pipe - - BT_ASSERT_X( m_stream, "BamFile::Seek() - null stream" ); - cerr << "BamPipe::Seek() - about to attempt seek" << endl; - bool result = ( fseek64(m_stream, position, SEEK_SET) == 0); - if ( !result ) { - cerr << "BamPipe can't be seeked in" << endl; - } - return result; - -// return ( fseek64(m_stream, position, SEEK_SET) == 0); - +bool BamPipe::Seek(const int64_t& ) { + SetErrorString("BamPipe::Seek", "random access not allowed in FIFO pipe"); + return false; } diff --git a/src/api/internal/BamReader_p.cpp b/src/api/internal/BamReader_p.cpp index 5d154a1..393b168 100644 --- a/src/api/internal/BamReader_p.cpp +++ b/src/api/internal/BamReader_p.cpp @@ -367,7 +367,7 @@ bool BamReaderPrivate::Open(const string& filename) { Close(); // open BgzfStream - m_stream.Open(filename, "rb"); + m_stream.Open(filename, IBamIODevice::ReadOnly); assert(m_stream); // load BAM metadata diff --git a/src/api/internal/BamWriter_p.cpp b/src/api/internal/BamWriter_p.cpp index ce5cfa9..100de2d 100644 --- a/src/api/internal/BamWriter_p.cpp +++ b/src/api/internal/BamWriter_p.cpp @@ -2,18 +2,15 @@ // BamWriter_p.cpp (c) 2010 Derek Barnett // Marth Lab, Department of Biology, Boston College // --------------------------------------------------------------------------- -// Last modified: 6 October 2011 (DB) +// Last modified: 7 October 2011 (DB) // --------------------------------------------------------------------------- // Provides the basic functionality for producing BAM files // *************************************************************************** #include #include -<<<<<<< HEAD -#include -======= #include ->>>>>>> iodevice +#include #include using namespace BamTools; using namespace BamTools::Internal; @@ -160,16 +157,10 @@ bool BamWriterPrivate::Open(const string& filename, const string& samHeaderText, const RefVector& referenceSequences) { -<<<<<<< HEAD try { -======= - // open the BGZF file for writing, return failure if error - if ( !m_stream.Open(filename, IBamIODevice::WriteOnly) ) - return false; ->>>>>>> iodevice - // open the BGZF file for writing, return failure if error - m_stream.Open(filename, "wb"); + // open the BGZF file for writing + m_stream.Open(filename, IBamIODevice::WriteOnly); // write BAM file 'metadata' components WriteMagicNumber(); diff --git a/src/api/internal/BgzfStream_p.cpp b/src/api/internal/BgzfStream_p.cpp index 2053220..f70b97e 100644 --- a/src/api/internal/BgzfStream_p.cpp +++ b/src/api/internal/BgzfStream_p.cpp @@ -25,9 +25,7 @@ using namespace std; // RaiiWrapper implementation // ---------------------------- -BgzfStream::RaiiWrapper::RaiiWrapper(void) - : Stream(0) -{ +BgzfStream::RaiiWrapper::RaiiWrapper(void) { CompressedBlock = new char[Constants::BGZF_MAX_BLOCK_SIZE]; UncompressedBlock = new char[Constants::BGZF_DEFAULT_BLOCK_SIZE]; } @@ -39,12 +37,6 @@ BgzfStream::RaiiWrapper::~RaiiWrapper(void) { delete[] UncompressedBlock; CompressedBlock = 0; UncompressedBlock = 0; - - if ( Stream ) { - fflush(Stream); - fclose(Stream); - Stream = 0; - } } // --------------------------- @@ -56,8 +48,6 @@ BgzfStream::BgzfStream(void) : m_blockLength(0) , m_blockOffset(0) , m_blockAddress(0) - , m_isOpen(false) - , m_isWriteOnly(false) , m_isWriteCompressed(true) , m_device(0) { } @@ -82,9 +72,14 @@ bool BgzfStream::CheckBlockHeader(char* header) { // closes BGZF file void BgzfStream::Close(void) { + // reset state + m_blockLength = 0; + m_blockOffset = 0; + m_blockAddress = 0; + m_isWriteCompressed = true; + // skip if no device open - if ( m_device == 0 ) - return; + if ( m_device == 0 ) return; // if writing to file, flush the current BGZF block, // then write an empty block (as EOF marker) @@ -98,20 +93,6 @@ void BgzfStream::Close(void) { m_device->Close(); delete m_device; m_device = 0; - - // ?? - fflush(Resources.Stream); - fclose(Resources.Stream); - Resources.Stream = 0; - - // reset state - m_blockLength = 0; - m_blockOffset = 0; - m_blockAddress = 0; - m_isOpen = false; - m_isWriteOnly = false; - m_isWriteCompressed = true; - } // compresses the current block @@ -134,7 +115,7 @@ size_t BgzfStream::DeflateBlock(void) { const int compressionLevel = ( m_isWriteCompressed ? Z_DEFAULT_COMPRESSION : 0 ); // loop to retry for blocks that do not compress enough - int inputLength = BlockOffset; + int inputLength = m_blockOffset; size_t compressedLength = 0; const unsigned int bufferSize = Constants::BGZF_MAX_BLOCK_SIZE; @@ -287,72 +268,22 @@ bool BgzfStream::IsOpen(void) const { return m_device->IsOpen(); } -bool BgzfStream::Open(const string& filename, const IBamIODevice::OpenMode mode) { +void BgzfStream::Open(const string& filename, const IBamIODevice::OpenMode mode) { // close current device if necessary Close(); - - // sanity check BT_ASSERT_X( (m_device == 0), "BgzfStream::Open() - unable to properly close previous IO device" ); // retrieve new IO device depending on filename m_device = BamDeviceFactory::CreateDevice(filename); - - // sanity check BT_ASSERT_X( m_device, "BgzfStream::Open() - unable to create IO device from filename" ); // if device fails to open if ( !m_device->Open(mode) ) { - cerr << "BgzfStream::Open() - unable to open IO device:" << endl; - cerr << m_device->ErrorString(); - return false; - } - - // otherwise, set flag & return true - m_isOpen = true; - m_isWriteOnly = ( mode == IBamIODevice::WriteOnly ); - return true; - -} - -// opens the BGZF file for reading (mode is either "rb" for reading, or "wb" for writing) -void BgzfStream::Open(const string& filename, const char* mode) { - - // make sure we're starting with fresh state - if ( IsOpen() ) - Close(); - - // determine open mode - if ( strcmp(mode, "rb") == 0 ) - m_isWriteOnly = false; - else if ( strcmp(mode, "wb") == 0) - m_isWriteOnly = true; - else { - const string message = string("unknown file mode: ") + mode; - throw BamException("BgzfStream::Open", message); - } - - // open BGZF stream on a file - if ( (filename != "stdin") && (filename != "stdout") && (filename != "-")) - Resources.Stream = fopen(filename.c_str(), mode); - - // open BGZF stream on stdin - else if ( (filename == "stdin" || filename == "-") && (strcmp(mode, "rb") == 0 ) ) - Resources.Stream = freopen(NULL, mode, stdin); - - // open BGZF stream on stdout - else if ( (filename == "stdout" || filename == "-") && (strcmp(mode, "wb") == 0) ) - Resources.Stream = freopen(NULL, mode, stdout); - - // ensure valid Stream - if ( !Resources.Stream ) { - const string message = string("unable to open file: ") + filename; + const string deviceError = m_device->GetErrorString(); + const string message = string("could not open BGZF stream: \n\t") + deviceError; throw BamException("BgzfStream::Open", message); } - - // set flag & return success - m_isOpen = true; - return true; } // reads BGZF data into a byte buffer @@ -394,8 +325,8 @@ size_t BgzfStream::Read(char* data, const size_t dataLength) { // update block data if ( m_blockOffset == m_blockLength ) { m_blockAddress = m_device->Tell(); - m_BlockOffset = 0; - m_BlockLength = 0; + m_blockOffset = 0; + m_blockLength = 0; } @@ -418,7 +349,7 @@ void BgzfStream::ReadBlock(void) { // if block header empty if ( numBytesRead == 0 ) { m_blockLength = 0; - return true; + return; } // if block header invalid size @@ -454,27 +385,26 @@ void BgzfStream::Seek(const int64_t& position) { BT_ASSERT_X( m_device, "BgzfStream::Seek() - trying to seek on null IO device"); - // skip if not open or not seek-able - if ( !IsOpen() /*|| !m_device->IsRandomAccess()*/ ) { - cerr << "BgzfStream::Seek() - device not open" << endl; - return false; - } + // skip if device is not open + if ( !IsOpen() ) return; // determine adjusted offset & address int blockOffset = (position & 0xFFFF); int64_t blockAddress = (position >> 16) & 0xFFFFFFFFFFFFLL; // attempt seek in file - if ( !m_device->Seek(blockAddress) ) { + if ( m_device->IsRandomAccess() && m_device->Seek(blockAddress) ) { + + // update block data & return success + m_blockLength = 0; + m_blockAddress = blockAddress; + m_blockOffset = blockOffset; + } + else { stringstream s(""); s << "unable to seek to position: " << position; throw BamException("BgzfStream::Seek", s.str()); } - - // update block data & return success - m_blockLength = 0; - m_blockAddress = blockAddress; - m_blockOffset = blockOffset; } void BgzfStream::SetWriteCompressed(bool ok) { @@ -483,7 +413,8 @@ void BgzfStream::SetWriteCompressed(bool ok) { // get file position in BGZF file int64_t BgzfStream::Tell(void) const { - if ( !m_isOpen ) return 0; + if ( !IsOpen() ) + return 0; return ( (m_blockAddress << 16) | (m_blockOffset & 0xFFFF) ); } @@ -495,8 +426,8 @@ size_t BgzfStream::Write(const char* data, const size_t dataLength) { "BgzfStream::Write() - trying to write to non-writable IO device"); // skip if file not open for writing - if ( !IsOpen || !IsWriteOnly ) - return false; + if ( !IsOpen() ) + return 0; // write blocks as needed til all data is written size_t numBytesWritten = 0; diff --git a/src/api/internal/BgzfStream_p.h b/src/api/internal/BgzfStream_p.h index 07aae52..0ad7a79 100644 --- a/src/api/internal/BgzfStream_p.h +++ b/src/api/internal/BgzfStream_p.h @@ -46,8 +46,7 @@ class BgzfStream { void Close(void); // returns true if BgzfStream open for IO bool IsOpen(void) const; - // opens the BGZF file (mode is either "rb" for reading, or "wb" for writing) - void Open(const std::string& filename, const char* mode); + // opens the BGZF file void Open(const std::string& filename, const IBamIODevice::OpenMode mode); // reads BGZF data into a byte buffer size_t Read(char* data, const size_t dataLength); @@ -84,10 +83,7 @@ class BgzfStream { unsigned int m_blockOffset; uint64_t m_blockAddress; - bool m_isOpen; - bool m_isWriteOnly; bool m_isWriteCompressed; - IBamIODevice* m_device; struct RaiiWrapper { @@ -95,7 +91,6 @@ class BgzfStream { ~RaiiWrapper(void); char* UncompressedBlock; char* CompressedBlock; - FILE* Stream; }; RaiiWrapper Resources; }; diff --git a/src/api/internal/ILocalIODevice_p.cpp b/src/api/internal/ILocalIODevice_p.cpp index 9e9a2b3..279ab77 100644 --- a/src/api/internal/ILocalIODevice_p.cpp +++ b/src/api/internal/ILocalIODevice_p.cpp @@ -2,7 +2,7 @@ // ILocalIODevice_p.cpp (c) 2011 Derek Barnett // Marth Lab, Department of Biology, Boston College // --------------------------------------------------------------------------- -// Last modified: 8 September 2011 (DB) +// Last modified: 7 October 2011 (DB) // --------------------------------------------------------------------------- // Provides shared behavior for files & pipes // *************************************************************************** @@ -32,25 +32,25 @@ void ILocalIODevice::Close(void) { // flush & close FILE* fflush(m_stream); fclose(m_stream); + m_stream = 0; - // reset internals + // reset other device state m_mode = IBamIODevice::NotOpen; - m_stream = 0; } size_t ILocalIODevice::Read(char* data, const unsigned int numBytes) { - BT_ASSERT_X( m_stream, "ILocalIODevice::Read() - null stream" ); - BT_ASSERT_X( (m_mode == IBamIODevice::ReadOnly), "ILocalIODevice::Read() - device not in read-only mode"); + BT_ASSERT_X( m_stream, "ILocalIODevice::Read: trying to read from null stream" ); + BT_ASSERT_X( (m_mode == IBamIODevice::ReadOnly), "ILocalIODevice::Read: device not in read-only mode"); return fread(data, sizeof(char), numBytes, m_stream); } int64_t ILocalIODevice::Tell(void) const { - BT_ASSERT_X( m_stream, "ILocalIODevice::Tell() - null stream" ); + BT_ASSERT_X( m_stream, "ILocalIODevice::Tell: trying to get file position fromnull stream" ); return ftell64(m_stream); } size_t ILocalIODevice::Write(const char* data, const unsigned int numBytes) { - BT_ASSERT_X( m_stream, "ILocalIODevice::Write() - null stream" ); - BT_ASSERT_X( (m_mode == IBamIODevice::WriteOnly), "ILocalIODevice::Write() - device not in write-only mode" ); + BT_ASSERT_X( m_stream, "ILocalIODevice::Write: tryint to write to null stream" ); + BT_ASSERT_X( (m_mode == IBamIODevice::WriteOnly), "ILocalIODevice::Write: device not in write-only mode" ); return fwrite(data, sizeof(char), numBytes, m_stream); } diff --git a/src/shared/bamtools_global.h b/src/shared/bamtools_global.h index 1b2a8af..4165740 100644 --- a/src/shared/bamtools_global.h +++ b/src/shared/bamtools_global.h @@ -2,7 +2,7 @@ // bamtools_global.h (c) 2010 Derek Barnett // Marth Lab, Department of Biology, Boston College // --------------------------------------------------------------------------- -// Last modified: 3 March 2011 (DB) +// Last modified: 7 October 2011 (DB) // --------------------------------------------------------------------------- // Provides the basic definitions for exporting & importing library symbols. // Also provides some platform-specific rules for definitions. @@ -38,13 +38,13 @@ */ #ifndef BAMTOOLS_LFS #define BAMTOOLS_LFS - #ifdef WIN32 - #define ftell64(a) _ftelli64(a) - #define fseek64(a,b,c) _fseeki64(a,b,c) - #else - #define ftell64(a) ftello(a) - #define fseek64(a,b,c) fseeko(a,b,c) - #endif +# ifdef WIN32 +# define ftell64(a) _ftelli64(a) +# define fseek64(a,b,c) _fseeki64(a,b,c) +# else +# define ftell64(a) ftello(a) +# define fseek64(a,b,c) fseeko(a,b,c) +# endif #endif // BAMTOOLS_LFS /*! \def ftell64(a) @@ -61,25 +61,33 @@ */ #ifndef BAMTOOLS_TYPES #define BAMTOOLS_TYPES - #ifdef _MSC_VER - typedef char int8_t; - typedef unsigned char uint8_t; - typedef short int16_t; - typedef unsigned short uint16_t; - typedef int int32_t; - typedef unsigned int uint32_t; - typedef long long int64_t; - typedef unsigned long long uint64_t; - #else - #include - #endif +# ifdef _MSC_VER + typedef char int8_t; + typedef unsigned char uint8_t; + typedef short int16_t; + typedef unsigned short uint16_t; + typedef int int32_t; + typedef unsigned int uint32_t; + typedef long long int64_t; + typedef unsigned long long uint64_t; +# else +# include +# endif #endif // BAMTOOLS_TYPES -#include -#include +inline void bamtools_noop(void) { } + #ifndef BAMTOOLS_ASSERTS -#define BT_ASSERT_UNREACHABLE assert( false ) -#define BT_ASSERT_X( condition, message ) if (!( condition )) throw std::runtime_error( message ); +#define BAMTOOLS_ASSERTS +# include +# include +# ifdef NDEBUG +# define BT_ASSERT_UNREACHABLE bamtools_noop() +# define BT_ASSERT_X( condition, message ) bamtools_noop() +# else +# define BT_ASSERT_UNREACHABLE assert( false ) +# define BT_ASSERT_X( condition, message ) if (!( condition )) { throw std::runtime_error( message ); } +# endif #endif // BAMTOOLS_ASSERTS #endif // BAMTOOLS_GLOBAL_H diff --git a/src/utils/bamtools_utilities.h b/src/utils/bamtools_utilities.h index d9a3192..9c1f7c9 100644 --- a/src/utils/bamtools_utilities.h +++ b/src/utils/bamtools_utilities.h @@ -2,7 +2,7 @@ // bamtools_utilities.h (c) 2010 Derek Barnett, Erik Garrison // Marth Lab, Department of Biology, Boston College // --------------------------------------------------------------------------- -// Last modified: 8 September 2011 +// Last modified: 7 October 2011 // --------------------------------------------------------------------------- // Provides general utilities used by BamTools sub-tools. // *************************************************************************** -- 2.39.2