From db22c91d44048c1eefebf90ef23554e68fc213fc Mon Sep 17 00:00:00 2001 From: pbfordev Date: Fri, 16 Jun 2017 21:07:50 +0200 Subject: [PATCH 1/9] Introduces wxBSTR, an RAII wrapper for MSW BSTR type. wxBSTR also supersedes wxBasicString. --- include/wx/msw/ole/activex.h | 2 +- include/wx/msw/ole/oleutils.h | 72 ++++++++++++++++++++++++----------- src/msw/mediactrl_am.cpp | 6 +-- src/msw/mediactrl_wmp10.cpp | 30 +++++++-------- src/msw/ole/access.cpp | 26 +++++-------- src/msw/ole/automtn.cpp | 6 +-- src/msw/ole/oleutils.cpp | 62 ++++++++++++++++++++---------- 7 files changed, 122 insertions(+), 82 deletions(-) diff --git a/include/wx/msw/ole/activex.h b/include/wx/msw/ole/activex.h index 1518671eb8..8ecd647208 100644 --- a/include/wx/msw/ole/activex.h +++ b/include/wx/msw/ole/activex.h @@ -21,7 +21,7 @@ // wx includes //--------------------------------------------------------------------------- -#include "wx/msw/ole/oleutils.h" // wxBasicString &c +#include "wx/msw/ole/oleutils.h" #include "wx/msw/ole/uuid.h" #include "wx/window.h" #include "wx/variant.h" diff --git a/include/wx/msw/ole/oleutils.h b/include/wx/msw/ole/oleutils.h index 1dab81b18a..73af64bf23 100644 --- a/include/wx/msw/ole/oleutils.h +++ b/include/wx/msw/ole/oleutils.h @@ -61,29 +61,6 @@ inline void wxOleUninitialize() ::OleUninitialize(); } -// wrapper around BSTR type (by Vadim Zeitlin) - -class WXDLLIMPEXP_CORE wxBasicString -{ -public: - // ctors & dtor - wxBasicString(const wxString& str); - wxBasicString(const wxBasicString& bstr); - ~wxBasicString(); - - wxBasicString& operator=(const wxBasicString& bstr); - - // accessors - // just get the string - operator BSTR() const { return m_bstrBuf; } - // retrieve a copy of our string - caller must SysFreeString() it later! - BSTR Get() const { return SysAllocString(m_bstrBuf); } - -private: - // actual string - BSTR m_bstrBuf; -}; - #if wxUSE_VARIANT // Convert variants class WXDLLIMPEXP_FWD_BASE wxVariant; @@ -195,6 +172,55 @@ WXDLLIMPEXP_CORE BSTR wxConvertStringToOle(const wxString& str); // Convert string from BSTR to wxString WXDLLIMPEXP_CORE wxString wxConvertStringFromOle(BSTR bStr); +// A thin RAII wrapper for BSTR, which can create BSTR +// from wxString as well as return the owned BSTR as wxString. +// Unlike _b_str_t, wxBSTR is NOT reference counted. +class WXDLLIMPEXP_CORE wxBSTR +{ +public: + wxBSTR() : m_bstr(NULL) {} + + // If copy is true then a copy of bstr is created, + // if not then ownership of bstr is taken. + wxBSTR(BSTR bstr, bool copy); + + // Creates BSTR from wxString + wxBSTR(const wxString& str) : m_bstr(::SysAllocString(str.wc_str(*wxConvCurrent))) {} + + // Creates a copy of BSTR owned by wxbstr + wxBSTR(const wxBSTR& wxbstr) : m_bstr(wxbstr.GetCopy()) {} + + // Frees the owned BSTR + ~wxBSTR() { Free(); } + + // Creates a copy of wxbstr + wxBSTR& operator=(const wxBSTR& wxbstr); + + // Takes ownership of bstr + void Attach(BSTR bstr); + + // Returns the owned BSTR and gives up its ownership + BSTR Detach(); + + // Frees the owned BSTR + void Free(); + + // Returnes the owned BSTR while keeping its ownership + BSTR GetBSTR() const { return m_bstr; } + operator BSTR() const { return GetBSTR(); } + + // Returns the address of owned BSTR + BSTR* GetAddress() { return &m_bstr; } + + // Return a copy of owned BSTR + BSTR GetCopy() const { return ::SysAllocString(m_bstr); } + + // Returns the owned BSTR convert to wxString + wxString GetwxString() const { return wxConvertStringFromOle(m_bstr); } +private: + BSTR m_bstr; +}; + #else // !wxUSE_OLE // ---------------------------------------------------------------------------- diff --git a/src/msw/mediactrl_am.cpp b/src/msw/mediactrl_am.cpp index 6821e423da..43cb9c197a 100644 --- a/src/msw/mediactrl_am.cpp +++ b/src/msw/mediactrl_am.cpp @@ -1127,7 +1127,7 @@ bool wxAMMediaBackend::Load(const wxURI& location, const wxURI& proxy) if(pPlay) { pPlay->put_UseHTTPProxy(VARIANT_TRUE); - pPlay->put_HTTPProxyHost(wxBasicString(proxy.GetServer()).Get()); + pPlay->put_HTTPProxyHost(wxBSTR(proxy.GetServer()).Detach()); pPlay->put_HTTPProxyPort(wxAtoi(proxy.GetPort())); pPlay->Release(); } @@ -1150,9 +1150,9 @@ bool wxAMMediaBackend::DoLoad(const wxString& location) // the docs say its async and put_FileName is not - // but in practice they both seem to be async anyway if(GetMP()) - hr = GetMP()->Open( wxBasicString(location).Get() ); + hr = GetMP()->Open( wxBSTR(location).Detach() ); else - hr = GetAM()->put_FileName( wxBasicString(location).Get() ); + hr = GetAM()->put_FileName( wxBSTR(location).Detach() ); if(FAILED(hr)) { diff --git a/src/msw/mediactrl_wmp10.cpp b/src/msw/mediactrl_wmp10.cpp index 533abf3c76..b8f85c723d 100644 --- a/src/msw/mediactrl_wmp10.cpp +++ b/src/msw/mediactrl_wmp10.cpp @@ -905,21 +905,21 @@ bool wxWMP10MediaBackend::Load(const wxURI& location, { long lOldSetting; if( pWMPNetwork->getProxySettings( - wxBasicString(location.GetScheme()).Get(), &lOldSetting + wxBSTR(location.GetScheme()).Detach(), &lOldSetting ) == 0 && pWMPNetwork->setProxySettings( - wxBasicString(location.GetScheme()).Get(), // protocol + wxBSTR(location.GetScheme()).Detach(), // protocol 2) == 0) // 2 == manually specify { BSTR bsOldName = NULL; long lOldPort = 0; pWMPNetwork->getProxyName( - wxBasicString(location.GetScheme()).Get(), + wxBSTR(location.GetScheme()).Detach(), &bsOldName); pWMPNetwork->getProxyPort( - wxBasicString(location.GetScheme()).Get(), + wxBSTR(location.GetScheme()).Detach(), &lOldPort); long lPort; @@ -936,11 +936,11 @@ bool wxWMP10MediaBackend::Load(const wxURI& location, } if( pWMPNetwork->setProxyName( - wxBasicString(location.GetScheme()).Get(), // proto - wxBasicString(server).Get() ) == 0 && + wxBSTR(location.GetScheme()).Detach(), // proto + wxBSTR(server).Detach() ) == 0 && pWMPNetwork->setProxyPort( - wxBasicString(location.GetScheme()).Get(), // proto + wxBSTR(location.GetScheme()).Detach(), // proto lPort ) == 0 ) @@ -948,16 +948,16 @@ bool wxWMP10MediaBackend::Load(const wxURI& location, bOK = DoLoad(location.BuildURI()); pWMPNetwork->setProxySettings( - wxBasicString(location.GetScheme()).Get(), // protocol + wxBSTR(location.GetScheme()).Detach(), // protocol lOldSetting); if(bsOldName) pWMPNetwork->setProxyName( - wxBasicString(location.GetScheme()).Get(), // protocol + wxBSTR(location.GetScheme()).Detach(), // protocol bsOldName); if(lOldPort) pWMPNetwork->setProxyPort( - wxBasicString(location.GetScheme()).Get(), // protocol + wxBSTR(location.GetScheme()).Detach(), // protocol lOldPort); pWMPNetwork->Release(); @@ -997,7 +997,7 @@ bool wxWMP10MediaBackend::DoLoad(const wxString& location) { IWMPMedia* pWMPMedia; - if( (hr = pWMPCore3->newMedia(wxBasicString(location).Get(), + if( (hr = pWMPCore3->newMedia(wxBSTR(location).Detach(), &pWMPMedia)) == 0) { // this (get_duration) will actually FAIL, but it will work. @@ -1012,7 +1012,7 @@ bool wxWMP10MediaBackend::DoLoad(const wxString& location) #endif { // just load it the "normal" way - hr = m_pWMPPlayer->put_URL( wxBasicString(location).Get() ); + hr = m_pWMPPlayer->put_URL( wxBSTR(location).Detach() ); } if(FAILED(hr)) @@ -1061,12 +1061,12 @@ bool wxWMP10MediaBackend::ShowPlayerControls(wxMediaCtrlPlayerControls flags) if(!flags) { m_pWMPPlayer->put_enabled(VARIANT_FALSE); - m_pWMPPlayer->put_uiMode(wxBasicString(wxT("none")).Get()); + m_pWMPPlayer->put_uiMode(wxBSTR(wxT("none")).Detach()); } else { // TODO: use "custom"? (note that CE only supports none/full) - m_pWMPPlayer->put_uiMode(wxBasicString(wxT("full")).Get()); + m_pWMPPlayer->put_uiMode(wxBSTR(wxT("full")).Detach()); m_pWMPPlayer->put_enabled(VARIANT_TRUE); } @@ -1358,7 +1358,7 @@ wxLongLong wxWMP10MediaBackend::GetDownloadTotal() if(m_pWMPPlayer->get_currentMedia(&pWMPMedia) == 0) { BSTR bsOut; - pWMPMedia->getItemInfo(wxBasicString(wxT("FileSize")).Get(), + pWMPMedia->getItemInfo(wxBSTR(wxT("FileSize")).Detach(), &bsOut); wxString sFileSize = wxConvertStringFromOle(bsOut); diff --git a/src/msw/ole/access.cpp b/src/msw/ole/access.cpp index 7c5d3541d5..d8ce5753ea 100644 --- a/src/msw/ole/access.cpp +++ b/src/msw/ole/access.cpp @@ -937,9 +937,8 @@ STDMETHODIMP wxIAccessible::get_accDefaultAction ( VARIANT varID, BSTR* pszDefau return S_FALSE; } else - { - wxBasicString basicString(defaultAction); - * pszDefaultAction = basicString.Get(); + { + * pszDefaultAction = wxBSTR(defaultAction).Detach(); return S_OK; } } @@ -998,9 +997,8 @@ STDMETHODIMP wxIAccessible::get_accDescription ( VARIANT varID, BSTR* pszDescrip return S_FALSE; } else - { - wxBasicString basicString(description); - * pszDescription = basicString.Get(); + { + * pszDescription = wxBSTR(description).Detach(); return S_OK; } } @@ -1059,9 +1057,8 @@ STDMETHODIMP wxIAccessible::get_accHelp ( VARIANT varID, BSTR* pszHelp) return S_FALSE; } else - { - wxBasicString basicString(helpString); - * pszHelp = basicString.Get(); + { + * pszHelp = wxBSTR(helpString).Detach(); return S_OK; } } @@ -1171,8 +1168,7 @@ STDMETHODIMP wxIAccessible::get_accKeyboardShortcut ( VARIANT varID, BSTR* pszKe } else { - wxBasicString basicString(keyboardShortcut); - * pszKeyboardShortcut = basicString.Get(); + * pszKeyboardShortcut = wxBSTR(keyboardShortcut).Detach(); return S_OK; } } @@ -1234,9 +1230,8 @@ STDMETHODIMP wxIAccessible::get_accName ( VARIANT varID, BSTR* pszName) return S_FALSE; } else - { - wxBasicString basicString(name); - *pszName = basicString.Get(); + { + *pszName = wxBSTR(name).Detach(); } return S_OK; } @@ -1416,8 +1411,7 @@ STDMETHODIMP wxIAccessible::get_accValue ( VARIANT varID, BSTR* pszValue) } else { - wxBasicString basicString(strValue); - * pszValue = basicString.Get(); + * pszValue = wxBSTR(strValue).Detach(); return S_OK; } } diff --git a/src/msw/ole/automtn.cpp b/src/msw/ole/automtn.cpp index fdbd9f4d4b..e77cdbb38f 100644 --- a/src/msw/ole/automtn.cpp +++ b/src/msw/ole/automtn.cpp @@ -130,7 +130,7 @@ bool wxAutomationObject::Invoke(const wxString& member, int action, } int namedArgStringCount = namedArgCount + 1; - wxVector argNames(namedArgStringCount, wxString()); + wxVector argNames(namedArgStringCount, wxString()); argNames[0] = member; // Note that arguments are specified in reverse order @@ -156,7 +156,7 @@ bool wxAutomationObject::Invoke(const wxString& member, int action, // Get the IDs for the member and its arguments. GetIDsOfNames expects the // member name as the first name, followed by argument names (if any). hr = ((IDispatch*)m_dispatchPtr)->GetIDsOfNames(IID_NULL, - // We rely on the fact that wxBasicString is + // We rely on the fact that wxBSTR is // just BSTR with some methods here. reinterpret_cast(&argNames[0]), 1 + namedArgCount, m_lcid, &dispIds[0]); @@ -499,7 +499,7 @@ namespace HRESULT wxCLSIDFromProgID(const wxString& progId, CLSID& clsId) { - HRESULT hr = CLSIDFromProgID(wxBasicString(progId), &clsId); + HRESULT hr = CLSIDFromProgID(wxBSTR(progId), &clsId); if ( FAILED(hr) ) { wxLogSysError(hr, _("Failed to find CLSID of \"%s\""), progId); diff --git a/src/msw/ole/oleutils.cpp b/src/msw/ole/oleutils.cpp index 67c1477753..341a716434 100644 --- a/src/msw/ole/oleutils.cpp +++ b/src/msw/ole/oleutils.cpp @@ -38,7 +38,7 @@ WXDLLEXPORT BSTR wxConvertStringToOle(const wxString& str) { - return wxBasicString(str).Get(); + return wxBSTR(str).Detach(); } WXDLLEXPORT wxString wxConvertStringFromOle(BSTR bStr) @@ -68,28 +68,48 @@ WXDLLEXPORT wxString wxConvertStringFromOle(BSTR bStr) } // ---------------------------------------------------------------------------- -// wxBasicString +// wxBSTR // ---------------------------------------------------------------------------- -wxBasicString::wxBasicString(const wxString& str) -{ - m_bstrBuf = SysAllocString(str.wc_str(*wxConvCurrent)); -} - -wxBasicString::wxBasicString(const wxBasicString& src) -{ - m_bstrBuf = src.Get(); -} - -wxBasicString& wxBasicString::operator=(const wxBasicString& src) -{ - SysReAllocString(&m_bstrBuf, src); - return *this; -} - -wxBasicString::~wxBasicString() -{ - SysFreeString(m_bstrBuf); +wxBSTR::wxBSTR(BSTR bstr, bool copy) +{ + if ( copy ) + m_bstr = ::SysAllocString(bstr); + else + m_bstr = bstr; +} + +wxBSTR& wxBSTR::operator=(const wxBSTR& wxbstr) +{ + if ( wxbstr != *this ) + { + Free(); + m_bstr = wxbstr.GetCopy(); + } + + return *this; +} + +void wxBSTR::Attach(BSTR bstr) +{ + wxCHECK_RET(m_bstr != bstr, wxS("Attaching already attached BSTR!")); + + Free(); + m_bstr = bstr; +} + +BSTR wxBSTR::Detach() +{ + BSTR bstr = m_bstr; + + m_bstr = NULL; + return bstr; +} + +void wxBSTR::Free() +{ + ::SysFreeString(m_bstr); + m_bstr = NULL; } From 82104e0e546b85f91b363e2fd528b9d8c6549e53 Mon Sep 17 00:00:00 2001 From: pbfordev Date: Fri, 16 Jun 2017 21:38:13 +0200 Subject: [PATCH 2/9] Use wxBSTR to prevent leaking BSTRs in wxWebViewIE. --- src/msw/webview_ie.cpp | 55 ++++++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/src/msw/webview_ie.cpp b/src/msw/webview_ie.cpp index 691a90aeb0..9de2345401 100644 --- a/src/msw/webview_ie.cpp +++ b/src/msw/webview_ie.cpp @@ -231,9 +231,9 @@ wxString wxWebViewIE::GetPageSource() const hr = bodyTag->get_parentElement(&htmlTag); if(SUCCEEDED(hr)) { - BSTR bstr; - if ( htmlTag->get_outerHTML(&bstr) == S_OK ) - source = wxString(bstr); + wxBSTR wxbstr; + if ( htmlTag->get_outerHTML(wxbstr.GetAddress()) == S_OK ) + source = wxbstr.GetwxString(); } } return source; @@ -576,9 +576,9 @@ wxString wxWebViewIE::GetCurrentTitle() const wxString s; if(document) { - BSTR title; - if ( document->get_nameProp(&title) == S_OK ) - s = title; + wxBSTR title; + if ( document->get_nameProp(title.GetAddress()) == S_OK ) + s = title.GetwxString(); } return s; @@ -705,10 +705,10 @@ bool wxWebViewIE::IsEditable() const if(document) { - BSTR mode; - if ( document->get_designMode(&mode) == S_OK ) + wxBSTR mode; + if ( document->get_designMode(mode.GetAddress()) == S_OK ) { - if ( wxString(mode) == "On" ) + if ( mode.GetwxString() == "On" ) return true; } } @@ -731,9 +731,9 @@ bool wxWebViewIE::HasSelection() const HRESULT hr = document->get_selection(&selection); if(SUCCEEDED(hr)) { - BSTR type; - if ( selection->get_type(&type) == S_OK ) - sel = wxString(type); + wxBSTR type; + if ( selection->get_type(type.GetAddress()) == S_OK ) + sel = type.GetwxString(); } return sel != "None"; } @@ -767,9 +767,9 @@ wxString wxWebViewIE::GetSelectedText() const hr = disrange->QueryInterface(IID_IHTMLTxtRange, (void**)&range); if(SUCCEEDED(hr)) { - BSTR text; - if ( range->get_text(&text) == S_OK ) - selected = wxString(text); + wxBSTR text; + if ( range->get_text(text.GetAddress()) == S_OK ) + selected = text.GetwxString(); } } } @@ -800,9 +800,9 @@ wxString wxWebViewIE::GetSelectedSource() const hr = disrange->QueryInterface(IID_IHTMLTxtRange, (void**)&range); if(SUCCEEDED(hr)) { - BSTR text; - if ( range->get_htmlText(&text) == S_OK ) - selected = wxString(text); + wxBSTR text; + if ( range->get_htmlText(text.GetAddress()) == S_OK ) + selected = text.GetwxString(); } } } @@ -841,9 +841,9 @@ wxString wxWebViewIE::GetPageText() const HRESULT hr = document->get_body(&body); if(SUCCEEDED(hr)) { - BSTR out; - if ( body->get_innerText(&out) == S_OK ) - text = wxString(out); + wxBSTR out; + if ( body->get_innerText(out.GetAddress()) == S_OK ) + text = out.GetwxString(); } return text; } @@ -950,7 +950,7 @@ wxCOMPtr wxWebViewIE::GetDocument() const bool wxWebViewIE::IsElementVisible(wxCOMPtr elm) { wxCOMPtr elm1 = elm; - BSTR tmp_bstr; + wxBSTR tmp_bstr; bool is_visible = true; //This method is not perfect but it does discover most of the hidden elements. //so if a better solution is found, then please do improve. @@ -963,13 +963,13 @@ bool wxWebViewIE::IsElementVisible(wxCOMPtr elm) if(SUCCEEDED(elm2->get_currentStyle(&style))) { //Check if the object has the style display:none. - if((style->get_display(&tmp_bstr) != S_OK) || + if((style->get_display(tmp_bstr.GetAddress()) != S_OK) || (tmp_bstr != NULL && (wxCRT_StricmpW(tmp_bstr, L"none") == 0))) { is_visible = false; } //Check if the object has the style visibility:hidden. - if((is_visible && (style->get_visibility(&tmp_bstr) != S_OK)) || + if((is_visible && (style->get_visibility(tmp_bstr.GetAddress()) != S_OK)) || (tmp_bstr != NULL && wxCRT_StricmpW(tmp_bstr, L"hidden") == 0)) { is_visible = false; @@ -1007,8 +1007,8 @@ void wxWebViewIE::FindInternal(const wxString& text, int flags, int internal_fla if(SUCCEEDED(document->QueryInterface(wxIID_IMarkupContainer, (void **)&pIMC))) { wxCOMPtr ptrBegin, ptrEnd; - BSTR attr_bstr = SysAllocString(L"style=\"background-color:#ffff00\""); - BSTR text_bstr = SysAllocString(text.wc_str()); + wxBSTR attr_bstr(L"style=\"background-color:#ffff00\""); + wxBSTR text_bstr(text.wc_str()); pIMS->CreateMarkupPointer(&ptrBegin); pIMS->CreateMarkupPointer(&ptrEnd); @@ -1064,9 +1064,6 @@ void wxWebViewIE::FindInternal(const wxString& text, int flags, int internal_fla } ptrBegin->MoveToPointer(ptrEnd); } - //Clean up. - SysFreeString(text_bstr); - SysFreeString(attr_bstr); } } } From 3dd7d57bd4884770e2a0cb83ba541ffb4fae09b6 Mon Sep 17 00:00:00 2001 From: pbfordev Date: Sat, 17 Jun 2017 08:52:48 +0200 Subject: [PATCH 3/9] wxBSTR cleanup: fixed a bug in wxBSTR::operator=(const wxBSTR& wxbstr); slightly improved comments in the code. --- include/wx/msw/ole/oleutils.h | 13 +++++++------ src/msw/ole/oleutils.cpp | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/include/wx/msw/ole/oleutils.h b/include/wx/msw/ole/oleutils.h index 73af64bf23..8e50251617 100644 --- a/include/wx/msw/ole/oleutils.h +++ b/include/wx/msw/ole/oleutils.h @@ -172,19 +172,20 @@ WXDLLIMPEXP_CORE BSTR wxConvertStringToOle(const wxString& str); // Convert string from BSTR to wxString WXDLLIMPEXP_CORE wxString wxConvertStringFromOle(BSTR bStr); -// A thin RAII wrapper for BSTR, which can create BSTR +// A thin RAII wrapper for BSTR, which can also create BSTR // from wxString as well as return the owned BSTR as wxString. // Unlike _b_str_t, wxBSTR is NOT reference counted. class WXDLLIMPEXP_CORE wxBSTR { public: + // Creates with the owned BSTR set to NULL wxBSTR() : m_bstr(NULL) {} // If copy is true then a copy of bstr is created, // if not then ownership of bstr is taken. wxBSTR(BSTR bstr, bool copy); - // Creates BSTR from wxString + // Creates the owned BSTR from wxString str wxBSTR(const wxString& str) : m_bstr(::SysAllocString(str.wc_str(*wxConvCurrent))) {} // Creates a copy of BSTR owned by wxbstr @@ -193,7 +194,7 @@ public: // Frees the owned BSTR ~wxBSTR() { Free(); } - // Creates a copy of wxbstr + // Creates a copy of the BSTR owned by wxbstr wxBSTR& operator=(const wxBSTR& wxbstr); // Takes ownership of bstr @@ -205,17 +206,17 @@ public: // Frees the owned BSTR void Free(); - // Returnes the owned BSTR while keeping its ownership + // Returns the owned BSTR while keeping its ownership BSTR GetBSTR() const { return m_bstr; } operator BSTR() const { return GetBSTR(); } // Returns the address of owned BSTR BSTR* GetAddress() { return &m_bstr; } - // Return a copy of owned BSTR + // Returns a copy of owned BSTR BSTR GetCopy() const { return ::SysAllocString(m_bstr); } - // Returns the owned BSTR convert to wxString + // Returns the owned BSTR converted to wxString wxString GetwxString() const { return wxConvertStringFromOle(m_bstr); } private: BSTR m_bstr; diff --git a/src/msw/ole/oleutils.cpp b/src/msw/ole/oleutils.cpp index 341a716434..6b9b37437d 100644 --- a/src/msw/ole/oleutils.cpp +++ b/src/msw/ole/oleutils.cpp @@ -81,7 +81,7 @@ wxBSTR::wxBSTR(BSTR bstr, bool copy) wxBSTR& wxBSTR::operator=(const wxBSTR& wxbstr) { - if ( wxbstr != *this ) + if ( this != &wxbstr ) { Free(); m_bstr = wxbstr.GetCopy(); From 684f0146f12271aaf9e48ecaa90e5ce3e4b1188c Mon Sep 17 00:00:00 2001 From: PB Date: Fri, 23 Jun 2017 17:05:03 +0200 Subject: [PATCH 4/9] Revert "Introduces wxBSTR, an RAII wrapper for MSW BSTR type. wxBSTR also supersedes wxBasicString." This reverts commit db22c91d44048c1eefebf90ef23554e68fc213fc. --- include/wx/msw/ole/activex.h | 2 +- include/wx/msw/ole/oleutils.h | 73 +++++++++++------------------------ src/msw/mediactrl_am.cpp | 6 +-- src/msw/mediactrl_wmp10.cpp | 30 +++++++------- src/msw/ole/access.cpp | 26 ++++++++----- src/msw/ole/automtn.cpp | 6 +-- src/msw/ole/oleutils.cpp | 62 ++++++++++------------------- src/msw/webview_ie.cpp | 55 +++++++++++++------------- 8 files changed, 111 insertions(+), 149 deletions(-) diff --git a/include/wx/msw/ole/activex.h b/include/wx/msw/ole/activex.h index 8ecd647208..1518671eb8 100644 --- a/include/wx/msw/ole/activex.h +++ b/include/wx/msw/ole/activex.h @@ -21,7 +21,7 @@ // wx includes //--------------------------------------------------------------------------- -#include "wx/msw/ole/oleutils.h" +#include "wx/msw/ole/oleutils.h" // wxBasicString &c #include "wx/msw/ole/uuid.h" #include "wx/window.h" #include "wx/variant.h" diff --git a/include/wx/msw/ole/oleutils.h b/include/wx/msw/ole/oleutils.h index 8e50251617..1dab81b18a 100644 --- a/include/wx/msw/ole/oleutils.h +++ b/include/wx/msw/ole/oleutils.h @@ -61,6 +61,29 @@ inline void wxOleUninitialize() ::OleUninitialize(); } +// wrapper around BSTR type (by Vadim Zeitlin) + +class WXDLLIMPEXP_CORE wxBasicString +{ +public: + // ctors & dtor + wxBasicString(const wxString& str); + wxBasicString(const wxBasicString& bstr); + ~wxBasicString(); + + wxBasicString& operator=(const wxBasicString& bstr); + + // accessors + // just get the string + operator BSTR() const { return m_bstrBuf; } + // retrieve a copy of our string - caller must SysFreeString() it later! + BSTR Get() const { return SysAllocString(m_bstrBuf); } + +private: + // actual string + BSTR m_bstrBuf; +}; + #if wxUSE_VARIANT // Convert variants class WXDLLIMPEXP_FWD_BASE wxVariant; @@ -172,56 +195,6 @@ WXDLLIMPEXP_CORE BSTR wxConvertStringToOle(const wxString& str); // Convert string from BSTR to wxString WXDLLIMPEXP_CORE wxString wxConvertStringFromOle(BSTR bStr); -// A thin RAII wrapper for BSTR, which can also create BSTR -// from wxString as well as return the owned BSTR as wxString. -// Unlike _b_str_t, wxBSTR is NOT reference counted. -class WXDLLIMPEXP_CORE wxBSTR -{ -public: - // Creates with the owned BSTR set to NULL - wxBSTR() : m_bstr(NULL) {} - - // If copy is true then a copy of bstr is created, - // if not then ownership of bstr is taken. - wxBSTR(BSTR bstr, bool copy); - - // Creates the owned BSTR from wxString str - wxBSTR(const wxString& str) : m_bstr(::SysAllocString(str.wc_str(*wxConvCurrent))) {} - - // Creates a copy of BSTR owned by wxbstr - wxBSTR(const wxBSTR& wxbstr) : m_bstr(wxbstr.GetCopy()) {} - - // Frees the owned BSTR - ~wxBSTR() { Free(); } - - // Creates a copy of the BSTR owned by wxbstr - wxBSTR& operator=(const wxBSTR& wxbstr); - - // Takes ownership of bstr - void Attach(BSTR bstr); - - // Returns the owned BSTR and gives up its ownership - BSTR Detach(); - - // Frees the owned BSTR - void Free(); - - // Returns the owned BSTR while keeping its ownership - BSTR GetBSTR() const { return m_bstr; } - operator BSTR() const { return GetBSTR(); } - - // Returns the address of owned BSTR - BSTR* GetAddress() { return &m_bstr; } - - // Returns a copy of owned BSTR - BSTR GetCopy() const { return ::SysAllocString(m_bstr); } - - // Returns the owned BSTR converted to wxString - wxString GetwxString() const { return wxConvertStringFromOle(m_bstr); } -private: - BSTR m_bstr; -}; - #else // !wxUSE_OLE // ---------------------------------------------------------------------------- diff --git a/src/msw/mediactrl_am.cpp b/src/msw/mediactrl_am.cpp index 43cb9c197a..6821e423da 100644 --- a/src/msw/mediactrl_am.cpp +++ b/src/msw/mediactrl_am.cpp @@ -1127,7 +1127,7 @@ bool wxAMMediaBackend::Load(const wxURI& location, const wxURI& proxy) if(pPlay) { pPlay->put_UseHTTPProxy(VARIANT_TRUE); - pPlay->put_HTTPProxyHost(wxBSTR(proxy.GetServer()).Detach()); + pPlay->put_HTTPProxyHost(wxBasicString(proxy.GetServer()).Get()); pPlay->put_HTTPProxyPort(wxAtoi(proxy.GetPort())); pPlay->Release(); } @@ -1150,9 +1150,9 @@ bool wxAMMediaBackend::DoLoad(const wxString& location) // the docs say its async and put_FileName is not - // but in practice they both seem to be async anyway if(GetMP()) - hr = GetMP()->Open( wxBSTR(location).Detach() ); + hr = GetMP()->Open( wxBasicString(location).Get() ); else - hr = GetAM()->put_FileName( wxBSTR(location).Detach() ); + hr = GetAM()->put_FileName( wxBasicString(location).Get() ); if(FAILED(hr)) { diff --git a/src/msw/mediactrl_wmp10.cpp b/src/msw/mediactrl_wmp10.cpp index b8f85c723d..533abf3c76 100644 --- a/src/msw/mediactrl_wmp10.cpp +++ b/src/msw/mediactrl_wmp10.cpp @@ -905,21 +905,21 @@ bool wxWMP10MediaBackend::Load(const wxURI& location, { long lOldSetting; if( pWMPNetwork->getProxySettings( - wxBSTR(location.GetScheme()).Detach(), &lOldSetting + wxBasicString(location.GetScheme()).Get(), &lOldSetting ) == 0 && pWMPNetwork->setProxySettings( - wxBSTR(location.GetScheme()).Detach(), // protocol + wxBasicString(location.GetScheme()).Get(), // protocol 2) == 0) // 2 == manually specify { BSTR bsOldName = NULL; long lOldPort = 0; pWMPNetwork->getProxyName( - wxBSTR(location.GetScheme()).Detach(), + wxBasicString(location.GetScheme()).Get(), &bsOldName); pWMPNetwork->getProxyPort( - wxBSTR(location.GetScheme()).Detach(), + wxBasicString(location.GetScheme()).Get(), &lOldPort); long lPort; @@ -936,11 +936,11 @@ bool wxWMP10MediaBackend::Load(const wxURI& location, } if( pWMPNetwork->setProxyName( - wxBSTR(location.GetScheme()).Detach(), // proto - wxBSTR(server).Detach() ) == 0 && + wxBasicString(location.GetScheme()).Get(), // proto + wxBasicString(server).Get() ) == 0 && pWMPNetwork->setProxyPort( - wxBSTR(location.GetScheme()).Detach(), // proto + wxBasicString(location.GetScheme()).Get(), // proto lPort ) == 0 ) @@ -948,16 +948,16 @@ bool wxWMP10MediaBackend::Load(const wxURI& location, bOK = DoLoad(location.BuildURI()); pWMPNetwork->setProxySettings( - wxBSTR(location.GetScheme()).Detach(), // protocol + wxBasicString(location.GetScheme()).Get(), // protocol lOldSetting); if(bsOldName) pWMPNetwork->setProxyName( - wxBSTR(location.GetScheme()).Detach(), // protocol + wxBasicString(location.GetScheme()).Get(), // protocol bsOldName); if(lOldPort) pWMPNetwork->setProxyPort( - wxBSTR(location.GetScheme()).Detach(), // protocol + wxBasicString(location.GetScheme()).Get(), // protocol lOldPort); pWMPNetwork->Release(); @@ -997,7 +997,7 @@ bool wxWMP10MediaBackend::DoLoad(const wxString& location) { IWMPMedia* pWMPMedia; - if( (hr = pWMPCore3->newMedia(wxBSTR(location).Detach(), + if( (hr = pWMPCore3->newMedia(wxBasicString(location).Get(), &pWMPMedia)) == 0) { // this (get_duration) will actually FAIL, but it will work. @@ -1012,7 +1012,7 @@ bool wxWMP10MediaBackend::DoLoad(const wxString& location) #endif { // just load it the "normal" way - hr = m_pWMPPlayer->put_URL( wxBSTR(location).Detach() ); + hr = m_pWMPPlayer->put_URL( wxBasicString(location).Get() ); } if(FAILED(hr)) @@ -1061,12 +1061,12 @@ bool wxWMP10MediaBackend::ShowPlayerControls(wxMediaCtrlPlayerControls flags) if(!flags) { m_pWMPPlayer->put_enabled(VARIANT_FALSE); - m_pWMPPlayer->put_uiMode(wxBSTR(wxT("none")).Detach()); + m_pWMPPlayer->put_uiMode(wxBasicString(wxT("none")).Get()); } else { // TODO: use "custom"? (note that CE only supports none/full) - m_pWMPPlayer->put_uiMode(wxBSTR(wxT("full")).Detach()); + m_pWMPPlayer->put_uiMode(wxBasicString(wxT("full")).Get()); m_pWMPPlayer->put_enabled(VARIANT_TRUE); } @@ -1358,7 +1358,7 @@ wxLongLong wxWMP10MediaBackend::GetDownloadTotal() if(m_pWMPPlayer->get_currentMedia(&pWMPMedia) == 0) { BSTR bsOut; - pWMPMedia->getItemInfo(wxBSTR(wxT("FileSize")).Detach(), + pWMPMedia->getItemInfo(wxBasicString(wxT("FileSize")).Get(), &bsOut); wxString sFileSize = wxConvertStringFromOle(bsOut); diff --git a/src/msw/ole/access.cpp b/src/msw/ole/access.cpp index d8ce5753ea..7c5d3541d5 100644 --- a/src/msw/ole/access.cpp +++ b/src/msw/ole/access.cpp @@ -937,8 +937,9 @@ STDMETHODIMP wxIAccessible::get_accDefaultAction ( VARIANT varID, BSTR* pszDefau return S_FALSE; } else - { - * pszDefaultAction = wxBSTR(defaultAction).Detach(); + { + wxBasicString basicString(defaultAction); + * pszDefaultAction = basicString.Get(); return S_OK; } } @@ -997,8 +998,9 @@ STDMETHODIMP wxIAccessible::get_accDescription ( VARIANT varID, BSTR* pszDescrip return S_FALSE; } else - { - * pszDescription = wxBSTR(description).Detach(); + { + wxBasicString basicString(description); + * pszDescription = basicString.Get(); return S_OK; } } @@ -1057,8 +1059,9 @@ STDMETHODIMP wxIAccessible::get_accHelp ( VARIANT varID, BSTR* pszHelp) return S_FALSE; } else - { - * pszHelp = wxBSTR(helpString).Detach(); + { + wxBasicString basicString(helpString); + * pszHelp = basicString.Get(); return S_OK; } } @@ -1168,7 +1171,8 @@ STDMETHODIMP wxIAccessible::get_accKeyboardShortcut ( VARIANT varID, BSTR* pszKe } else { - * pszKeyboardShortcut = wxBSTR(keyboardShortcut).Detach(); + wxBasicString basicString(keyboardShortcut); + * pszKeyboardShortcut = basicString.Get(); return S_OK; } } @@ -1230,8 +1234,9 @@ STDMETHODIMP wxIAccessible::get_accName ( VARIANT varID, BSTR* pszName) return S_FALSE; } else - { - *pszName = wxBSTR(name).Detach(); + { + wxBasicString basicString(name); + *pszName = basicString.Get(); } return S_OK; } @@ -1411,7 +1416,8 @@ STDMETHODIMP wxIAccessible::get_accValue ( VARIANT varID, BSTR* pszValue) } else { - * pszValue = wxBSTR(strValue).Detach(); + wxBasicString basicString(strValue); + * pszValue = basicString.Get(); return S_OK; } } diff --git a/src/msw/ole/automtn.cpp b/src/msw/ole/automtn.cpp index e77cdbb38f..fdbd9f4d4b 100644 --- a/src/msw/ole/automtn.cpp +++ b/src/msw/ole/automtn.cpp @@ -130,7 +130,7 @@ bool wxAutomationObject::Invoke(const wxString& member, int action, } int namedArgStringCount = namedArgCount + 1; - wxVector argNames(namedArgStringCount, wxString()); + wxVector argNames(namedArgStringCount, wxString()); argNames[0] = member; // Note that arguments are specified in reverse order @@ -156,7 +156,7 @@ bool wxAutomationObject::Invoke(const wxString& member, int action, // Get the IDs for the member and its arguments. GetIDsOfNames expects the // member name as the first name, followed by argument names (if any). hr = ((IDispatch*)m_dispatchPtr)->GetIDsOfNames(IID_NULL, - // We rely on the fact that wxBSTR is + // We rely on the fact that wxBasicString is // just BSTR with some methods here. reinterpret_cast(&argNames[0]), 1 + namedArgCount, m_lcid, &dispIds[0]); @@ -499,7 +499,7 @@ namespace HRESULT wxCLSIDFromProgID(const wxString& progId, CLSID& clsId) { - HRESULT hr = CLSIDFromProgID(wxBSTR(progId), &clsId); + HRESULT hr = CLSIDFromProgID(wxBasicString(progId), &clsId); if ( FAILED(hr) ) { wxLogSysError(hr, _("Failed to find CLSID of \"%s\""), progId); diff --git a/src/msw/ole/oleutils.cpp b/src/msw/ole/oleutils.cpp index 6b9b37437d..67c1477753 100644 --- a/src/msw/ole/oleutils.cpp +++ b/src/msw/ole/oleutils.cpp @@ -38,7 +38,7 @@ WXDLLEXPORT BSTR wxConvertStringToOle(const wxString& str) { - return wxBSTR(str).Detach(); + return wxBasicString(str).Get(); } WXDLLEXPORT wxString wxConvertStringFromOle(BSTR bStr) @@ -68,48 +68,28 @@ WXDLLEXPORT wxString wxConvertStringFromOle(BSTR bStr) } // ---------------------------------------------------------------------------- -// wxBSTR +// wxBasicString // ---------------------------------------------------------------------------- -wxBSTR::wxBSTR(BSTR bstr, bool copy) -{ - if ( copy ) - m_bstr = ::SysAllocString(bstr); - else - m_bstr = bstr; -} - -wxBSTR& wxBSTR::operator=(const wxBSTR& wxbstr) -{ - if ( this != &wxbstr ) - { - Free(); - m_bstr = wxbstr.GetCopy(); - } - - return *this; -} - -void wxBSTR::Attach(BSTR bstr) -{ - wxCHECK_RET(m_bstr != bstr, wxS("Attaching already attached BSTR!")); - - Free(); - m_bstr = bstr; -} - -BSTR wxBSTR::Detach() -{ - BSTR bstr = m_bstr; - - m_bstr = NULL; - return bstr; -} - -void wxBSTR::Free() -{ - ::SysFreeString(m_bstr); - m_bstr = NULL; +wxBasicString::wxBasicString(const wxString& str) +{ + m_bstrBuf = SysAllocString(str.wc_str(*wxConvCurrent)); +} + +wxBasicString::wxBasicString(const wxBasicString& src) +{ + m_bstrBuf = src.Get(); +} + +wxBasicString& wxBasicString::operator=(const wxBasicString& src) +{ + SysReAllocString(&m_bstrBuf, src); + return *this; +} + +wxBasicString::~wxBasicString() +{ + SysFreeString(m_bstrBuf); } diff --git a/src/msw/webview_ie.cpp b/src/msw/webview_ie.cpp index 9de2345401..691a90aeb0 100644 --- a/src/msw/webview_ie.cpp +++ b/src/msw/webview_ie.cpp @@ -231,9 +231,9 @@ wxString wxWebViewIE::GetPageSource() const hr = bodyTag->get_parentElement(&htmlTag); if(SUCCEEDED(hr)) { - wxBSTR wxbstr; - if ( htmlTag->get_outerHTML(wxbstr.GetAddress()) == S_OK ) - source = wxbstr.GetwxString(); + BSTR bstr; + if ( htmlTag->get_outerHTML(&bstr) == S_OK ) + source = wxString(bstr); } } return source; @@ -576,9 +576,9 @@ wxString wxWebViewIE::GetCurrentTitle() const wxString s; if(document) { - wxBSTR title; - if ( document->get_nameProp(title.GetAddress()) == S_OK ) - s = title.GetwxString(); + BSTR title; + if ( document->get_nameProp(&title) == S_OK ) + s = title; } return s; @@ -705,10 +705,10 @@ bool wxWebViewIE::IsEditable() const if(document) { - wxBSTR mode; - if ( document->get_designMode(mode.GetAddress()) == S_OK ) + BSTR mode; + if ( document->get_designMode(&mode) == S_OK ) { - if ( mode.GetwxString() == "On" ) + if ( wxString(mode) == "On" ) return true; } } @@ -731,9 +731,9 @@ bool wxWebViewIE::HasSelection() const HRESULT hr = document->get_selection(&selection); if(SUCCEEDED(hr)) { - wxBSTR type; - if ( selection->get_type(type.GetAddress()) == S_OK ) - sel = type.GetwxString(); + BSTR type; + if ( selection->get_type(&type) == S_OK ) + sel = wxString(type); } return sel != "None"; } @@ -767,9 +767,9 @@ wxString wxWebViewIE::GetSelectedText() const hr = disrange->QueryInterface(IID_IHTMLTxtRange, (void**)&range); if(SUCCEEDED(hr)) { - wxBSTR text; - if ( range->get_text(text.GetAddress()) == S_OK ) - selected = text.GetwxString(); + BSTR text; + if ( range->get_text(&text) == S_OK ) + selected = wxString(text); } } } @@ -800,9 +800,9 @@ wxString wxWebViewIE::GetSelectedSource() const hr = disrange->QueryInterface(IID_IHTMLTxtRange, (void**)&range); if(SUCCEEDED(hr)) { - wxBSTR text; - if ( range->get_htmlText(text.GetAddress()) == S_OK ) - selected = text.GetwxString(); + BSTR text; + if ( range->get_htmlText(&text) == S_OK ) + selected = wxString(text); } } } @@ -841,9 +841,9 @@ wxString wxWebViewIE::GetPageText() const HRESULT hr = document->get_body(&body); if(SUCCEEDED(hr)) { - wxBSTR out; - if ( body->get_innerText(out.GetAddress()) == S_OK ) - text = out.GetwxString(); + BSTR out; + if ( body->get_innerText(&out) == S_OK ) + text = wxString(out); } return text; } @@ -950,7 +950,7 @@ wxCOMPtr wxWebViewIE::GetDocument() const bool wxWebViewIE::IsElementVisible(wxCOMPtr elm) { wxCOMPtr elm1 = elm; - wxBSTR tmp_bstr; + BSTR tmp_bstr; bool is_visible = true; //This method is not perfect but it does discover most of the hidden elements. //so if a better solution is found, then please do improve. @@ -963,13 +963,13 @@ bool wxWebViewIE::IsElementVisible(wxCOMPtr elm) if(SUCCEEDED(elm2->get_currentStyle(&style))) { //Check if the object has the style display:none. - if((style->get_display(tmp_bstr.GetAddress()) != S_OK) || + if((style->get_display(&tmp_bstr) != S_OK) || (tmp_bstr != NULL && (wxCRT_StricmpW(tmp_bstr, L"none") == 0))) { is_visible = false; } //Check if the object has the style visibility:hidden. - if((is_visible && (style->get_visibility(tmp_bstr.GetAddress()) != S_OK)) || + if((is_visible && (style->get_visibility(&tmp_bstr) != S_OK)) || (tmp_bstr != NULL && wxCRT_StricmpW(tmp_bstr, L"hidden") == 0)) { is_visible = false; @@ -1007,8 +1007,8 @@ void wxWebViewIE::FindInternal(const wxString& text, int flags, int internal_fla if(SUCCEEDED(document->QueryInterface(wxIID_IMarkupContainer, (void **)&pIMC))) { wxCOMPtr ptrBegin, ptrEnd; - wxBSTR attr_bstr(L"style=\"background-color:#ffff00\""); - wxBSTR text_bstr(text.wc_str()); + BSTR attr_bstr = SysAllocString(L"style=\"background-color:#ffff00\""); + BSTR text_bstr = SysAllocString(text.wc_str()); pIMS->CreateMarkupPointer(&ptrBegin); pIMS->CreateMarkupPointer(&ptrEnd); @@ -1064,6 +1064,9 @@ void wxWebViewIE::FindInternal(const wxString& text, int flags, int internal_fla } ptrBegin->MoveToPointer(ptrEnd); } + //Clean up. + SysFreeString(text_bstr); + SysFreeString(attr_bstr); } } } From 60563ce0cef1470431507d0d0f6c31b95611fd93 Mon Sep 17 00:00:00 2001 From: PB Date: Fri, 23 Jun 2017 17:06:56 +0200 Subject: [PATCH 5/9] Modify wxBasicString so it is better suited to work as an RAII wrapper for BSTR. --- include/wx/msw/ole/oleutils.h | 45 ++++++++++++++++++++++++------ src/msw/ole/oleutils.cpp | 52 +++++++++++++++++++++++++++++------ 2 files changed, 80 insertions(+), 17 deletions(-) diff --git a/include/wx/msw/ole/oleutils.h b/include/wx/msw/ole/oleutils.h index 1dab81b18a..2477c25b95 100644 --- a/include/wx/msw/ole/oleutils.h +++ b/include/wx/msw/ole/oleutils.h @@ -66,18 +66,45 @@ inline void wxOleUninitialize() class WXDLLIMPEXP_CORE wxBasicString { public: - // ctors & dtor - wxBasicString(const wxString& str); - wxBasicString(const wxBasicString& bstr); - ~wxBasicString(); - wxBasicString& operator=(const wxBasicString& bstr); + // Takes over the ownership of bstr + wxBasicString(BSTR bstr = NULL); + + // Constructs the BSTR from wxString + wxBasicString(const wxString& str); + + // Creates a copy of the BSTR owned by bstr + wxBasicString(const wxBasicString& bstr); + + // Frees the owned BSTR + ~wxBasicString(); + - // accessors - // just get the string + // Sets its BSTR to a copy of the BSTR owned by bstr + wxBasicString& operator=(const wxBasicString& bstr); + + // Creates its BSTR from wxString + wxBasicString& operator=(const wxString& str); + + // Takes over the ownership of bstr + wxBasicString& operator=(BSTR bstr); + + // Returns the owned BSTR and gives up its ownership + BSTR Detach(); + + /// Returns the owned BSTR while keeping its ownership operator BSTR() const { return m_bstrBuf; } - // retrieve a copy of our string - caller must SysFreeString() it later! - BSTR Get() const { return SysAllocString(m_bstrBuf); } + + // Returns the address of the owned BSTR + operator BSTR*() { return &m_bstrBuf; } + + // Returns a copy of the owned BSTR + BSTR Copy() const { return SysAllocString(m_bstrBuf); } + + + // retrieve a copy of our string - caller must SysFreeString() it later! + wxDEPRECATED_MSG("use Copy() instead") + BSTR Get() const { return Copy(); } private: // actual string diff --git a/src/msw/ole/oleutils.cpp b/src/msw/ole/oleutils.cpp index 67c1477753..b56a1bbf07 100644 --- a/src/msw/ole/oleutils.cpp +++ b/src/msw/ole/oleutils.cpp @@ -71,20 +71,19 @@ WXDLLEXPORT wxString wxConvertStringFromOle(BSTR bStr) // wxBasicString // ---------------------------------------------------------------------------- +wxBasicString::wxBasicString(BSTR bstr) +{ + m_bstrBuf = bstr; +} + wxBasicString::wxBasicString(const wxString& str) { m_bstrBuf = SysAllocString(str.wc_str(*wxConvCurrent)); } -wxBasicString::wxBasicString(const wxBasicString& src) +wxBasicString::wxBasicString(const wxBasicString& bstr) { - m_bstrBuf = src.Get(); -} - -wxBasicString& wxBasicString::operator=(const wxBasicString& src) -{ - SysReAllocString(&m_bstrBuf, src); - return *this; + m_bstrBuf = bstr.Copy(); } wxBasicString::~wxBasicString() @@ -92,6 +91,43 @@ wxBasicString::~wxBasicString() SysFreeString(m_bstrBuf); } +wxBasicString& wxBasicString::operator=(const wxBasicString& src) +{ + if ( this != &src ) + { + SysFreeString(m_bstrBuf); + m_bstrBuf = src.Copy(); + } + + return *this; +} + +wxBasicString& wxBasicString::operator=(const wxString& str) +{ + SysFreeString(m_bstrBuf); + m_bstrBuf = ::SysAllocString(str.wc_str(*wxConvCurrent)); + + return *this; +} + +wxBasicString& wxBasicString::operator=(BSTR bstr) +{ + wxCHECK_MSG(m_bstrBuf != bstr, *this, wxS("Attempting to attach already attached BSTR!")); + + SysFreeString(m_bstrBuf); + m_bstrBuf = bstr; + + return *this; +} + +BSTR wxBasicString::Detach() +{ + BSTR bstr = m_bstrBuf; + + m_bstrBuf = NULL; + + return bstr; +} // ---------------------------------------------------------------------------- // Convert variants From 9030d9390a02aa69681b23c04edab12465b3b027 Mon Sep 17 00:00:00 2001 From: PB Date: Fri, 23 Jun 2017 17:32:03 +0200 Subject: [PATCH 6/9] Replace calls to deprecated wxBasicString::Get() with calls to wxBasicString::Detach() which is not deprecated but also more efficient in cases like these. --- src/msw/mediactrl_am.cpp | 6 +++--- src/msw/mediactrl_wmp10.cpp | 24 ++++++++++++------------ src/msw/ole/access.cpp | 18 ++++++------------ src/msw/ole/oleutils.cpp | 2 +- 4 files changed, 22 insertions(+), 28 deletions(-) diff --git a/src/msw/mediactrl_am.cpp b/src/msw/mediactrl_am.cpp index 6821e423da..eb181093fe 100644 --- a/src/msw/mediactrl_am.cpp +++ b/src/msw/mediactrl_am.cpp @@ -1127,7 +1127,7 @@ bool wxAMMediaBackend::Load(const wxURI& location, const wxURI& proxy) if(pPlay) { pPlay->put_UseHTTPProxy(VARIANT_TRUE); - pPlay->put_HTTPProxyHost(wxBasicString(proxy.GetServer()).Get()); + pPlay->put_HTTPProxyHost(wxBasicString(proxy.GetServer()).Detach()); pPlay->put_HTTPProxyPort(wxAtoi(proxy.GetPort())); pPlay->Release(); } @@ -1150,9 +1150,9 @@ bool wxAMMediaBackend::DoLoad(const wxString& location) // the docs say its async and put_FileName is not - // but in practice they both seem to be async anyway if(GetMP()) - hr = GetMP()->Open( wxBasicString(location).Get() ); + hr = GetMP()->Open( wxBasicString(location).Detach() ); else - hr = GetAM()->put_FileName( wxBasicString(location).Get() ); + hr = GetAM()->put_FileName( wxBasicString(location).Detach() ); if(FAILED(hr)) { diff --git a/src/msw/mediactrl_wmp10.cpp b/src/msw/mediactrl_wmp10.cpp index 533abf3c76..b9d145d4e7 100644 --- a/src/msw/mediactrl_wmp10.cpp +++ b/src/msw/mediactrl_wmp10.cpp @@ -909,17 +909,17 @@ bool wxWMP10MediaBackend::Load(const wxURI& location, ) == 0 && pWMPNetwork->setProxySettings( - wxBasicString(location.GetScheme()).Get(), // protocol + wxBasicString(location.GetScheme()).Detach(), // protocol 2) == 0) // 2 == manually specify { BSTR bsOldName = NULL; long lOldPort = 0; pWMPNetwork->getProxyName( - wxBasicString(location.GetScheme()).Get(), + wxBasicString(location.GetScheme()).Detach(), &bsOldName); pWMPNetwork->getProxyPort( - wxBasicString(location.GetScheme()).Get(), + wxBasicString(location.GetScheme()).Detach(), &lOldPort); long lPort; @@ -936,11 +936,11 @@ bool wxWMP10MediaBackend::Load(const wxURI& location, } if( pWMPNetwork->setProxyName( - wxBasicString(location.GetScheme()).Get(), // proto + wxBasicString(location.GetScheme()).Detach(), // proto wxBasicString(server).Get() ) == 0 && pWMPNetwork->setProxyPort( - wxBasicString(location.GetScheme()).Get(), // proto + wxBasicString(location.GetScheme()).Detach(), // proto lPort ) == 0 ) @@ -948,16 +948,16 @@ bool wxWMP10MediaBackend::Load(const wxURI& location, bOK = DoLoad(location.BuildURI()); pWMPNetwork->setProxySettings( - wxBasicString(location.GetScheme()).Get(), // protocol + wxBasicString(location.GetScheme()).Detach(), // protocol lOldSetting); if(bsOldName) pWMPNetwork->setProxyName( - wxBasicString(location.GetScheme()).Get(), // protocol + wxBasicString(location.GetScheme()).Detach(), // protocol bsOldName); if(lOldPort) pWMPNetwork->setProxyPort( - wxBasicString(location.GetScheme()).Get(), // protocol + wxBasicString(location.GetScheme()).Detach(), // protocol lOldPort); pWMPNetwork->Release(); @@ -1012,7 +1012,7 @@ bool wxWMP10MediaBackend::DoLoad(const wxString& location) #endif { // just load it the "normal" way - hr = m_pWMPPlayer->put_URL( wxBasicString(location).Get() ); + hr = m_pWMPPlayer->put_URL( wxBasicString(location).Detach() ); } if(FAILED(hr)) @@ -1061,12 +1061,12 @@ bool wxWMP10MediaBackend::ShowPlayerControls(wxMediaCtrlPlayerControls flags) if(!flags) { m_pWMPPlayer->put_enabled(VARIANT_FALSE); - m_pWMPPlayer->put_uiMode(wxBasicString(wxT("none")).Get()); + m_pWMPPlayer->put_uiMode(wxBasicString(wxT("none")).Detach()); } else { // TODO: use "custom"? (note that CE only supports none/full) - m_pWMPPlayer->put_uiMode(wxBasicString(wxT("full")).Get()); + m_pWMPPlayer->put_uiMode(wxBasicString(wxT("full")).Detach()); m_pWMPPlayer->put_enabled(VARIANT_TRUE); } @@ -1358,7 +1358,7 @@ wxLongLong wxWMP10MediaBackend::GetDownloadTotal() if(m_pWMPPlayer->get_currentMedia(&pWMPMedia) == 0) { BSTR bsOut; - pWMPMedia->getItemInfo(wxBasicString(wxT("FileSize")).Get(), + pWMPMedia->getItemInfo(wxBasicString(wxT("FileSize")).Detach(), &bsOut); wxString sFileSize = wxConvertStringFromOle(bsOut); diff --git a/src/msw/ole/access.cpp b/src/msw/ole/access.cpp index 7c5d3541d5..6dd3a7baa5 100644 --- a/src/msw/ole/access.cpp +++ b/src/msw/ole/access.cpp @@ -938,8 +938,7 @@ STDMETHODIMP wxIAccessible::get_accDefaultAction ( VARIANT varID, BSTR* pszDefau } else { - wxBasicString basicString(defaultAction); - * pszDefaultAction = basicString.Get(); + * pszDefaultAction = wxBasicString(defaultAction).Detach(); return S_OK; } } @@ -999,8 +998,7 @@ STDMETHODIMP wxIAccessible::get_accDescription ( VARIANT varID, BSTR* pszDescrip } else { - wxBasicString basicString(description); - * pszDescription = basicString.Get(); + * pszDescription = wxBasicString(description).Detach(); return S_OK; } } @@ -1060,8 +1058,7 @@ STDMETHODIMP wxIAccessible::get_accHelp ( VARIANT varID, BSTR* pszHelp) } else { - wxBasicString basicString(helpString); - * pszHelp = basicString.Get(); + * pszHelp = wxBasicString(helpString).Detach(); return S_OK; } } @@ -1171,8 +1168,7 @@ STDMETHODIMP wxIAccessible::get_accKeyboardShortcut ( VARIANT varID, BSTR* pszKe } else { - wxBasicString basicString(keyboardShortcut); - * pszKeyboardShortcut = basicString.Get(); + * pszKeyboardShortcut = wxBasicString(keyboardShortcut).Detach(); return S_OK; } } @@ -1235,8 +1231,7 @@ STDMETHODIMP wxIAccessible::get_accName ( VARIANT varID, BSTR* pszName) } else { - wxBasicString basicString(name); - *pszName = basicString.Get(); + *pszName = wxBasicString(name).Detach(); } return S_OK; } @@ -1416,8 +1411,7 @@ STDMETHODIMP wxIAccessible::get_accValue ( VARIANT varID, BSTR* pszValue) } else { - wxBasicString basicString(strValue); - * pszValue = basicString.Get(); + * pszValue = wxBasicString(strValue).Detach(); return S_OK; } } diff --git a/src/msw/ole/oleutils.cpp b/src/msw/ole/oleutils.cpp index b56a1bbf07..5a24918fb6 100644 --- a/src/msw/ole/oleutils.cpp +++ b/src/msw/ole/oleutils.cpp @@ -38,7 +38,7 @@ WXDLLEXPORT BSTR wxConvertStringToOle(const wxString& str) { - return wxBasicString(str).Get(); + return wxBasicString(str).Detach(); } WXDLLEXPORT wxString wxConvertStringFromOle(BSTR bStr) From 564c0aafa541b8783ac26ebe3c260667b759d099 Mon Sep 17 00:00:00 2001 From: PB Date: Fri, 23 Jun 2017 18:16:44 +0200 Subject: [PATCH 7/9] Use wxBasicString instead of BSTR in wxWebViewIE to prevent memory leaks. --- src/msw/webview_ie.cpp | 57 +++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/src/msw/webview_ie.cpp b/src/msw/webview_ie.cpp index 691a90aeb0..18635ec7a6 100644 --- a/src/msw/webview_ie.cpp +++ b/src/msw/webview_ie.cpp @@ -231,9 +231,9 @@ wxString wxWebViewIE::GetPageSource() const hr = bodyTag->get_parentElement(&htmlTag); if(SUCCEEDED(hr)) { - BSTR bstr; - if ( htmlTag->get_outerHTML(&bstr) == S_OK ) - source = wxString(bstr); + wxBasicString bstr; + if ( htmlTag->get_outerHTML(bstr) == S_OK ) + source = bstr; } } return source; @@ -576,8 +576,8 @@ wxString wxWebViewIE::GetCurrentTitle() const wxString s; if(document) { - BSTR title; - if ( document->get_nameProp(&title) == S_OK ) + wxBasicString title; + if ( document->get_nameProp(title) == S_OK ) s = title; } @@ -705,8 +705,8 @@ bool wxWebViewIE::IsEditable() const if(document) { - BSTR mode; - if ( document->get_designMode(&mode) == S_OK ) + wxBasicString mode; + if ( document->get_designMode(mode) == S_OK ) { if ( wxString(mode) == "On" ) return true; @@ -731,9 +731,9 @@ bool wxWebViewIE::HasSelection() const HRESULT hr = document->get_selection(&selection); if(SUCCEEDED(hr)) { - BSTR type; - if ( selection->get_type(&type) == S_OK ) - sel = wxString(type); + wxBasicString type; + if ( selection->get_type(type) == S_OK ) + sel = type; } return sel != "None"; } @@ -767,9 +767,9 @@ wxString wxWebViewIE::GetSelectedText() const hr = disrange->QueryInterface(IID_IHTMLTxtRange, (void**)&range); if(SUCCEEDED(hr)) { - BSTR text; - if ( range->get_text(&text) == S_OK ) - selected = wxString(text); + wxBasicString text; + if ( range->get_text(text) == S_OK ) + selected = text; } } } @@ -800,9 +800,9 @@ wxString wxWebViewIE::GetSelectedSource() const hr = disrange->QueryInterface(IID_IHTMLTxtRange, (void**)&range); if(SUCCEEDED(hr)) { - BSTR text; - if ( range->get_htmlText(&text) == S_OK ) - selected = wxString(text); + wxBasicString text; + if ( range->get_htmlText(text) == S_OK ) + selected = text; } } } @@ -841,9 +841,9 @@ wxString wxWebViewIE::GetPageText() const HRESULT hr = document->get_body(&body); if(SUCCEEDED(hr)) { - BSTR out; - if ( body->get_innerText(&out) == S_OK ) - text = wxString(out); + wxBasicString out; + if ( body->get_innerText(out) == S_OK ) + text = out; } return text; } @@ -950,7 +950,8 @@ wxCOMPtr wxWebViewIE::GetDocument() const bool wxWebViewIE::IsElementVisible(wxCOMPtr elm) { wxCOMPtr elm1 = elm; - BSTR tmp_bstr; + wxBasicString display_bstr; + wxBasicString visibility_bstr; bool is_visible = true; //This method is not perfect but it does discover most of the hidden elements. //so if a better solution is found, then please do improve. @@ -963,14 +964,14 @@ bool wxWebViewIE::IsElementVisible(wxCOMPtr elm) if(SUCCEEDED(elm2->get_currentStyle(&style))) { //Check if the object has the style display:none. - if((style->get_display(&tmp_bstr) != S_OK) || - (tmp_bstr != NULL && (wxCRT_StricmpW(tmp_bstr, L"none") == 0))) + if((style->get_display(display_bstr) != S_OK) || + ((BSTR)display_bstr != NULL && (wxCRT_StricmpW(display_bstr, L"none") == 0))) { is_visible = false; } //Check if the object has the style visibility:hidden. - if((is_visible && (style->get_visibility(&tmp_bstr) != S_OK)) || - (tmp_bstr != NULL && wxCRT_StricmpW(tmp_bstr, L"hidden") == 0)) + if((is_visible && (style->get_visibility(visibility_bstr) != S_OK)) || + ((BSTR)visibility_bstr != NULL && wxCRT_StricmpW(visibility_bstr, L"hidden") == 0)) { is_visible = false; } @@ -1007,8 +1008,9 @@ void wxWebViewIE::FindInternal(const wxString& text, int flags, int internal_fla if(SUCCEEDED(document->QueryInterface(wxIID_IMarkupContainer, (void **)&pIMC))) { wxCOMPtr ptrBegin, ptrEnd; - BSTR attr_bstr = SysAllocString(L"style=\"background-color:#ffff00\""); - BSTR text_bstr = SysAllocString(text.wc_str()); + wxBasicString attr_bstr(wxString("style=\"background-color:#ffff00\"")); + wxBasicString text_bstr(text.wc_str()); + pIMS->CreateMarkupPointer(&ptrBegin); pIMS->CreateMarkupPointer(&ptrEnd); @@ -1064,9 +1066,6 @@ void wxWebViewIE::FindInternal(const wxString& text, int flags, int internal_fla } ptrBegin->MoveToPointer(ptrEnd); } - //Clean up. - SysFreeString(text_bstr); - SysFreeString(attr_bstr); } } } From ca3f919da9a899b1734f0b11ce668efb13084111 Mon Sep 17 00:00:00 2001 From: PB Date: Sat, 1 Jul 2017 12:50:24 +0200 Subject: [PATCH 8/9] Make wxBasicString safer and easier to use as a BSTR RAII wrapper. --- include/wx/msw/ole/oleutils.h | 67 ++++++++++++++++++----------------- src/msw/ole/oleutils.cpp | 62 ++++++++++++++++++++------------ src/msw/webview_ie.cpp | 29 +++++++-------- 3 files changed, 88 insertions(+), 70 deletions(-) diff --git a/include/wx/msw/ole/oleutils.h b/include/wx/msw/ole/oleutils.h index 2477c25b95..d207eeaf78 100644 --- a/include/wx/msw/ole/oleutils.h +++ b/include/wx/msw/ole/oleutils.h @@ -65,47 +65,48 @@ inline void wxOleUninitialize() class WXDLLIMPEXP_CORE wxBasicString { -public: +public: + // Takes over the ownership of bstr + explicit wxBasicString(BSTR bstr = NULL); - // Takes over the ownership of bstr - wxBasicString(BSTR bstr = NULL); - - // Constructs the BSTR from wxString - wxBasicString(const wxString& str); - - // Creates a copy of the BSTR owned by bstr - wxBasicString(const wxBasicString& bstr); - - // Frees the owned BSTR - ~wxBasicString(); - + // Constructs the BSTR from wxString + wxBasicString(const wxString& str); - // Sets its BSTR to a copy of the BSTR owned by bstr - wxBasicString& operator=(const wxBasicString& bstr); + // Creates a copy of the BSTR owned by bstr + wxBasicString(const wxBasicString& bstr); - // Creates its BSTR from wxString - wxBasicString& operator=(const wxString& str); + // Frees the owned BSTR + ~wxBasicString(); + + // Returns the owned BSTR and gives up its ownership + BSTR Detach(); + + // Frees the owned BSTR + void Free(); + + // Returns a copy of the owned BSTR, + // the caller is responsible for freeing it + BSTR Copy() const { return SysAllocString(m_bstrBuf); } + + // Returns the address of the owned BSTR, not to be called + // when wxBasicString already contains a non-NULL BSTR + BSTR* ByRef(); + + // Sets its BSTR to a copy of the BSTR owned by bstr + wxBasicString& operator=(const wxBasicString& bstr); + + // Creates its BSTR from wxString + wxBasicString& operator=(const wxString& str); - // Takes over the ownership of bstr - wxBasicString& operator=(BSTR bstr); - - // Returns the owned BSTR and gives up its ownership - BSTR Detach(); + // Takes over the ownership of bstr + wxBasicString& operator=(BSTR bstr); /// Returns the owned BSTR while keeping its ownership operator BSTR() const { return m_bstrBuf; } - - // Returns the address of the owned BSTR - operator BSTR*() { return &m_bstrBuf; } - - // Returns a copy of the owned BSTR - BSTR Copy() const { return SysAllocString(m_bstrBuf); } - - - // retrieve a copy of our string - caller must SysFreeString() it later! - wxDEPRECATED_MSG("use Copy() instead") - BSTR Get() const { return Copy(); } + // retrieve a copy of our string - caller must SysFreeString() it later! + wxDEPRECATED_MSG("use Copy() instead") + BSTR Get() const { return Copy(); } private: // actual string BSTR m_bstrBuf; diff --git a/src/msw/ole/oleutils.cpp b/src/msw/ole/oleutils.cpp index 5a24918fb6..d1378fdecf 100644 --- a/src/msw/ole/oleutils.cpp +++ b/src/msw/ole/oleutils.cpp @@ -73,7 +73,7 @@ WXDLLEXPORT wxString wxConvertStringFromOle(BSTR bStr) wxBasicString::wxBasicString(BSTR bstr) { - m_bstrBuf = bstr; + m_bstrBuf = bstr; } wxBasicString::wxBasicString(const wxString& str) @@ -91,42 +91,58 @@ wxBasicString::~wxBasicString() SysFreeString(m_bstrBuf); } +BSTR wxBasicString::Detach() +{ + BSTR bstr = m_bstrBuf; + + m_bstrBuf = NULL; + + return bstr; +} + +void wxBasicString::Free() +{ + SysFreeString(m_bstrBuf); + m_bstrBuf = NULL; +} + +BSTR* wxBasicString::ByRef() +{ + wxASSERT_MSG(!m_bstrBuf, + wxS("Can't get direct access to initialized BSTR")); + return &m_bstrBuf; +} + wxBasicString& wxBasicString::operator=(const wxBasicString& src) { - if ( this != &src ) - { - SysFreeString(m_bstrBuf); - m_bstrBuf = src.Copy(); - } + if ( this != &src ) + { + wxCHECK_MSG(m_bstrBuf == NULL || m_bstrBuf != src.m_bstrBuf, + *this, wxS("Attempting to assign already owned BSTR")); + SysFreeString(m_bstrBuf); + m_bstrBuf = src.Copy(); + } - return *this; + return *this; } wxBasicString& wxBasicString::operator=(const wxString& str) { - SysFreeString(m_bstrBuf); - m_bstrBuf = ::SysAllocString(str.wc_str(*wxConvCurrent)); + SysFreeString(m_bstrBuf); + m_bstrBuf = SysAllocString(str.wc_str(*wxConvCurrent)); - return *this; + return *this; } wxBasicString& wxBasicString::operator=(BSTR bstr) { - wxCHECK_MSG(m_bstrBuf != bstr, *this, wxS("Attempting to attach already attached BSTR!")); + wxCHECK_MSG(m_bstrBuf == NULL || m_bstrBuf != bstr, + *this, wxS("Attempting to assign already owned BSTR")); - SysFreeString(m_bstrBuf); - m_bstrBuf = bstr; + SysFreeString(m_bstrBuf); + m_bstrBuf = bstr; - return *this; -} - -BSTR wxBasicString::Detach() -{ - BSTR bstr = m_bstrBuf; - - m_bstrBuf = NULL; - - return bstr; + return *this; } // ---------------------------------------------------------------------------- diff --git a/src/msw/webview_ie.cpp b/src/msw/webview_ie.cpp index 18635ec7a6..79ea18b78f 100644 --- a/src/msw/webview_ie.cpp +++ b/src/msw/webview_ie.cpp @@ -232,7 +232,7 @@ wxString wxWebViewIE::GetPageSource() const if(SUCCEEDED(hr)) { wxBasicString bstr; - if ( htmlTag->get_outerHTML(bstr) == S_OK ) + if ( htmlTag->get_outerHTML(bstr.ByRef()) == S_OK ) source = bstr; } } @@ -577,7 +577,7 @@ wxString wxWebViewIE::GetCurrentTitle() const if(document) { wxBasicString title; - if ( document->get_nameProp(title) == S_OK ) + if ( document->get_nameProp(title.ByRef()) == S_OK ) s = title; } @@ -706,7 +706,7 @@ bool wxWebViewIE::IsEditable() const if(document) { wxBasicString mode; - if ( document->get_designMode(mode) == S_OK ) + if ( document->get_designMode(mode.ByRef()) == S_OK ) { if ( wxString(mode) == "On" ) return true; @@ -732,7 +732,7 @@ bool wxWebViewIE::HasSelection() const if(SUCCEEDED(hr)) { wxBasicString type; - if ( selection->get_type(type) == S_OK ) + if ( selection->get_type(type.ByRef()) == S_OK ) sel = type; } return sel != "None"; @@ -768,7 +768,7 @@ wxString wxWebViewIE::GetSelectedText() const if(SUCCEEDED(hr)) { wxBasicString text; - if ( range->get_text(text) == S_OK ) + if ( range->get_text(text.ByRef()) == S_OK ) selected = text; } } @@ -801,7 +801,7 @@ wxString wxWebViewIE::GetSelectedSource() const if(SUCCEEDED(hr)) { wxBasicString text; - if ( range->get_htmlText(text) == S_OK ) + if ( range->get_htmlText(text.ByRef()) == S_OK ) selected = text; } } @@ -842,7 +842,7 @@ wxString wxWebViewIE::GetPageText() const if(SUCCEEDED(hr)) { wxBasicString out; - if ( body->get_innerText(out) == S_OK ) + if ( body->get_innerText(out.ByRef()) == S_OK ) text = out; } return text; @@ -949,9 +949,7 @@ wxCOMPtr wxWebViewIE::GetDocument() const bool wxWebViewIE::IsElementVisible(wxCOMPtr elm) { - wxCOMPtr elm1 = elm; - wxBasicString display_bstr; - wxBasicString visibility_bstr; + wxCOMPtr elm1 = elm; bool is_visible = true; //This method is not perfect but it does discover most of the hidden elements. //so if a better solution is found, then please do improve. @@ -963,15 +961,18 @@ bool wxWebViewIE::IsElementVisible(wxCOMPtr elm) wxCOMPtr style; if(SUCCEEDED(elm2->get_currentStyle(&style))) { + wxBasicString display_bstr; + wxBasicString visibility_bstr; + //Check if the object has the style display:none. - if((style->get_display(display_bstr) != S_OK) || - ((BSTR)display_bstr != NULL && (wxCRT_StricmpW(display_bstr, L"none") == 0))) + if((style->get_display(display_bstr.ByRef()) != S_OK) || + wxString(display_bstr).IsSameAs(wxS("none"), false)) { is_visible = false; } //Check if the object has the style visibility:hidden. - if((is_visible && (style->get_visibility(visibility_bstr) != S_OK)) || - ((BSTR)visibility_bstr != NULL && wxCRT_StricmpW(visibility_bstr, L"hidden") == 0)) + if((is_visible && (style->get_visibility(visibility_bstr.ByRef()) != S_OK)) || + wxString(visibility_bstr).IsSameAs(wxS("hidden"), false)) { is_visible = false; } From 6725a0a1b812341849235ab8588cfd3ba66d168f Mon Sep 17 00:00:00 2001 From: PB Date: Sat, 1 Jul 2017 13:07:59 +0200 Subject: [PATCH 9/9] Fix BSTR leaks in wxAMMediaBackend and wxWMP10MediaBackend. --- src/msw/mediactrl_am.cpp | 6 +++--- src/msw/mediactrl_wmp10.cpp | 34 +++++++++++++++++----------------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/msw/mediactrl_am.cpp b/src/msw/mediactrl_am.cpp index eb181093fe..806b6c61ef 100644 --- a/src/msw/mediactrl_am.cpp +++ b/src/msw/mediactrl_am.cpp @@ -1127,7 +1127,7 @@ bool wxAMMediaBackend::Load(const wxURI& location, const wxURI& proxy) if(pPlay) { pPlay->put_UseHTTPProxy(VARIANT_TRUE); - pPlay->put_HTTPProxyHost(wxBasicString(proxy.GetServer()).Detach()); + pPlay->put_HTTPProxyHost(wxBasicString(proxy.GetServer())); pPlay->put_HTTPProxyPort(wxAtoi(proxy.GetPort())); pPlay->Release(); } @@ -1150,9 +1150,9 @@ bool wxAMMediaBackend::DoLoad(const wxString& location) // the docs say its async and put_FileName is not - // but in practice they both seem to be async anyway if(GetMP()) - hr = GetMP()->Open( wxBasicString(location).Detach() ); + hr = GetMP()->Open( wxBasicString(location) ); else - hr = GetAM()->put_FileName( wxBasicString(location).Detach() ); + hr = GetAM()->put_FileName( wxBasicString(location) ); if(FAILED(hr)) { diff --git a/src/msw/mediactrl_wmp10.cpp b/src/msw/mediactrl_wmp10.cpp index b9d145d4e7..da014b8644 100644 --- a/src/msw/mediactrl_wmp10.cpp +++ b/src/msw/mediactrl_wmp10.cpp @@ -905,21 +905,21 @@ bool wxWMP10MediaBackend::Load(const wxURI& location, { long lOldSetting; if( pWMPNetwork->getProxySettings( - wxBasicString(location.GetScheme()).Get(), &lOldSetting + wxBasicString(location.GetScheme()), &lOldSetting ) == 0 && pWMPNetwork->setProxySettings( - wxBasicString(location.GetScheme()).Detach(), // protocol + wxBasicString(location.GetScheme()), // protocol 2) == 0) // 2 == manually specify { - BSTR bsOldName = NULL; + wxBasicString bsOldName; long lOldPort = 0; pWMPNetwork->getProxyName( - wxBasicString(location.GetScheme()).Detach(), - &bsOldName); + wxBasicString(location.GetScheme()), + bsOldName.ByRef()); pWMPNetwork->getProxyPort( - wxBasicString(location.GetScheme()).Detach(), + wxBasicString(location.GetScheme()), &lOldPort); long lPort; @@ -936,11 +936,11 @@ bool wxWMP10MediaBackend::Load(const wxURI& location, } if( pWMPNetwork->setProxyName( - wxBasicString(location.GetScheme()).Detach(), // proto - wxBasicString(server).Get() ) == 0 && + wxBasicString(location.GetScheme()), // proto + wxBasicString(server) ) == 0 && pWMPNetwork->setProxyPort( - wxBasicString(location.GetScheme()).Detach(), // proto + wxBasicString(location.GetScheme()), // proto lPort ) == 0 ) @@ -948,16 +948,16 @@ bool wxWMP10MediaBackend::Load(const wxURI& location, bOK = DoLoad(location.BuildURI()); pWMPNetwork->setProxySettings( - wxBasicString(location.GetScheme()).Detach(), // protocol + wxBasicString(location.GetScheme()), // protocol lOldSetting); if(bsOldName) pWMPNetwork->setProxyName( - wxBasicString(location.GetScheme()).Detach(), // protocol + wxBasicString(location.GetScheme()), // protocol bsOldName); if(lOldPort) pWMPNetwork->setProxyPort( - wxBasicString(location.GetScheme()).Detach(), // protocol + wxBasicString(location.GetScheme()), // protocol lOldPort); pWMPNetwork->Release(); @@ -997,7 +997,7 @@ bool wxWMP10MediaBackend::DoLoad(const wxString& location) { IWMPMedia* pWMPMedia; - if( (hr = pWMPCore3->newMedia(wxBasicString(location).Get(), + if( (hr = pWMPCore3->newMedia(wxBasicString(location), &pWMPMedia)) == 0) { // this (get_duration) will actually FAIL, but it will work. @@ -1012,7 +1012,7 @@ bool wxWMP10MediaBackend::DoLoad(const wxString& location) #endif { // just load it the "normal" way - hr = m_pWMPPlayer->put_URL( wxBasicString(location).Detach() ); + hr = m_pWMPPlayer->put_URL( wxBasicString(location) ); } if(FAILED(hr)) @@ -1061,12 +1061,12 @@ bool wxWMP10MediaBackend::ShowPlayerControls(wxMediaCtrlPlayerControls flags) if(!flags) { m_pWMPPlayer->put_enabled(VARIANT_FALSE); - m_pWMPPlayer->put_uiMode(wxBasicString(wxT("none")).Detach()); + m_pWMPPlayer->put_uiMode(wxBasicString(wxT("none"))); } else { // TODO: use "custom"? (note that CE only supports none/full) - m_pWMPPlayer->put_uiMode(wxBasicString(wxT("full")).Detach()); + m_pWMPPlayer->put_uiMode(wxBasicString(wxT("full"))); m_pWMPPlayer->put_enabled(VARIANT_TRUE); } @@ -1358,7 +1358,7 @@ wxLongLong wxWMP10MediaBackend::GetDownloadTotal() if(m_pWMPPlayer->get_currentMedia(&pWMPMedia) == 0) { BSTR bsOut; - pWMPMedia->getItemInfo(wxBasicString(wxT("FileSize")).Detach(), + pWMPMedia->getItemInfo(wxBasicString(wxT("FileSize")), &bsOut); wxString sFileSize = wxConvertStringFromOle(bsOut);