From 99218bb2ce16f3123a5f352a7f6c5490f74d538d Mon Sep 17 00:00:00 2001 From: Chris Jones Date: Mon, 27 Sep 2021 16:16:01 -0500 Subject: [PATCH] File now contains an IOChannel Changing inheritance to containment avoids diamond inheritance with Storage. Removed some unused methods. --- Utilities/StorageFactory/interface/File.h | 9 ++-- .../StorageFactory/interface/IOChannel.h | 18 +++----- Utilities/StorageFactory/interface/Storage.h | 2 +- Utilities/StorageFactory/src/File.cc | 45 ++++++++----------- Utilities/StorageFactory/src/IOChannel.cc | 20 +-------- Utilities/StorageFactory/src/UnixFile.cc | 12 ++--- Utilities/StorageFactory/src/UnixIOChannel.cc | 10 ----- 7 files changed, 39 insertions(+), 77 deletions(-) diff --git a/Utilities/StorageFactory/interface/File.h b/Utilities/StorageFactory/interface/File.h index bbe7925789c10..f38ce120379b7 100644 --- a/Utilities/StorageFactory/interface/File.h +++ b/Utilities/StorageFactory/interface/File.h @@ -10,7 +10,7 @@ namespace edm::storage { /** Basic file-related functions. Nicked from SEAL. */ - class File : public IOChannel, public Storage { + class File : public Storage { public: File(void); File(IOFD fd, bool autoclose = true); @@ -32,6 +32,8 @@ namespace edm::storage { using Storage::write; using Storage::writev; + IOFD fd() const { return m_channel.fd(); } + bool prefetch(const IOPosBuffer *what, IOSize n) override; IOSize read(void *into, IOSize n) override; IOSize read(void *into, IOSize n, IOOffset pos) override; @@ -46,8 +48,8 @@ namespace edm::storage { void resize(IOOffset size) override; - void flush(void) override; - void close(void) override; + void flush() override; + void close() override; virtual void abort(void); virtual void setAutoClose(bool closeit); @@ -63,6 +65,7 @@ namespace edm::storage { static void sysopen(const char *name, int flags, int perms, IOFD &newfd, unsigned &newflags); static bool sysclose(IOFD fd, int *error = nullptr); + IOChannel m_channel; unsigned m_flags; }; } // namespace edm::storage diff --git a/Utilities/StorageFactory/interface/IOChannel.h b/Utilities/StorageFactory/interface/IOChannel.h index 5d8877b9e1410..016b5a2b45061 100644 --- a/Utilities/StorageFactory/interface/IOChannel.h +++ b/Utilities/StorageFactory/interface/IOChannel.h @@ -8,10 +8,10 @@ namespace edm::storage { /** Base class for stream-oriented I/O sources and targets, based on the operating system file descriptor. */ - class IOChannel : public virtual IOInput, public virtual IOOutput { + class IOChannel : public IOInput, public IOOutput { public: IOChannel(IOFD fd = EDM_IOFD_INVALID); - ~IOChannel(void) override; + ~IOChannel() override; // implicit copy constructor // implicit assignment operator @@ -24,17 +24,11 @@ namespace edm::storage { IOSize write(const void *from, IOSize n) override; IOSize writev(const IOBuffer *from, IOSize buffers) override; - virtual IOFD fd(void) const; - virtual void fd(IOFD value); // FIXME: dangerous? + IOFD fd() const; + void fd(IOFD value); // FIXME: dangerous? - virtual void close(void); - - virtual void setBlocking(bool value); - virtual bool isBlocking(void) const; - - protected: - // System implementation - bool sysclose(IOFD fd, int *error = nullptr); + void setBlocking(bool value); + bool isBlocking() const; private: IOFD m_fd; /*< System file descriptor. */ diff --git a/Utilities/StorageFactory/interface/Storage.h b/Utilities/StorageFactory/interface/Storage.h index b0e6c748b84f6..e7ec50235488a 100644 --- a/Utilities/StorageFactory/interface/Storage.h +++ b/Utilities/StorageFactory/interface/Storage.h @@ -18,7 +18,7 @@ namespace edm::storage { constexpr int PREFETCH_PROBE_LENGTH = 4096; - class Storage : public virtual IOInput, public virtual IOOutput { + class Storage : public IOInput, public IOOutput { public: enum Relative { SET, CURRENT, END }; diff --git a/Utilities/StorageFactory/src/File.cc b/Utilities/StorageFactory/src/File.cc index 64f49d5e676f8..9d276b15ea8a7 100644 --- a/Utilities/StorageFactory/src/File.cc +++ b/Utilities/StorageFactory/src/File.cc @@ -55,24 +55,15 @@ using namespace edm::storage::IOFlags; ////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////// /** Create a new file object without a file attached to it. */ -File::File(void) { - fd(EDM_IOFD_INVALID); - m_flags = 0; -} +File::File() : m_channel(EDM_IOFD_INVALID) { m_flags = 0; } /** Create a new file object from a file descriptor. The descriptor will be closed automatically when the file object is destructed if @a autoclose is @c true (the default). */ -File::File(IOFD fd, bool autoclose /* = true */) { - this->fd(fd); - m_flags = autoclose ? InternalAutoClose : 0; -} +File::File(IOFD fd, bool autoclose /* = true */) : m_channel(fd) { m_flags = autoclose ? InternalAutoClose : 0; } /** Internal function for copying file objects to retain the state flags. */ -File::File(IOFD fd, unsigned flags) { - this->fd(fd); - m_flags = flags; -} +File::File(IOFD fd, unsigned flags) : m_channel(fd) { m_flags = flags; } /** Create a new file object by calling #open() with the given arguments. */ File::File(const char *name, int flags /*= OpenRead*/, int perms /*= 0666*/) { open(name, flags, perms); } @@ -109,17 +100,17 @@ void File::setAutoClose(bool autoclose) { will not close its file descriptor on destruction, the original object (@c this) will. */ File *File::duplicate(bool copy) const { - File *dup = new File(fd(), copy ? m_flags : 0); + File *dup = new File(m_channel.fd(), copy ? m_flags : 0); return copy ? this->duplicate(dup) : dup; } /** Internal implementation of #duplicate() to actually duplicate the file handle into @a child. */ File *File::duplicate(File *child) const { - IOFD fd = this->fd(); + IOFD fd = m_channel.fd(); assert(fd != EDM_IOFD_INVALID); assert(child); - child->fd(sysduplicate(fd)); + child->m_channel.fd(sysduplicate(fd)); child->m_flags = m_flags; return child; } @@ -164,7 +155,7 @@ void File::open(const char *name, int flags /*= OpenRead*/, int perms /*= 0666*/ assert(flags & (OpenRead | OpenWrite)); // If I am already open, close the old file first. - if (fd() != EDM_IOFD_INVALID && (m_flags & InternalAutoClose)) + if (m_channel.fd() != EDM_IOFD_INVALID && (m_flags & InternalAutoClose)) close(); IOFD newfd = EDM_IOFD_INVALID; @@ -172,19 +163,19 @@ void File::open(const char *name, int flags /*= OpenRead*/, int perms /*= 0666*/ sysopen(name, flags, perms, newfd, newflags); - fd(newfd); + m_channel.fd(newfd); m_flags = newflags; } void File::attach(IOFD fd) { - this->fd(fd); + m_channel.fd(fd); m_flags = 0; } ////////////////////////////////////////////////////////////////////// /** Prefetch data for the file. */ bool File::prefetch(const IOPosBuffer *what, IOSize n) { - IOFD fd = this->fd(); + IOFD fd = m_channel.fd(); for (IOSize i = 0; i < n; ++i) { #if F_RDADVISE radvisory info; @@ -201,10 +192,10 @@ bool File::prefetch(const IOPosBuffer *what, IOSize n) { } /** Read from the file. */ -IOSize File::read(void *into, IOSize n) { return IOChannel::read(into, n); } +IOSize File::read(void *into, IOSize n) { return m_channel.read(into, n); } /** Read from the file. */ -IOSize File::readv(IOBuffer *into, IOSize length) { return IOChannel::readv(into, length); } +IOSize File::readv(IOBuffer *into, IOSize length) { return m_channel.readv(into, length); } /** Write to the file. */ IOSize File::write(const void *from, IOSize n) { @@ -213,7 +204,7 @@ IOSize File::write(const void *from, IOSize n) { if (m_flags & OpenAppend) position(0, END); - IOSize s = IOChannel::write(from, n); + IOSize s = m_channel.write(from, n); if (m_flags & OpenUnbuffered) // FIXME: Exception handling? @@ -229,7 +220,7 @@ IOSize File::writev(const IOBuffer *from, IOSize length) { if (m_flags & OpenAppend) position(0, END); - IOSize s = IOChannel::writev(from, length); + IOSize s = m_channel.writev(from, length); if (m_flags & OpenUnbuffered) // FIXME: Exception handling? @@ -240,7 +231,7 @@ IOSize File::writev(const IOBuffer *from, IOSize length) { /** Close the file. */ void File::close() { - IOFD fd = this->fd(); + IOFD fd = m_channel.fd(); assert(fd != EDM_IOFD_INVALID); int error; @@ -248,15 +239,15 @@ void File::close() { throwStorageError("FileCloseError", "Calling File::close()", "sysclose", error); m_flags &= ~InternalAutoClose; - this->fd(EDM_IOFD_INVALID); + m_channel.fd(EDM_IOFD_INVALID); } /** Close the file and ignore all errors. */ void File::abort() { - IOFD fd = this->fd(); + IOFD fd = m_channel.fd(); if (fd != EDM_IOFD_INVALID) { sysclose(fd); m_flags &= ~InternalAutoClose; - this->fd(EDM_IOFD_INVALID); + m_channel.fd(EDM_IOFD_INVALID); } } diff --git a/Utilities/StorageFactory/src/IOChannel.cc b/Utilities/StorageFactory/src/IOChannel.cc index 3288ba155659b..a180fae0a631f 100644 --- a/Utilities/StorageFactory/src/IOChannel.cc +++ b/Utilities/StorageFactory/src/IOChannel.cc @@ -61,13 +61,13 @@ using namespace edm::storage; IOChannel::IOChannel(IOFD fd /* = EDM_IOFD_INVALID */) : m_fd(fd) {} -IOChannel::~IOChannel(void) {} +IOChannel::~IOChannel() {} ////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////// /** Get the system file descriptor of the channel. */ -IOFD IOChannel::fd(void) const { return m_fd; } +IOFD IOChannel::fd() const { return m_fd; } /** Set the system file descriptor of the channel. (FIXME: This is dangerous. How to deal with WIN32 flags and state object?) */ @@ -76,19 +76,3 @@ void IOChannel::fd(IOFD value) { // FIXME: reset state? m_fd = value; } - -////////////////////////////////////////////////////////////////////// -////////////////////////////////////////////////////////////////////// -////////////////////////////////////////////////////////////////////// -/** Close the channel. By default closes the underlying operating - system file descriptor. */ -void IOChannel::close(void) { - if (fd() == EDM_IOFD_INVALID) - return; - - int error = 0; - if (!sysclose(fd(), &error)) - throwStorageError("FileCloseError", "Calling IOChannel::close()", "sysclose()", error); - - fd(EDM_IOFD_INVALID); -} diff --git a/Utilities/StorageFactory/src/UnixFile.cc b/Utilities/StorageFactory/src/UnixFile.cc index 4e734827be298..3183c29f24644 100644 --- a/Utilities/StorageFactory/src/UnixFile.cc +++ b/Utilities/StorageFactory/src/UnixFile.cc @@ -62,7 +62,7 @@ IOSize File::read(void *into, IOSize n, IOOffset pos) { ssize_t s; do - s = ::pread(fd(), into, n, pos); + s = ::pread(m_channel.fd(), into, n, pos); while (s == -1 && errno == EINTR); if (s == -1) @@ -76,7 +76,7 @@ IOSize File::write(const void *from, IOSize n, IOOffset pos) { ssize_t s; do - s = ::pwrite(fd(), from, n, pos); + s = ::pwrite(m_channel.fd(), from, n, pos); while (s == -1 && errno == EINTR); if (s == -1) @@ -90,7 +90,7 @@ IOSize File::write(const void *from, IOSize n, IOOffset pos) { } IOOffset File::size(void) const { - IOFD fd = this->fd(); + IOFD fd = m_channel.fd(); assert(fd != EDM_IOFD_INVALID); struct stat info; @@ -101,7 +101,7 @@ IOOffset File::size(void) const { } IOOffset File::position(IOOffset offset, Relative whence /* = SET */) { - IOFD fd = this->fd(); + IOFD fd = m_channel.fd(); assert(fd != EDM_IOFD_INVALID); assert(whence == CURRENT || whence == SET || whence == END); @@ -114,7 +114,7 @@ IOOffset File::position(IOOffset offset, Relative whence /* = SET */) { } void File::resize(IOOffset size) { - IOFD fd = this->fd(); + IOFD fd = m_channel.fd(); assert(fd != EDM_IOFD_INVALID); if (ftruncate(fd, size) == -1) @@ -122,7 +122,7 @@ void File::resize(IOOffset size) { } void File::flush(void) { - IOFD fd = this->fd(); + IOFD fd = m_channel.fd(); assert(fd != EDM_IOFD_INVALID); #if _POSIX_SYNCHRONIZED_IO > 0 diff --git a/Utilities/StorageFactory/src/UnixIOChannel.cc b/Utilities/StorageFactory/src/UnixIOChannel.cc index ef1aa8799a5e5..868d9a3ae1f61 100644 --- a/Utilities/StorageFactory/src/UnixIOChannel.cc +++ b/Utilities/StorageFactory/src/UnixIOChannel.cc @@ -122,13 +122,3 @@ bool IOChannel::isBlocking(void) const { return true; #endif // O_NONBLOCK } - -////////////////////////////////////////////////////////////////////// -////////////////////////////////////////////////////////////////////// -////////////////////////////////////////////////////////////////////// -bool IOChannel::sysclose(IOFD fd, int *error /* = 0 */) { - int ret = ::close(fd); - if (error) - *error = errno; - return ret != -1; -}