Skip to content

Commit 3653412

Browse files
lyskov-ailyskov-ai
authored
Refactor ozstream raw pointers to unique_ptr; modernize orstream copy ops (#667)
## Summary - Replace three raw owning pointers in `ozstream` (`char_buffer_p_`, `zip_stream_p_`, `mpi_stream_p_`) with `std::unique_ptr`, eliminating manual `delete`/`nullptr` pairs in `open()`, `open_append()`, `open_append_if_existed()`, `close()`, and the buffer helpers - Modernize the private-undefined copy constructor/assignment in `orstream` to explicit `= delete`, consistent with the approach used in `irstream`/`izstream` (PR #664) ## Test plan - [x] Full debug build passes (`python3 ./scons.py -j16 mode=debug`) - [ ] No behavior change expected — ownership semantics are identical; `close()` still runs `zflush_finalize()` and `mpi_ostream::close()/clear()` before releasing the pointers
1 parent bf9e7e8 commit 3653412

3 files changed

Lines changed: 26 additions & 40 deletions

File tree

‎source/src/utility/io/orstream.hh‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -58,19 +58,10 @@ protected: // Creation
5858
= default;
5959

6060

61-
private: // Creation
61+
public: // Creation (deleted)
6262

63-
64-
/// @brief Copy constructor: Undefined
65-
orstream( orstream const & );
66-
67-
68-
private: // Methods: assignment
69-
70-
71-
/// @brief Copy assignment: Undefined
72-
orstream &
73-
operator =( orstream const & );
63+
orstream( orstream const & ) = delete;
64+
orstream & operator =( orstream const & ) = delete;
7465

7566

7667
public: // Methods: conversion

‎source/src/utility/io/ozstream.cc‎

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ ozstream::open_append_if_existed( std::string const& filename_a, std::stringstre
4040
#ifdef USEMPI
4141
// std::cout << "MPI_Reroute " << (bMPI_reroute_stream_ ? " active " : " not-active ") << std::endl;
4242
if ( bMPI_reroute_stream_ ) { // this is switched via call to static function enable_MPI_reroute()
43-
mpi_stream_p_ = new mpi_stream::mpi_ostream( filename_a, mpi_FileBuf_master_rank_, preprinted_header, true );
44-
if ( ( !mpi_stream_p_ ) || ( !( *mpi_stream_p_ ) ) ) {
43+
mpi_stream_p_.reset( new mpi_stream::mpi_ostream( filename_a, mpi_FileBuf_master_rank_, preprinted_header, true ) );
44+
if ( !( *mpi_stream_p_ ) ) {
4545
compression_ = NONE;
4646
} else compression_ = UNCOMPRESSED;
4747
return;
@@ -94,8 +94,8 @@ ozstream::open(
9494
// std::cout << "MPI_Reroute " << (bMPI_reroute_stream_ ? " active " : " not-active ") << std::endl;
9595
if ( bMPI_reroute_stream_ ) {
9696
std::stringstream no_header;
97-
mpi_stream_p_ = new mpi_stream::mpi_ostream( filename_, mpi_FileBuf_master_rank_, no_header, open_mode & ios::app );
98-
if ( ( !mpi_stream_p_ ) || ( !( *mpi_stream_p_ ) ) ) {
97+
mpi_stream_p_.reset( new mpi_stream::mpi_ostream( filename_, mpi_FileBuf_master_rank_, no_header, open_mode & ios::app ) );
98+
if ( !( *mpi_stream_p_ ) ) {
9999
compression_ = NONE;
100100
} else {
101101
compression_ = UNCOMPRESSED;
@@ -139,12 +139,10 @@ ozstream::open(
139139

140140
// Attach zip_ostream to ofstream if gzip file
141141
if ( compression_ == GZIP ) {
142-
// zip_stream_p_ deleted by close() above so don't have to here
143-
zip_stream_p_ = new zip_ostream( of_stream_, true, static_cast< size_t >( Z_DEFAULT_COMPRESSION ), zlib_stream::DefaultStrategy, 15, 8, buffer_size_ );
144-
if ( ( !zip_stream_p_ ) || ( !( *zip_stream_p_ ) ) ||
145-
( !zip_stream_p_->is_gzip() ) ) { // zip_stream not in good state
146-
if ( zip_stream_p_ ) delete zip_stream_p_;
147-
zip_stream_p_ = nullptr;
142+
// zip_stream_p_ cleared by close() above so don't have to here
143+
zip_stream_p_.reset( new zip_ostream( of_stream_, true, static_cast< size_t >( Z_DEFAULT_COMPRESSION ), zlib_stream::DefaultStrategy, 15, 8, buffer_size_ ) );
144+
if ( !( *zip_stream_p_ ) || !zip_stream_p_->is_gzip() ) { // zip_stream not in good state
145+
zip_stream_p_.reset();
148146
of_stream_.close();
149147
// Set failbit so failure can be detected
150148
of_stream_.setstate( ios_base::failbit );
@@ -175,8 +173,8 @@ ozstream::open_append( std::string const & filename_a )
175173
// std::cout << "MPI_Reroute " << (bMPI_reroute_stream_ ? " active " : " not-active ") << std::endl;
176174
if ( bMPI_reroute_stream_ ) {
177175
std::stringstream no_header;
178-
mpi_stream_p_ = new mpi_stream::mpi_ostream( filename_, mpi_FileBuf_master_rank_, no_header, true );
179-
if ( ( !mpi_stream_p_ ) || ( !( *mpi_stream_p_ ) ) ) {
176+
mpi_stream_p_.reset( new mpi_stream::mpi_ostream( filename_, mpi_FileBuf_master_rank_, no_header, true ) );
177+
if ( !( *mpi_stream_p_ ) ) {
180178
compression_ = NONE;
181179
} else compression_ = UNCOMPRESSED;
182180
return;
@@ -208,11 +206,10 @@ ozstream::open_append( std::string const & filename_a )
208206

209207
// Attach zip_ostream to ofstream if gzip file
210208
if ( compression_ == GZIP ) {
211-
// zip_stream_p_ deleted by close() above so don't have to here
212-
zip_stream_p_ = new zip_ostream( of_stream_, true, static_cast< size_t >( Z_DEFAULT_COMPRESSION ), zlib_stream::DefaultStrategy, 15, 8, buffer_size_ );
213-
if ( ( !zip_stream_p_ ) || ( !( *zip_stream_p_ ) ) ||
214-
( !zip_stream_p_->is_gzip() ) ) { // zip_stream not in good state
215-
delete zip_stream_p_; zip_stream_p_ = nullptr;
209+
// zip_stream_p_ cleared by close() above so don't have to here
210+
zip_stream_p_.reset( new zip_ostream( of_stream_, true, static_cast< size_t >( Z_DEFAULT_COMPRESSION ), zlib_stream::DefaultStrategy, 15, 8, buffer_size_ ) );
211+
if ( !( *zip_stream_p_ ) || !zip_stream_p_->is_gzip() ) { // zip_stream not in good state
212+
zip_stream_p_.reset();
216213
of_stream_.close();
217214
// Set failbit so failure can be detected
218215
of_stream_.setstate( ios_base::failbit );

‎source/src/utility/io/ozstream.hh‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141

4242
// C++ headers
4343
#include <fstream>
44+
#include <memory>
4445

4546
namespace utility {
4647
namespace io {
@@ -438,15 +439,14 @@ public: // Methods: i/o
438439
#endif
439440
if ( zip_stream_p_ ) {
440441
zip_stream_p_->zflush_finalize();
441-
delete zip_stream_p_; zip_stream_p_ = nullptr;
442+
zip_stream_p_.reset();
442443
}
443444
of_stream_.close();
444445
of_stream_.clear();
445446
if ( mpi_stream_p_ ) {
446447
mpi_stream_p_->close();
447448
mpi_stream_p_->clear();
448-
delete mpi_stream_p_;
449-
mpi_stream_p_ = nullptr;
449+
mpi_stream_p_.reset();
450450
}
451451
compression_ = NONE;
452452
filename_.clear();
@@ -692,8 +692,8 @@ private: // buffer management
692692
allocate_assign_char_buffer()
693693
{
694694
if ( !char_buffer_p_ && !of_stream_.is_open() ) {
695-
char_buffer_p_ = new char[ buffer_size_ ];
696-
of_stream_.rdbuf()->pubsetbuf( char_buffer_p_, buffer_size_ );
695+
char_buffer_p_.reset( new char[ buffer_size_ ] );
696+
of_stream_.rdbuf()->pubsetbuf( char_buffer_p_.get(), buffer_size_ );
697697

698698
return true;
699699
}
@@ -710,8 +710,7 @@ private: // buffer management
710710
destroy_char_buffer()
711711
{
712712
if ( char_buffer_p_ && !of_stream_.is_open() ) {
713-
delete [] char_buffer_p_;
714-
char_buffer_p_ = nullptr;
713+
char_buffer_p_.reset();
715714

716715
return true;
717716
}
@@ -750,13 +749,12 @@ private: // Fields
750749
std::streamsize buffer_size_;
751750

752751
/// @brief character buffer pointer (owning)
753-
char * char_buffer_p_;
752+
std::unique_ptr<char[]> char_buffer_p_;
754753

755754
/// @brief Zip file stream pointer (owning)
756-
zlib_stream::zip_ostream *zip_stream_p_;
755+
std::unique_ptr<zlib_stream::zip_ostream> zip_stream_p_;
757756

758-
759-
mpi_stream::mpi_ostream *mpi_stream_p_;
757+
std::unique_ptr<mpi_stream::mpi_ostream> mpi_stream_p_;
760758

761759
static bool bMPI_reroute_stream_;
762760
static int mpi_FileBuf_master_rank_;

0 commit comments

Comments
 (0)