Cleaning up casts and parameter in posix file streams. Fixed up putc implementation for file streams to correctly copy the char.

Author: stgates

Release/include/cpprest/details/fileio.h

@@ -155,15 +155,6 @@ _ASYNCRTIMP bool __cdecl _close_fsb(_In_ concurrency::streams::details::_file_in
 /// <returns>0 if the read request is still outstanding, -1 if the request failed, otherwise the size of the data read into the buffer</returns>
 _ASYNCRTIMP size_t __cdecl _putn_fsb(_In_ concurrency::streams::details::_file_info *info, _In_ concurrency::streams::details::_filestream_callback *callback, const void *ptr, size_t count, size_t char_size);
 
-/// <summary>
-/// Write a single byte to the file stream.
-/// </summary>
-/// <param name="info">The file info record of the file</param>
-/// <param name="callback">A pointer to the callback interface to invoke when the write request is completed.</param>
-/// <param name="ptr">A pointer to a buffer where the data should be placed</param>
-/// <returns>0 if the read request is still outstanding, -1 if the request failed, otherwise the size of the data read into the buffer</returns>
-_ASYNCRTIMP size_t __cdecl _putc_fsb(_In_ concurrency::streams::details::_file_info *info, _In_ concurrency::streams::details::_filestream_callback *callback, int ch, size_t char_size);
-
 /// <summary>
 /// Read data from a file stream into a buffer
 /// </summary>

Release/include/cpprest/filestream.h

@@ -277,17 +277,24 @@ namespace details {
             /// <returns>A <c>task</c> that holds the value of the byte written. This is EOF if the write operation fails.</returns>
         virtual pplx::task<int_type> _putc(_CharType ch)
         {
-            auto result_tce = pplx::task_completion_event<int_type>();
-            auto callback = new _filestream_callback_putc(m_info, result_tce, ch);
-
-            size_t written = _putc_fsb(m_info, callback, ch, sizeof(_CharType));
+            auto result_tce = pplx::task_completion_event<size_t>();
+            auto callback = new _filestream_callback_write<size_t>(m_info, result_tce);
 
-            if ( written == sizeof(_CharType) )
+            // Potentially we should consider deprecating this API, it is TERRIBLY inefficient.
+            auto sharedCh = std::make_shared<_CharType>(ch);
+            size_t written = _putn_fsb(m_info, callback, sharedCh.get(), 1, sizeof(_CharType));
+            
+            
+            if (written == sizeof(_CharType))
             {
                 delete callback;
                 return pplx::task_from_result<int_type>(ch);
             }
-            return pplx::create_task(result_tce);
+            
+            return pplx::create_task(result_tce).then([sharedCh](size_t)
+            {
+                return static_cast<int_type>(*sharedCh);
+            });
         }
 
         /// <summary>
@@ -843,36 +850,6 @@ namespace details {
             pplx::task_completion_event<size_t> m_op;
         };
 
-        class _filestream_callback_putc : public details::_filestream_callback
-        {
-        public:
-            _filestream_callback_putc(_In_ _file_info *info, pplx::task_completion_event<int_type> op, _CharType ch) : m_info(info), m_op(op), m_ch(ch) { }
-
-            virtual void on_completed(size_t result)
-            {
-                if ( result == sizeof(_CharType) )
-                {
-                    m_op.set(m_ch);
-                }
-                else
-                {
-                    m_op.set(traits::eof());
-                }
-                delete this;
-            }
-
-            virtual void on_error(const std::exception_ptr & e)
-            {
-                m_op.set_exception(e);
-                delete this;
-            }
-
-        private:
-            _file_info *m_info;
-            pplx::task_completion_event<int_type> m_op;
-            int_type             m_ch;
-        };
-
         class _filestream_callback_bumpc : public details::_filestream_callback
         {
         public:

Release/src/streams/fileio_posix.cpp

@@ -99,20 +99,20 @@ class io_scheduler
 /// demand, it's referenced through a shared_ptr<T> and retrieved through function call.
 /// </summary>
 boost::mutex _g_lock;
-std::shared_ptr<io_scheduler> _g_scheduler;
+std::unique_ptr<io_scheduler> _g_scheduler;
 
 /// <summary>
 /// Get the I/O scheduler instance.
 /// </summary>
-std::shared_ptr<io_scheduler> get_scheduler()
+io_scheduler * get_scheduler()
 {
     boost::lock_guard<boost::mutex> lck(_g_lock);
     if ( !_g_scheduler )
     {
-        _g_scheduler = std::make_shared<io_scheduler>();
+        _g_scheduler = std::unique_ptr<io_scheduler>(new io_scheduler());
     }
 
-    return _g_scheduler;
+    return _g_scheduler.get();
 }
 
 /***
@@ -130,12 +130,11 @@ std::shared_ptr<io_scheduler> get_scheduler()
 /// </summary>
 struct _file_info_impl : _file_info
 {
-    _file_info_impl(std::shared_ptr<io_scheduler> sched, int handle, std::ios_base::openmode mode, bool buffer_reads) :
+    _file_info_impl(int handle, std::ios_base::openmode mode, bool buffer_reads) :
         _file_info(mode, 512),
         m_handle(handle),
         m_buffer_reads(buffer_reads),
-        m_outstanding_writes(0),
-        m_scheduler(sched)
+        m_outstanding_writes(0)
     {
     }
 
@@ -152,13 +151,6 @@ struct _file_info_impl : _file_info
     std::vector<_filestream_callback *> m_sync_waiters;
 
     std::atomic<long> m_outstanding_writes;
-
-    /// <summary>
-    /// A pointer to the scheduler instance used.
-    /// </summary>
-    /// <remarks>Even though we're using a singleton scheduler instance, it may not always be
-    /// the case, so it seems a good idea to keep a pointer here.</remarks>
-    std::shared_ptr<io_scheduler> m_scheduler;
 };
 
 }}}
@@ -174,8 +166,6 @@ bool _finish_create(int fh, _filestream_callback *callback, std::ios_base::openm
 {
     if ( fh != -1 )
     {
-        std::shared_ptr<io_scheduler> sched = get_scheduler();
-
         // Buffer reads internally if and only if we're just reading (not also writing) and
         // if the file is opened exclusively. If either is false, we're better off just
         // letting the OS do its buffering, even if it means that prompt reads won't
@@ -188,11 +178,11 @@ bool _finish_create(int fh, _filestream_callback *callback, std::ios_base::openm
             lseek(fh, 0, SEEK_END);
         }
 
-        auto info = new _file_info_impl(sched, fh, mode, buffer);
+        auto info = new _file_info_impl(fh, mode, buffer);
 
         if ( mode & std::ios_base::app || mode & std::ios_base::ate )
         {
-            info->m_wrpos = (size_t)-1; // Start at the end of the file.
+            info->m_wrpos = static_cast<size_t>(-1); // Start at the end of the file.
         }
 
         callback->on_opened(info);
@@ -285,7 +275,7 @@ bool _close_fsb_nolock(_file_info **info, Concurrency::streams::details::_filest
     if ( callback == nullptr ) return false;
     if ( info == nullptr || *info == nullptr ) return false;
 
-    _file_info_impl *fInfo = (_file_info_impl *)*info;
+    _file_info_impl *fInfo = static_cast<_file_info_impl *>(*info);
 
     if ( fInfo->m_handle == -1 ) return false;
 
@@ -344,7 +334,7 @@ bool _close_fsb(_file_info **info, Concurrency::streams::details::_filestream_ca
 /// <param name="ptr">A pointer to the data to write</param>
 /// <param name="count">The size (in bytes) of the data</param>
 /// <returns>0 if the write request is still outstanding, -1 if the request failed, otherwise the size of the data written</returns>
-size_t _write_file_async(Concurrency::streams::details::_file_info_impl *fInfo, Concurrency::streams::details::_filestream_callback *callback, const uint8_t *ptr, size_t count, size_t position)
+size_t _write_file_async(Concurrency::streams::details::_file_info_impl *fInfo, Concurrency::streams::details::_filestream_callback *callback, const void *ptr, size_t count, size_t position)
 {
     // async file writes are emulated using tasks
     auto sched = get_scheduler();
@@ -357,7 +347,7 @@ size_t _write_file_async(Concurrency::streams::details::_file_info_impl *fInfo,
         off_t abs_position;
         bool must_restore_pos;
         off_t orig_pos;
-        if( position == (size_t)-1 )
+        if( position == static_cast<size_t>(-1) )
         {
             orig_pos = lseek(fInfo->m_handle, 0, SEEK_CUR);
             abs_position = lseek(fInfo->m_handle, 0, SEEK_END);
@@ -399,7 +389,6 @@ size_t _write_file_async(Concurrency::streams::details::_file_info_impl *fInfo,
             }
         }
 
-        delete[] ptr; // free buffer
         sched->complete_io();
     });
 
@@ -441,7 +430,7 @@ template<typename Func>
 class _filestream_callback_fill_buffer : public _filestream_callback
 {
 public:
-    _filestream_callback_fill_buffer(_file_info *info, _filestream_callback *callback, Func func) : m_info(info), m_func(func), m_callback(callback) { }
+    _filestream_callback_fill_buffer(_file_info *info, _filestream_callback *callback, const Func &func) : m_info(info), m_func(func), m_callback(callback) { }
 
     virtual void on_completed(size_t result) override
     {
@@ -461,7 +450,7 @@ class _filestream_callback_fill_buffer : public _filestream_callback
 };
 
 template<typename Func>
-_filestream_callback_fill_buffer<Func> *create_callback(_file_info *info, _filestream_callback *callback, Func func)
+_filestream_callback_fill_buffer<Func> *create_callback(_file_info *info, _filestream_callback *callback, const Func &func)
 {
     return new _filestream_callback_fill_buffer<Func>(info, callback, func);
 }
@@ -474,7 +463,7 @@ size_t _fill_buffer_fsb(_file_info_impl *fInfo, _filestream_callback *callback,
     if ( fInfo->m_buffer == nullptr )
     {
         fInfo->m_bufsize = std::max(PageSize, byteCount);
-        fInfo->m_buffer = new char[(size_t)fInfo->m_bufsize];
+        fInfo->m_buffer = new char[static_cast<size_t>(fInfo->m_bufsize)];
         fInfo->m_bufoff = fInfo->m_rdpos;
 
         auto cb = create_callback(fInfo, callback,
@@ -500,7 +489,7 @@ size_t _fill_buffer_fsb(_file_info_impl *fInfo, _filestream_callback *callback,
 
         // Then, we allocate a new buffer.
 
-        char *newbuf = new char[(size_t)fInfo->m_bufsize];
+        char *newbuf = new char[static_cast<size_t>(fInfo->m_bufsize)];
 
         // Then, we copy the unread part to the new buffer and delete the old buffer
 
@@ -538,14 +527,13 @@ size_t _fill_buffer_fsb(_file_info_impl *fInfo, _filestream_callback *callback,
 /// <returns>0 if the read request is still outstanding, -1 if the request failed, otherwise the size of the data read into the buffer</returns>
 size_t _getn_fsb(Concurrency::streams::details::_file_info *info, Concurrency::streams::details::_filestream_callback *callback, void *ptr, size_t count, size_t charSize)
 {
-    if ( callback == nullptr ) return (size_t)-1;
-    if ( info == nullptr ) return (size_t)-1;
+    if ( callback == nullptr || info == nullptr ) return static_cast<size_t>(-1);
 
     _file_info_impl *fInfo = (_file_info_impl *)info;
 
     pplx::extensibility::scoped_recursive_lock_t lock(info->m_lock);
 
-    if ( fInfo->m_handle == -1 ) return (size_t)-1;
+    if ( fInfo->m_handle == -1 ) return static_cast<size_t>(-1);
 
     size_t byteCount = count * charSize;
 
@@ -590,18 +578,15 @@ size_t _getn_fsb(Concurrency::streams::details::_file_info *info, Concurrency::s
 /// <returns>0 if the read request is still outstanding, -1 if the request failed, otherwise the size of the data read into the buffer</returns>
 size_t _putn_fsb(Concurrency::streams::details::_file_info *info, Concurrency::streams::details::_filestream_callback *callback, const void *ptr, size_t count, size_t charSize)
 {
-    if ( callback == nullptr ) return (size_t)-1;
-    if ( info == nullptr ) return (size_t)-1;
+    if (callback == nullptr || info == nullptr) return static_cast<size_t>(-1);
 
-    _file_info_impl *fInfo = (_file_info_impl *)info;
+    _file_info_impl *fInfo = static_cast<_file_info_impl *>(info);
 
     pplx::extensibility::scoped_recursive_lock_t lock(fInfo->m_lock);
 
-    if ( fInfo->m_handle == -1 ) return (size_t)-1;
+    if ( fInfo->m_handle == -1 ) return static_cast<size_t>(-1);
 
     size_t byteSize = count * charSize;
-    uint8_t *buf = new uint8_t[byteSize];
-    memcpy(buf, ptr, byteSize);
 
     // To preserve the async write order, we have to move the write head before read.
     auto lastPos = fInfo->m_wrpos;
@@ -611,19 +596,7 @@ size_t _putn_fsb(Concurrency::streams::details::_file_info *info, Concurrency::s
         lastPos *= charSize;
     }
 
-    return _write_file_async(fInfo, callback, buf, byteSize, lastPos);
-}
-
-/// <summary>
-/// Write a single byte to the file stream.
-/// </summary>
-/// <param name="info">The file info record of the file</param>
-/// <param name="callback">A pointer to the callback interface to invoke when the write request is completed.</param>
-/// <param name="ptr">A pointer to a buffer where the data should be placed</param>
-/// <returns>0 if the read request is still outstanding, -1 if the request failed, otherwise the size of the data read into the buffer</returns>
-size_t _putc_fsb(Concurrency::streams::details::_file_info *info, Concurrency::streams::details::_filestream_callback *callback, int ch, size_t charSize)
-{
-    return _putn_fsb(info, callback, &ch, 1, charSize);
+    return _write_file_async(fInfo, callback, ptr, byteSize, lastPos);
 }
 
 /// <summary>
@@ -637,7 +610,7 @@ bool _sync_fsb(Concurrency::streams::details::_file_info *info, Concurrency::str
     if ( callback == nullptr ) return false;
     if ( info == nullptr ) return false;
 
-    _file_info_impl *fInfo = (_file_info_impl *)info;
+    _file_info_impl *fInfo = static_cast<_file_info_impl *>(info);
 
     pplx::extensibility::scoped_recursive_lock_t lock(fInfo->m_lock);
 
@@ -659,13 +632,13 @@ bool _sync_fsb(Concurrency::streams::details::_file_info *info, Concurrency::str
 /// <returns>New file position or -1 if error</returns>
 size_t _seekrdtoend_fsb(Concurrency::streams::details::_file_info *info, int64_t offset, size_t char_size)
 {
-    if ( info == nullptr ) return (size_t)-1;
+    if ( info == nullptr ) return static_cast<size_t>(-1);
 
-    _file_info_impl *fInfo = (_file_info_impl *)info;
+    _file_info_impl *fInfo = static_cast<_file_info_impl *>(info);
 
     pplx::extensibility::scoped_recursive_lock_t lock(info->m_lock);
 
-    if ( fInfo->m_handle == -1 ) return (size_t)-1;
+    if ( fInfo->m_handle == -1 ) return static_cast<size_t>(-1);
 
     if ( fInfo->m_buffer != nullptr )
     {
@@ -676,21 +649,21 @@ size_t _seekrdtoend_fsb(Concurrency::streams::details::_file_info *info, int64_t
 
     auto newpos = lseek(fInfo->m_handle, static_cast<off_t>(offset * char_size), SEEK_END);
 
-    if ( newpos == -1 ) return (size_t)-1;
+    if ( newpos == -1 ) return static_cast<size_t>(-1);
 
     fInfo->m_rdpos = newpos / char_size;
     return fInfo->m_rdpos;
 }
 
 utility::size64_t _get_size(_In_ concurrency::streams::details::_file_info *info, size_t char_size)
 {
-    if ( info == nullptr ) return (size_t)-1;
+    if ( info == nullptr ) return static_cast<size_t>(-1);
 
-    _file_info_impl *fInfo = (_file_info_impl *)info;
+    _file_info_impl *fInfo = static_cast<_file_info_impl *>(info);
 
     pplx::extensibility::scoped_recursive_lock_t lock(info->m_lock);
 
-    if ( fInfo->m_handle == -1 ) return (size_t)-1;
+    if ( fInfo->m_handle == -1 ) return static_cast<size_t>(-1);
 
     if ( fInfo->m_buffer != nullptr )
     {
@@ -720,13 +693,13 @@ utility::size64_t _get_size(_In_ concurrency::streams::details::_file_info *info
 /// <returns>New file position or -1 if error</returns>
 size_t _seekrdpos_fsb(Concurrency::streams::details::_file_info *info, size_t pos, size_t)
 {
-    if ( info == nullptr ) return (size_t)-1;
+    if ( info == nullptr ) return static_cast<size_t>(-1);
 
-    _file_info_impl *fInfo = (_file_info_impl *)info;
+    _file_info_impl *fInfo = static_cast<_file_info_impl *>(info);
 
     pplx::extensibility::scoped_recursive_lock_t lock(info->m_lock);
 
-    if ( fInfo->m_handle == -1 ) return (size_t)-1;
+    if ( fInfo->m_handle == -1 ) return static_cast<size_t>(-1);
 
     if ( pos < fInfo->m_bufoff || pos > (fInfo->m_bufoff+fInfo->m_buffill) )
     {
@@ -747,13 +720,13 @@ size_t _seekrdpos_fsb(Concurrency::streams::details::_file_info *info, size_t po
 /// <returns>New file position or -1 if error</returns>
 size_t _seekwrpos_fsb(Concurrency::streams::details::_file_info *info, size_t pos, size_t)
 {
-    if ( info == nullptr ) return (size_t)-1;
+    if ( info == nullptr ) return static_cast<size_t>(-1);
 
-    _file_info_impl *fInfo = (_file_info_impl *)info;
+    _file_info_impl *fInfo = static_cast<_file_info_impl *>(info);
 
     pplx::extensibility::scoped_recursive_lock_t lock(info->m_lock);
 
-    if ( fInfo->m_handle == -1 ) return (size_t)-1;
+    if ( fInfo->m_handle == -1 ) return static_cast<size_t>(-1);
 
     fInfo->m_wrpos = pos;
     return fInfo->m_wrpos;

Release/src/streams/fileio_win32.cpp

@@ -809,18 +809,6 @@ size_t __cdecl _putn_fsb(_In_ streams::details::_file_info *info, _In_ streams::
     return _write_file_async(fInfo, callback, ptr, count*char_size, lastPos);
 }
 
-/// <summary>
-/// Write a single byte to the file stream.
-/// </summary>
-/// <param name="info">The file info record of the file</param>
-/// <param name="callback">A pointer to the callback interface to invoke when the write request is completed.</param>
-/// <param name="ptr">A pointer to a buffer where the data should be placed</param>
-/// <returns>0 if the read request is still outstanding, -1 if the request failed, otherwise the size of the data read into the buffer</returns>
-size_t __cdecl _putc_fsb(_In_ streams::details::_file_info *info, _In_ streams::details::_filestream_callback *callback, int ch, size_t char_size)
-{
-    return _putn_fsb(info, callback, &ch, 1, char_size);
-}
-
 /// <summary>
 /// Flush all buffered data to the underlying file.
 /// </summary>