[vlc-devel] [PATCH] nothrow new in cases where it maches intent

Hugo Beauzée-Luyssen hugo at beauzee.fr
Wed Feb 17 13:06:20 CET 2016


On 02/17/2016 12:16 PM, Filip Roséen wrote:
> I wrote a hackish script to locate instances where new can throw
> but where the original author has assumed that it will return
> nullptr when there is a memory allocation problem.
>
> In short, cases such as `ptr = new T; if (ptr) ...` has now
> been changed to `ptr = new (std::nothrow) T; if (ptr) ...`.
>
> Since a throwing `new` will always yield a non-nullptr pointer,
> code that follows similar patterns to the previous example are
> therefor redundant.
>
> Example (from modules/access/dshow/filter.cpp):
>
>      *ppEnum = new CaptureEnumMediaTypes( p_input, p_pin, this );
>
>      if( *ppEnum == NULL )
>        return E_OUTOFMEMORY; // unreachable, new will never return NULL
>
> Fixed:
>
>      *ppEnum = new (std::nothrow) CaptureEnumMediaTypes( p_input, p_pin, this );
>
>      if( *ppEnum == NULL )
>        return E_OUTOFMEMORY;
> ---
>   modules/access/dshow/filter.cpp               | 11 +++++++----
>   modules/access/live555.cpp                    |  3 ++-
>   modules/demux/adaptative/mp4/AtomsReader.cpp  |  3 ++-
>   modules/demux/mkv/matroska_segment.cpp        |  8 +++++---
>   modules/demux/mkv/mkv.cpp                     |  4 +++-
>   modules/demux/mkv/virtual_segment.cpp         |  3 ++-
>   modules/demux/sid.cpp                         |  8 +++++---
>   modules/gui/qt4/dialogs/fingerprintdialog.cpp |  3 ++-
>   modules/gui/skins2/commands/async_queue.cpp   |  3 ++-
>   modules/gui/skins2/src/art_manager.cpp        |  4 +++-
>   modules/gui/skins2/src/var_manager.cpp        |  4 ++--
>   modules/gui/skins2/x11/x11_window.cpp         |  4 +++-
>   12 files changed, 38 insertions(+), 20 deletions(-)
>
> diff --git a/modules/access/dshow/filter.cpp b/modules/access/dshow/filter.cpp
> index 073a2f3..a6b0995 100644
> --- a/modules/access/dshow/filter.cpp
> +++ b/modules/access/dshow/filter.cpp
> @@ -42,6 +42,9 @@
>   #include "vlc_dshow.h"
>
>   #include <initguid.h>
> +
> +#include <new>
> +
>   DEFINE_GUID(MEDIASUBTYPE_HDYC ,0x43594448 /* CYDH */ , 0x0000, 0x0010, 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71);
>   DEFINE_GUID(MEDIASUBTYPE_DIVX ,0x58564944 /* XVID */ , 0x0000, 0x0010, 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71);
>
> @@ -561,7 +564,7 @@ STDMETHODIMP CapturePin::EnumMediaTypes( IEnumMediaTypes **ppEnum )
>       msg_Dbg( p_input, "CapturePin::EnumMediaTypes" );
>   #endif
>
> -    *ppEnum = new CaptureEnumMediaTypes( p_input, this, NULL );
> +    *ppEnum = new (std::nothrow) CaptureEnumMediaTypes( p_input, this, NULL );
>
>       if( *ppEnum == NULL ) return E_OUTOFMEMORY;
>
> @@ -849,7 +852,7 @@ STDMETHODIMP CaptureFilter::EnumPins( IEnumPins ** ppEnum )
>   #endif
>
>       /* Create a new ref counted enumerator */
> -    *ppEnum = new CaptureEnumPins( p_input, this, NULL );
> +    *ppEnum = new (std::nothrow) CaptureEnumPins( p_input, this, NULL );
>       return *ppEnum == NULL ? E_OUTOFMEMORY : NOERROR;
>   };
>   STDMETHODIMP CaptureFilter::FindPin( LPCWSTR, IPin ** )
> @@ -1021,7 +1024,7 @@ STDMETHODIMP CaptureEnumPins::Clone( IEnumPins **ppEnum )
>       msg_Dbg( p_input, "CaptureEnumPins::Clone" );
>   #endif
>
> -    *ppEnum = new CaptureEnumPins( p_input, p_filter, this );
> +    *ppEnum = new (std::nothrow) CaptureEnumPins( p_input, p_filter, this );
>       if( *ppEnum == NULL ) return E_OUTOFMEMORY;
>
>       return NOERROR;
> @@ -1181,7 +1184,7 @@ STDMETHODIMP CaptureEnumMediaTypes::Clone( IEnumMediaTypes **ppEnum )
>       msg_Dbg( p_input, "CaptureEnumMediaTypes::Clone" );
>   #endif
>
> -    *ppEnum = new CaptureEnumMediaTypes( p_input, p_pin, this );
> +    *ppEnum = new (std::nothrow) CaptureEnumMediaTypes( p_input, p_pin, this );
>       if( *ppEnum == NULL ) return E_OUTOFMEMORY;
>
>       return NOERROR;
> diff --git a/modules/access/live555.cpp b/modules/access/live555.cpp
> index 86f0187..56ecc35 100644
> --- a/modules/access/live555.cpp
> +++ b/modules/access/live555.cpp
> @@ -47,6 +47,7 @@
>   #include <limits.h>
>   #include <assert.h>
>
> +#include <new>
>
>   #if defined( _WIN32 )
>   #   include <winsock2.h>
> @@ -608,7 +609,7 @@ createnew:
>       if( var_CreateGetBool( p_demux, "rtsp-http" ) )
>           i_http_port = var_InheritInteger( p_demux, "rtsp-http-port" );
>
> -    p_sys->rtsp = new RTSPClientVlc( *p_sys->env, psz_url,
> +    p_sys->rtsp = new (std::nothrow) RTSPClientVlc( *p_sys->env, psz_url,
>                                        var_InheritInteger( p_demux, "verbose" ) > 1 ? 1 : 0,
>                                        "LibVLC/" VERSION, i_http_port, p_sys );
>       if( !p_sys->rtsp )
> diff --git a/modules/demux/adaptative/mp4/AtomsReader.cpp b/modules/demux/adaptative/mp4/AtomsReader.cpp
> index e2f357f..fe84e16 100644
> --- a/modules/demux/adaptative/mp4/AtomsReader.cpp
> +++ b/modules/demux/adaptative/mp4/AtomsReader.cpp
> @@ -18,6 +18,7 @@
>    * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
>    *****************************************************************************/
>   #include "AtomsReader.hpp"
> +#include <new>
>
>   using namespace adaptative::mp4;
>
> @@ -52,7 +53,7 @@ bool AtomsReader::parseBlock(block_t *p_block)
>       stream_t *stream = stream_MemoryNew( object, p_block->p_buffer, p_block->i_buffer, true);
>       if (stream)
>       {
> -        rootbox = new MP4_Box_t;
> +        rootbox = new (std::nothrow) MP4_Box_t;
>           if(!rootbox)
>           {
>               stream_Delete(stream);
> diff --git a/modules/demux/mkv/matroska_segment.cpp b/modules/demux/mkv/matroska_segment.cpp
> index 16231f6..77b569e 100644
> --- a/modules/demux/mkv/matroska_segment.cpp
> +++ b/modules/demux/mkv/matroska_segment.cpp
> @@ -28,6 +28,8 @@
>   #include "util.hpp"
>   #include "Ebml_parser.hpp"
>
> +#include <new>
> +
>   matroska_segment_c::matroska_segment_c( demux_sys_t & demuxer, EbmlStream & estream )
>       :segment(NULL)
>       ,es(estream)
> @@ -280,7 +282,7 @@ SimpleTag * matroska_segment_c::ParseSimpleTags( KaxTagSimple *tag, int target_t
>       EbmlElement *el;
>       EbmlParser *ep = new EbmlParser( &es, tag, &sys.demuxer,
>                                        var_InheritBool( &sys.demuxer, "mkv-use-dummy" ) );
> -    SimpleTag * p_simple = new SimpleTag;
> +    SimpleTag * p_simple = new (std::nothrow) SimpleTag;
>       size_t max_size = tag->GetSize();
>       size_t size = 0;
>
> @@ -392,7 +394,7 @@ void matroska_segment_c::LoadTags( KaxTags *tags )
>       {
>           if( MKV_IS_ID( el, KaxTag ) )
>           {
> -            Tag * p_tag = new Tag;
> +            Tag * p_tag = new (std::nothrow) Tag;
>               if(!p_tag)
>               {
>                   msg_Err( &sys.demuxer,"Couldn't allocate memory for tag... ignoring it");
> @@ -993,7 +995,7 @@ void matroska_segment_c::Seek( mtime_t i_mk_date, mtime_t i_mk_time_offset, int6
>               }
>               if( tracks[i_track]->fmt.i_cat == i_cat )
>               {
> -                spoint * seekpoint = new spoint(i_track, i_mk_seek_time, i_seek_position, i_seek_position);
> +                spoint * seekpoint = new (std::nothrow) spoint(i_track, i_mk_seek_time, i_seek_position, i_seek_position);
>                   if( unlikely( !seekpoint ) )
>                   {
>                       for( spoint * sp = p_first; sp; )
> diff --git a/modules/demux/mkv/mkv.cpp b/modules/demux/mkv/mkv.cpp
> index 5c51d9a..b8927a4 100644
> --- a/modules/demux/mkv/mkv.cpp
> +++ b/modules/demux/mkv/mkv.cpp
> @@ -33,6 +33,8 @@
>
>   #include "stream_io_callback.hpp"
>
> +#include <new>
> +
>   extern "C" {
>   #include "../../modules/codec/dts_header.h"
>   }
> @@ -111,7 +113,7 @@ static int Open( vlc_object_t * p_this )
>       p_demux->p_sys      = p_sys = new demux_sys_t( *p_demux );
>
>       p_io_callback = new vlc_stream_io_callback( p_demux->s, false );
> -    p_io_stream = new EbmlStream( *p_io_callback );
> +    p_io_stream = new (std::nothrow) EbmlStream( *p_io_callback );
>
>       if( p_io_stream == NULL )
>       {
> diff --git a/modules/demux/mkv/virtual_segment.cpp b/modules/demux/mkv/virtual_segment.cpp
> index bafd743..a66003c 100644
> --- a/modules/demux/mkv/virtual_segment.cpp
> +++ b/modules/demux/mkv/virtual_segment.cpp
> @@ -23,6 +23,7 @@
>    * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
>    *****************************************************************************/
>   #include <vector>
> +#include <new>
>
>   #include "demux.hpp"
>
> @@ -67,7 +68,7 @@ virtual_chapter_c * virtual_chapter_c::CreateVirtualChapter( chapter_item_c * p_
>       if ( !p_segment->b_preloaded )
>           p_segment->Preload();
>
> -    virtual_chapter_c * p_vchap = new virtual_chapter_c( p_segment, p_chap, start, stop );
> +    virtual_chapter_c * p_vchap = new (std::nothrow) virtual_chapter_c( p_segment, p_chap, start, stop );
>
>       if( !p_vchap )
>           return NULL;
> diff --git a/modules/demux/sid.cpp b/modules/demux/sid.cpp
> index d2a71a3..b095d71 100644
> --- a/modules/demux/sid.cpp
> +++ b/modules/demux/sid.cpp
> @@ -43,6 +43,8 @@
>   #include <sidplay/sidplay2.h>
>   #include <sidplay/builders/resid.h>
>
> +#include <new>
> +
>   static int  Open (vlc_object_t *);
>   static void Close (vlc_object_t *);
>
> @@ -104,7 +106,7 @@ static int Open (vlc_object_t *obj)
>           goto error;
>       }
>
> -    tune = new SidTune(0);
> +    tune = new (std::nothrow) SidTune(0);
>       if (unlikely (tune==NULL)) {
>           free (data);
>           goto error;
> @@ -115,7 +117,7 @@ static int Open (vlc_object_t *obj)
>       if (!result)
>           goto error;
>
> -    player = new sidplay2();
> +    player = new (std::nothrow) sidplay2();
>       if (unlikely(player==NULL))
>           goto error;
>
> @@ -131,7 +133,7 @@ static int Open (vlc_object_t *obj)
>       sys->info = player->info();
>       sys->config = player->config();
>
> -    builder = new ReSIDBuilder ("ReSID");
> +    builder = new (std::nothrow) ReSIDBuilder ("ReSID");
>       if (unlikely(builder==NULL))
>           goto error;
>
> diff --git a/modules/gui/qt4/dialogs/fingerprintdialog.cpp b/modules/gui/qt4/dialogs/fingerprintdialog.cpp
> index 0294764..0c86970 100644
> --- a/modules/gui/qt4/dialogs/fingerprintdialog.cpp
> +++ b/modules/gui/qt4/dialogs/fingerprintdialog.cpp
> @@ -26,6 +26,7 @@
>
>   #include <QLabel>
>   #include <QListWidgetItem>
> +#include <new>
>
>   FingerprintDialog::FingerprintDialog(QWidget *parent, intf_thread_t *p_intf,
>                                        input_item_t *p_item ) :
> @@ -49,7 +50,7 @@ FingerprintDialog::FingerprintDialog(QWidget *parent, intf_thread_t *p_intf,
>       CONNECT( ui->buttonBox, rejected(), this, close() );
>       CONNECT( ui->buttonsBox, rejected(), this, close() );
>
> -    t = new Chromaprint( p_intf );
> +    t = new (std::nothrow) Chromaprint( p_intf );
>       if ( t )
>       {
>           CONNECT( t, finished(), this, handleResults() );
> diff --git a/modules/gui/skins2/commands/async_queue.cpp b/modules/gui/skins2/commands/async_queue.cpp
> index fca11d0..fcdbecb 100644
> --- a/modules/gui/skins2/commands/async_queue.cpp
> +++ b/modules/gui/skins2/commands/async_queue.cpp
> @@ -26,6 +26,7 @@
>   #include "../src/os_factory.hpp"
>   #include "../src/os_timer.hpp"
>
> +#include <new>
>
>   AsyncQueue::AsyncQueue( intf_thread_t *pIntf ): SkinObject( pIntf ),
>       m_cmdFlush( this )
> @@ -54,7 +55,7 @@ AsyncQueue *AsyncQueue::instance( intf_thread_t *pIntf )
>       if( ! pIntf->p_sys->p_queue )
>       {
>           AsyncQueue *pQueue;
> -        pQueue = new AsyncQueue( pIntf );
> +        pQueue = new (std::nothrow) AsyncQueue( pIntf );
>           if( pQueue )
>           {
>                // Initialization succeeded
> diff --git a/modules/gui/skins2/src/art_manager.cpp b/modules/gui/skins2/src/art_manager.cpp
> index 4533b75..f4dec63 100644
> --- a/modules/gui/skins2/src/art_manager.cpp
> +++ b/modules/gui/skins2/src/art_manager.cpp
> @@ -28,6 +28,8 @@
>   #include "art_manager.hpp"
>   #include <vlc_image.h>
>
> +#include <new>
> +
>   #define MAX_ART_CACHED    2
>
>
> @@ -91,7 +93,7 @@ ArtBitmap* ArtManager::getArtBitmap( std::string uriName )
>       }
>
>       // create and retain a new ArtBitmap since uri is not yet known
> -    ArtBitmap* pArt = new ArtBitmap( getIntf(), m_pImageHandler, uriName );
> +    ArtBitmap* pArt = new (std::nothrow) ArtBitmap( getIntf(), m_pImageHandler, uriName );
>       if( pArt && pArt->getWidth() && pArt->getHeight() )
>       {
>           if( m_listBitmap.size() == MAX_ART_CACHED )
> diff --git a/modules/gui/skins2/src/var_manager.cpp b/modules/gui/skins2/src/var_manager.cpp
> index 209a538..fc91159 100644
> --- a/modules/gui/skins2/src/var_manager.cpp
> +++ b/modules/gui/skins2/src/var_manager.cpp
> @@ -23,7 +23,7 @@
>    *****************************************************************************/
>
>   #include "var_manager.hpp"
> -
> +#include <new>
>
>   VarManager::VarManager( intf_thread_t *pIntf ): SkinObject( pIntf ),
>       m_pTooltipText( NULL ), m_pHelpText( NULL )
> @@ -62,7 +62,7 @@ VarManager *VarManager::instance( intf_thread_t *pIntf )
>       if( ! pIntf->p_sys->p_varManager )
>       {
>           VarManager *pVarManager;
> -        pVarManager = new VarManager( pIntf );
> +        pVarManager = new (std::nothrow) VarManager( pIntf );
>           if( pVarManager )
>           {
>               pIntf->p_sys->p_varManager = pVarManager;
> diff --git a/modules/gui/skins2/x11/x11_window.cpp b/modules/gui/skins2/x11/x11_window.cpp
> index 7e83648..c38202f 100644
> --- a/modules/gui/skins2/x11/x11_window.cpp
> +++ b/modules/gui/skins2/x11/x11_window.cpp
> @@ -38,6 +38,8 @@
>   #include <assert.h>
>   #include <limits.h>
>
> +#include <new>
> +
>   X11Window::X11Window( intf_thread_t *pIntf, GenericWindow &rWindow,
>                         X11Display &rDisplay, bool dragDrop, bool playOnDrop,
>                         X11Window *pParentWindow, GenericWindow::WindowType_t type ):
> @@ -201,7 +203,7 @@ X11Window::X11Window( intf_thread_t *pIntf, GenericWindow &rWindow,
>       long host_name_max = sysconf( _SC_HOST_NAME_MAX );
>       if( host_name_max <= 0 )
>           host_name_max = _POSIX_HOST_NAME_MAX;
> -    hostname = new char[host_name_max];
> +    hostname = new (std::nothrow) char[host_name_max];
>       if( hostname && gethostname( hostname, host_name_max ) == 0 )
>       {
>           hostname[host_name_max - 1] = '\0';
>

Hi Filip,

Merged, thanks!

Regards,


More information about the vlc-devel mailing list