From d97e0458914991214e3d396273862855aff66234 Mon Sep 17 00:00:00 2001 From: Thorsten Behrens Date: Mon, 1 Oct 2018 03:09:35 +0200 Subject: [PATCH] vcl: no raw pointers For ImplJobSetup. Also, check memcmp mem size more properly Change-Id: Idcf20bf1b51bc2508f3d37e018efd18e591a6099 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/26648 Tested-by: Jenkins Reviewed-by: Thorsten Behrens --- vcl/headless/svpprn.cxx | 14 +++----------- vcl/inc/jobdata.hxx | 2 +- vcl/inc/jobset.h | 9 ++++----- vcl/osx/salprn.cxx | 3 +-- vcl/source/gdi/jobset.cxx | 30 +++++++++++++---------------- vcl/source/gdi/print.cxx | 4 +--- vcl/unx/generic/print/genprnpsp.cxx | 18 ++++++----------- vcl/unx/generic/printer/jobdata.cxx | 6 +++--- vcl/win/gdi/salprn.cxx | 17 ++++++---------- 9 files changed, 38 insertions(+), 65 deletions(-) diff --git a/vcl/headless/svpprn.cxx b/vcl/headless/svpprn.cxx index b745ba692e65..6c40a0fd8d2f 100644 --- a/vcl/headless/svpprn.cxx +++ b/vcl/headless/svpprn.cxx @@ -146,21 +146,13 @@ static void copyJobDataToJobSetup( ImplJobSetup* pJobSetup, JobData& rData ) } // copy the whole context - if( pJobSetup->GetDriverData() ) - std::free( const_cast(pJobSetup->GetDriverData()) ); sal_uInt32 nBytes; - void* pBuffer = nullptr; + std::unique_ptr pBuffer; if( rData.getStreamBuffer( pBuffer, nBytes ) ) - { - pJobSetup->SetDriverDataLen( nBytes ); - pJobSetup->SetDriverData( static_cast(pBuffer) ); - } + pJobSetup->SetDriverData( std::move(pBuffer), nBytes ); else - { - pJobSetup->SetDriverDataLen( 0 ); - pJobSetup->SetDriverData( nullptr ); - } + pJobSetup->SetDriverData( nullptr, 0 ); } // SalInstance diff --git a/vcl/inc/jobdata.hxx b/vcl/inc/jobdata.hxx index ae9db11e69f6..46110057a888 100644 --- a/vcl/inc/jobdata.hxx +++ b/vcl/inc/jobdata.hxx @@ -69,7 +69,7 @@ struct VCL_DLLPUBLIC JobData // creates a new buffer using new // it is up to the user to delete it again - bool getStreamBuffer( void*& pData, sal_uInt32& bytes ); + bool getStreamBuffer( std::unique_ptr& pData, sal_uInt32& bytes ); static bool constructFromStreamBuffer( const void* pData, sal_uInt32 bytes, JobData& rJobData ); }; diff --git a/vcl/inc/jobset.h b/vcl/inc/jobset.h index e3f154254e35..7c0d0b55a4c3 100644 --- a/vcl/inc/jobset.h +++ b/vcl/inc/jobset.h @@ -25,6 +25,7 @@ #include #include #include +#include // see com.sun.star.portal.client.JobSetupSystem.idl: #define JOBSETUP_SYSTEM_WINDOWS 1 @@ -44,7 +45,7 @@ private: tools::Long mnPaperWidth; //< paper width (100th mm) tools::Long mnPaperHeight; //< paper height (100th mm) sal_uInt32 mnDriverDataLen; //< length of system specific data - sal_uInt8* mpDriverData; //< system specific data (will be streamed a byte block) + std::unique_ptr mpDriverData; //< system specific data (will be streamed a byte block) bool mbPapersizeFromSetup; // setup mode PrinterSetupMode meSetupMode; @@ -86,10 +87,8 @@ public: void SetPaperHeight(tools::Long nHeight); sal_uInt32 GetDriverDataLen() const { return mnDriverDataLen; } - void SetDriverDataLen(sal_uInt32 nDriverDataLen); - - const sal_uInt8* GetDriverData() const { return mpDriverData; } - void SetDriverData(sal_uInt8* pDriverData); + const sal_uInt8* GetDriverData() const { return mpDriverData.get(); } + void SetDriverData(std::unique_ptr pDriverData, sal_uInt32 nDriverDataLen); bool GetPapersizeFromSetup() const { return mbPapersizeFromSetup; } void SetPapersizeFromSetup(bool bPapersizeFromSetup); diff --git a/vcl/osx/salprn.cxx b/vcl/osx/salprn.cxx index e9101e390085..9f9c8c08f3db 100644 --- a/vcl/osx/salprn.cxx +++ b/vcl/osx/salprn.cxx @@ -190,8 +190,7 @@ bool AquaSalInfoPrinter::SetPrinterData( ImplJobSetup* io_pSetupData ) io_pSetupData->SetOrientation( mePageOrientation ); io_pSetupData->SetPaperBin( 0 ); - io_pSetupData->SetDriverData( static_cast(std::malloc( 4 )) ); - io_pSetupData->SetDriverDataLen( 4 ); + io_pSetupData->SetDriverData( std::make_unique(4), 4 ); } else bSuccess = false; diff --git a/vcl/source/gdi/jobset.cxx b/vcl/source/gdi/jobset.cxx index 9969a6165327..c9ed0d9626ec 100644 --- a/vcl/source/gdi/jobset.cxx +++ b/vcl/source/gdi/jobset.cxx @@ -65,7 +65,6 @@ ImplJobSetup::ImplJobSetup() mnPaperWidth = 0; mnPaperHeight = 0; mnDriverDataLen = 0; - mpDriverData = nullptr; mbPapersizeFromSetup = false; meSetupMode = PrinterSetupMode::DocumentGlobal; } @@ -81,14 +80,15 @@ ImplJobSetup::ImplJobSetup( const ImplJobSetup& rJobSetup ) : mnPaperWidth( rJobSetup.GetPaperWidth() ), mnPaperHeight( rJobSetup.GetPaperHeight() ), mnDriverDataLen( rJobSetup.GetDriverDataLen() ), + mpDriverData(), mbPapersizeFromSetup( rJobSetup.GetPapersizeFromSetup() ), meSetupMode( rJobSetup.GetPrinterSetupMode() ), maValueMap( rJobSetup.GetValueMap() ) { if ( rJobSetup.GetDriverData() ) { - mpDriverData = static_cast(std::malloc( mnDriverDataLen )); - memcpy( mpDriverData, rJobSetup.GetDriverData(), mnDriverDataLen ); + mpDriverData.reset( new sal_uInt8[mnDriverDataLen] ); + memcpy( mpDriverData.get(), rJobSetup.GetDriverData(), mnDriverDataLen ); } else mpDriverData = nullptr; @@ -96,7 +96,6 @@ ImplJobSetup::ImplJobSetup( const ImplJobSetup& rJobSetup ) : ImplJobSetup::~ImplJobSetup() { - std::free( mpDriverData ); } void ImplJobSetup::SetSystem(sal_uInt16 nSystem) @@ -144,16 +143,12 @@ void ImplJobSetup::SetPaperHeight(tools::Long nPaperHeight) mnPaperHeight = nPaperHeight; } -void ImplJobSetup::SetDriverDataLen(sal_uInt32 nDriverDataLen) +void ImplJobSetup::SetDriverData(std::unique_ptr pDriverData, sal_uInt32 nDriverDataLen) { + mpDriverData = std::move(pDriverData); mnDriverDataLen = nDriverDataLen; } -void ImplJobSetup::SetDriverData(sal_uInt8* pDriverData) -{ - mpDriverData = pDriverData; -} - void ImplJobSetup::SetPapersizeFromSetup(bool bPapersizeFromSetup) { mbPapersizeFromSetup = bPapersizeFromSetup; @@ -187,7 +182,9 @@ bool ImplJobSetup::operator==( const ImplJobSetup& rImplJobSetup ) const mbPapersizeFromSetup == rImplJobSetup.mbPapersizeFromSetup && mnDriverDataLen == rImplJobSetup.mnDriverDataLen && maValueMap == rImplJobSetup.maValueMap && - memcmp( mpDriverData, rImplJobSetup.mpDriverData, mnDriverDataLen ) == 0; + memcmp( mpDriverData.get(), + rImplJobSetup.mpDriverData.get(), + std::min(mnDriverDataLen, rImplJobSetup.mnDriverDataLen)) == 0; } namespace @@ -283,7 +280,7 @@ SvStream& ReadJobSetup( SvStream& rIStream, JobSetup& rJobSetup ) Impl364JobSetupData* pOldJobData = reinterpret_cast(pTempBuf.get() + sizeof( ImplOldJobSetupData )); sal_uInt16 nOldJobDataSize = SVBT16ToUInt16( pOldJobData->nSize ); rJobData.SetSystem( SVBT16ToUInt16( pOldJobData->nSystem ) ); - rJobData.SetDriverDataLen( SVBT32ToUInt32( pOldJobData->nDriverDataLen ) ); + const sal_uInt32 nDriverDataLen = SVBT32ToUInt32( pOldJobData->nDriverDataLen ); rJobData.SetOrientation( static_cast(SVBT16ToUInt16( pOldJobData->nOrientation )) ); rJobData.SetDuplexMode( DuplexMode::Unknown ); rJobData.SetPaperBin( SVBT16ToUInt16( pOldJobData->nPaperBin ) ); @@ -297,7 +294,7 @@ SvStream& ReadJobSetup( SvStream& rIStream, JobSetup& rJobSetup ) } rJobData.SetPaperWidth( static_cast(SVBT32ToUInt32( pOldJobData->nPaperWidth )) ); rJobData.SetPaperHeight( static_cast(SVBT32ToUInt32( pOldJobData->nPaperHeight )) ); - if ( rJobData.GetDriverDataLen() ) + if ( nDriverDataLen ) { const char* pDriverData = reinterpret_cast(pOldJobData) + nOldJobDataSize; const char* pDriverDataEnd = pDriverData + rJobData.GetDriverDataLen(); @@ -307,10 +304,9 @@ SvStream& ReadJobSetup( SvStream& rIStream, JobSetup& rJobSetup ) } else { - sal_uInt8* pNewDriverData = static_cast( - std::malloc( rJobData.GetDriverDataLen() )); - memcpy( pNewDriverData, pDriverData, rJobData.GetDriverDataLen() ); - rJobData.SetDriverData( pNewDriverData ); + auto pNewDriverData = std::make_unique( nDriverDataLen ); + memcpy( pNewDriverData.get(), pDriverData, nDriverDataLen ); + rJobData.SetDriverData( std::move(pNewDriverData), nDriverDataLen ); } } if( nSystem == JOBSET_FILE605_SYSTEM ) diff --git a/vcl/source/gdi/print.cxx b/vcl/source/gdi/print.cxx index 3e92abd02487..eff94a9211ea 100644 --- a/vcl/source/gdi/print.cxx +++ b/vcl/source/gdi/print.cxx @@ -616,9 +616,7 @@ void Printer::ImplInit( SalPrinterQueueInfo* pInfo ) if ( rData.GetPrinterName() != pInfo->maPrinterName || rData.GetDriver() != pInfo->maDriver ) { - std::free( const_cast(rData.GetDriverData()) ); - rData.SetDriverData(nullptr); - rData.SetDriverDataLen(0); + rData.SetDriverData(nullptr, 0); } } diff --git a/vcl/unx/generic/print/genprnpsp.cxx b/vcl/unx/generic/print/genprnpsp.cxx index 78f3af4292c0..33990decad1c 100644 --- a/vcl/unx/generic/print/genprnpsp.cxx +++ b/vcl/unx/generic/print/genprnpsp.cxx @@ -226,20 +226,16 @@ static void copyJobDataToJobSetup( ImplJobSetup* pJobSetup, JobData& rData ) } // copy the whole context - if( pJobSetup->GetDriverData() ) - std::free( const_cast(pJobSetup->GetDriverData()) ); sal_uInt32 nBytes; - void* pBuffer = nullptr; + std::unique_ptr pBuffer; if( rData.getStreamBuffer( pBuffer, nBytes ) ) { - pJobSetup->SetDriverDataLen( nBytes ); - pJobSetup->SetDriverData( static_cast(pBuffer) ); + pJobSetup->SetDriverData( std::move(pBuffer), nBytes ); } else { - pJobSetup->SetDriverDataLen( 0 ); - pJobSetup->SetDriverData( nullptr ); + pJobSetup->SetDriverData( nullptr, 0 ); } pJobSetup->SetPapersizeFromSetup( rData.m_bPapersizeFromSetup ); } @@ -428,14 +424,12 @@ bool PspSalInfoPrinter::Setup( weld::Window* pFrame, ImplJobSetup* pJobSetup ) if (SetupPrinterDriver(pFrame, aInfo)) { - std::free( const_cast(pJobSetup->GetDriverData()) ); - pJobSetup->SetDriverData( nullptr ); + pJobSetup->SetDriverData( nullptr, 0 ); sal_uInt32 nBytes; - void* pBuffer = nullptr; + std::unique_ptr pBuffer; aInfo.getStreamBuffer( pBuffer, nBytes ); - pJobSetup->SetDriverDataLen( nBytes ); - pJobSetup->SetDriverData( static_cast(pBuffer) ); + pJobSetup->SetDriverData( std::move(pBuffer), nBytes ); // copy everything to job setup copyJobDataToJobSetup( pJobSetup, aInfo ); diff --git a/vcl/unx/generic/printer/jobdata.cxx b/vcl/unx/generic/printer/jobdata.cxx index c53bf06f7041..5fc6a7870006 100644 --- a/vcl/unx/generic/printer/jobdata.cxx +++ b/vcl/unx/generic/printer/jobdata.cxx @@ -87,7 +87,7 @@ void JobData::setPaperBin( int i_nPaperBin ) } } -bool JobData::getStreamBuffer( void*& pData, sal_uInt32& bytes ) +bool JobData::getStreamBuffer( std::unique_ptr& pData, sal_uInt32& bytes ) { // consistency checks if( ! m_pParser ) @@ -141,8 +141,8 @@ bool JobData::getStreamBuffer( void*& pData, sal_uInt32& bytes ) // success bytes = static_cast(aStream.Tell()); - pData = std::malloc( bytes ); - memcpy( pData, aStream.GetData(), bytes ); + pData = std::make_unique( bytes ); + memcpy( pData.get(), aStream.GetData(), bytes ); return true; } diff --git a/vcl/win/gdi/salprn.cxx b/vcl/win/gdi/salprn.cxx index 78c8135a93d2..0631bcd37c3f 100644 --- a/vcl/win/gdi/salprn.cxx +++ b/vcl/win/gdi/salprn.cxx @@ -317,9 +317,7 @@ static bool ImplTestSalJobSetup( WinSalInfoPrinter const * pPrinter, } if ( bDelete ) { - std::free( const_cast(pSetupData->GetDriverData()) ); - pSetupData->SetDriverData( nullptr ); - pSetupData->SetDriverDataLen( 0 ); + pSetupData->SetDriverData( nullptr, 0 ); } } @@ -340,6 +338,7 @@ static bool ImplUpdateSalJobSetup( WinSalInfoPrinter const * pPrinter, ImplJobSe LONG nRet; HWND hWnd = nullptr; DWORD nMode = DM_OUT_BUFFER; + std::unique_ptr pDriverData; SalDriverData* pOutBuffer = nullptr; BYTE const * pInBuffer = nullptr; @@ -354,7 +353,9 @@ static bool ImplUpdateSalJobSetup( WinSalInfoPrinter const * pPrinter, ImplJobSe // make Outputbuffer const std::size_t nDriverDataLen = sizeof(SalDriverData) + nSysJobSize-1; - pOutBuffer = static_cast(rtl_allocateZeroMemory( nDriverDataLen )); + pDriverData = std::make_unique( nDriverDataLen ); + memset(pDriverData.get(), 0, nDriverDataLen); + pOutBuffer = reinterpret_cast(pDriverData.get()); pOutBuffer->mnSysSignature = SAL_DRIVERDATA_SYSSIGN; // calculate driver data offset including structure padding pOutBuffer->mnDriverOffset = sal::static_int_cast( @@ -390,10 +391,7 @@ static bool ImplUpdateSalJobSetup( WinSalInfoPrinter const * pPrinter, ImplJobSe ClosePrinter( hPrn ); if( (nRet < 0) || (pVisibleDlgParent && (nRet == IDCANCEL)) ) - { - std::free( pOutBuffer ); return false; - } // fill up string buffers with 0 so they do not influence a JobSetup's memcmp if( reinterpret_cast(pOutDevMode)->dmSize >= 64 ) @@ -410,10 +408,7 @@ static bool ImplUpdateSalJobSetup( WinSalInfoPrinter const * pPrinter, ImplJobSe } // update data - if ( pSetupData->GetDriverData() ) - std::free( const_cast(pSetupData->GetDriverData()) ); - pSetupData->SetDriverDataLen( nDriverDataLen ); - pSetupData->SetDriverData(reinterpret_cast(pOutBuffer)); + pSetupData->SetDriverData(std::move(pDriverData), nDriverDataLen); pSetupData->SetSystem( JOBSETUP_SYSTEM_WINDOWS ); return true;