tdf#120703 (PVS)

V745 A 'wchar_t *' type string is incorrectly converted to 'BSTR' type string.
     Consider using 'SysAllocString' function.

V560 A part of conditional expression is always false: !ppdispParent.

V595 The 'm_pIParent' pointer was utilized before it was verified against nullptr.
     Check lines: 673, 675.

V745 A 'wchar_t *' type string is incorrectly converted to 'BSTR' type string.
     Consider using 'SysAllocString' function.

V530 The return value of function 'SysAllocString' is required to be utilized.

V1032 The pointer '& pChildXAcc' is cast to a more strictly aligned pointer
     type.

V512 A call of the 'GetUNOInterface' function will lead to overflow of the buffer
     '& pChildXAcc'.

V547 Expression 'pSeq' is always true.

V547 Expression 'pChild' is always true.

V512 A call of the 'GetUNOInterface' function will lead to overflow of the buffer
     '& pTempUNO'.

V1032 The pointer '& pTempUNO' is cast to a more strictly aligned pointer type.

V716 Suspicious type conversion in return statement: returned HRESULT, but function
     actually returns BOOL.

Change-Id: I2185992dad0127144416783e25476d05c0365b74
Reviewed-on: https://gerrit.libreoffice.org/62075
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

winaccessibility/source/UAccCOM/MAccessible.cxx

@@ -195,23 +195,19 @@ CMAccessible::~CMAccessible()
     if(m_pszName!=nullptr)
     {
         SAFE_SYSFREESTRING(m_pszName);
-        m_pszName=nullptr;
     }
     if(m_pszValue!=nullptr)
     {
         SAFE_SYSFREESTRING(m_pszValue);
-        m_pszValue=nullptr;
     }
     if(m_pszDescription!=nullptr)
     {
         SAFE_SYSFREESTRING(m_pszDescription);
-        m_pszDescription=nullptr;
     }
 
     if(m_pszActionDescription!=nullptr)
     {
         SAFE_SYSFREESTRING(m_pszActionDescription);
-        m_pszActionDescription=nullptr;
     }
 
     if(m_pIParent)
@@ -252,7 +248,7 @@ STDMETHODIMP CMAccessible::get_accParent(IDispatch **ppdispParent)
         else if(m_hwnd)
         {
             HRESULT hr = AccessibleObjectFromWindow(m_hwnd, OBJID_WINDOW, IID_IAccessible, reinterpret_cast<void**>(ppdispParent));
-            if( ! SUCCEEDED( hr ) || ! ppdispParent )
+            if (!SUCCEEDED(hr) || !*ppdispParent)
             {
                 return S_FALSE;
             }
@@ -671,9 +667,9 @@ STDMETHODIMP CMAccessible::get_accKeyboardShortcut(VARIANT varChild, BSTR *pszKe
                             VARIANT varParentRole;
                             VariantInit( &varParentRole );
 
-                            m_pIParent->get_accRole(varChild, &varParentRole);
-
-                            if( m_pIParent && varParentRole.lVal == ROLE_SYSTEM_COMBOBOX ) // edit in comboBox
+                            if (m_pIParent
+                                && SUCCEEDED(m_pIParent->get_accRole(varChild, &varParentRole))
+                                && varParentRole.lVal == ROLE_SYSTEM_COMBOBOX) // edit in comboBox
                             {
                                 m_pIParent->get_accKeyboardShortcut(varChild, pszKeyboardShortcut);
                                 return S_OK;
@@ -1436,7 +1432,10 @@ IMAccessible* CMAccessible::GetNavigateChildForDM(VARIANT varCur, short flags)
     }
 
     IMAccessible* pCurChild = nullptr;
-    XAccessible* pChildXAcc = nullptr;
+    union {
+        XAccessible* pChildXAcc;
+        hyper nHyper = 0;
+    };
     Reference<XAccessible> pRChildXAcc;
     XAccessibleContext* pChildContext = nullptr;
     int index = 0,delta=0;
@@ -1455,7 +1454,7 @@ IMAccessible* CMAccessible::GetNavigateChildForDM(VARIANT varCur, short flags)
         {
             return nullptr;
         }
-        pCurChild->GetUNOInterface(reinterpret_cast<hyper*>(&pChildXAcc));
+        pCurChild->GetUNOInterface(&nHyper);
         if(pChildXAcc==nullptr)
         {
             return nullptr;
@@ -1907,12 +1906,11 @@ STDMETHODIMP CMAccessible:: get_groupPosition(long __RPC_FAR *groupLevel,long __
             if ( xGroupPosition.is() )
             {
                 Sequence< sal_Int32 > rSeq = xGroupPosition->getGroupPosition( makeAny( pRContext ) );
-                sal_Int32* pSeq = rSeq.getArray();
-                if ( pSeq )
+                if (rSeq.getLength() >= 3)
                 {
-                    *groupLevel = pSeq[0];
-                    *similarItemsInGroup = pSeq[1];
-                    *positionInGroup = pSeq[2];
+                    *groupLevel = rSeq[0];
+                    *similarItemsInGroup = rSeq[1];
+                    *positionInGroup = rSeq[2];
                     return S_OK;
                 }
                 return S_OK;
@@ -2089,13 +2087,11 @@ STDMETHODIMP CMAccessible:: get_windowHandle(HWND __RPC_FAR *windowHandle)
 
         HWND nHwnd = m_hwnd;
         IAccessible* pParent = m_pIParent;
-        CMAccessible* pChild = this;
         while((nHwnd==nullptr) && pParent)
         {
-            pChild = static_cast<CMAccessible*>(pParent);
-            if(pChild)
+            if (CMAccessible* pChild = dynamic_cast<CMAccessible*>(pParent))
             {
-                pParent = static_cast<IAccessible*>(pChild->m_pIParent);
+                pParent = pChild->m_pIParent;
                 nHwnd = pChild->m_hwnd;
             }
             else
@@ -2314,8 +2310,11 @@ STDMETHODIMP CMAccessible::accSelect(long flagsSelect, VARIANT varChild)
 
     if( flagsSelect&SELFLAG_TAKEFOCUS )
     {
-        XAccessible * pTempUNO = nullptr;
-        pSelectAcc->GetUNOInterface(reinterpret_cast<hyper*>(&pTempUNO));
+        union {
+            XAccessible* pTempUNO;
+            hyper nHyper = 0;
+        };
+        pSelectAcc->GetUNOInterface(&nHyper);
 
         if( pTempUNO == nullptr )
             return NULL;
@@ -2642,12 +2641,13 @@ BOOL
 CMAccessible::get_IAccessibleFromXAccessible(XAccessible * pXAcc, IAccessible **ppIA)
 {
 
-    ENTER_PROTECTED_BLOCK
+    try
+    {
 
         // #CHECK#
         if(ppIA == nullptr)
         {
-            return E_INVALIDARG;
+            return FALSE;
         }
         BOOL isGet = FALSE;
         if(g_pAgent)
@@ -2658,7 +2658,11 @@ CMAccessible::get_IAccessibleFromXAccessible(XAccessible * pXAcc, IAccessible **
         else
             return FALSE;
 
-        LEAVE_PROTECTED_BLOCK
+    }
+    catch(...)
+    {
+        return FALSE;
+    }
 }
 
 OUString CMAccessible::get_StringFromAny(Any const & pAny)

winaccessibility/source/UAccCOM/MAccessible.h

@@ -165,12 +165,12 @@ public:
     STDMETHOD(SetXAccessible)(hyper) override;
 
 private:
-    OLECHAR* m_pszName;
-    OLECHAR* m_pszValue;
-    OLECHAR* m_pszActionDescription;
+    BSTR m_pszName;
+    BSTR m_pszValue;
+    BSTR m_pszActionDescription;
     unsigned short m_iRole;
     DWORD   m_dState;
-    OLECHAR* m_pszDescription;
+    BSTR m_pszDescription;
     IMAccessible* m_pIParent;
     Location m_sLocation;