• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 611
  • Last Modified:

Vector of Objects getting Uninitilized Memory error in my debug tool, Rational Purify

Hi Expert,
I have a COM DLL that I'm working on using VS2005 with only the windows library.  In this, I have declare a vector to store objects that are created based on my own class.  I've also coded the copy constructor and the Operator = overload so the vector would accept it.  Each field of each object that req mem allocation has memory allocated for it before I call the vector.Push_back. I'm getting Uninitilized Memory error in my debug tool, Rational Purify.  If I'm just run Active Directory, client of my COM, my dll would crash with unhandled win32 error.  I'm stuck here becuase I can't debug any further until I can resolve this bug.  

I've listed some code and the error message below.  Can you tell me what I'm missing here?  Thank you.
I was also told to use wstring instead of wchar_t* declaration for string member variables in the class.  
question: Do I need to allocate(new) and de-allocate(delete) memory space for wstring?  Do I allocate the same as I would for wchanr_t* as listed in my present code?  Can you point out a good article on wstring conversion functions becuase I have quite a bit of variables that the wstring variable needs to convert to or work with like PWSTR, LPCSWSTR, _bstr_t...  thanks.

     vector <CUserContextData*> userDataObjects;
     vector <CUserContextData*>::iterator userDataIter;
   CUserContextData *newUserData = new CUserContextData;

I was told that the std vecor doesn't like stroing pointer to objects of class so here is my new declaration:

    vector <CUserContextData> userDataObjects;
     vector <CUserContextData>::iterator userDataIter;
     CUserContextData newUserData;
...allocated memory and assign values to the member data fields.
          userDataObjects.push_back(newUserData);


class CUserContextData
{
public:
            
    bool newObject, bWinLogOn, bUnifiedID, symarkEnabled, UIDChanged;
      std::wstring shell, homeDir, primaryGroup, symarkUserRef, LoginName, contextName;
       LPCWSTR lpcwContextName;
    int symarkUID, listViewIndex, UID;
      //std::wstring test;
      void CUserContextData::Copier(const CUserContextData &newUserData)
      {
            const wchar_t *lpcTemp;
            int size;

            newObject = newUserData.newObject;
            bWinLogOn = newUserData.bWinLogOn;
            bUnifiedID = newUserData.bUnifiedID;
            symarkEnabled = newUserData.symarkEnabled;
            UIDChanged = newUserData.UIDChanged;
            symarkUID = newUserData.symarkUID;
            listViewIndex = newUserData.listViewIndex;
            UID = newUserData.UID;
             
            if((newUserData.lpcwContextName != NULL) && ((size =wcslen(newUserData.lpcwContextName))>0))
            {
                  wchar_t *temp = new wchar_t[size+1];
                  wcscpy_s(temp, size+1, newUserData.lpcwContextName);
                  lpcwContextName = (LPCWSTR)temp;
            }
            
            if((newUserData.contextName != NULL) && ((size =wcslen(newUserData.contextName))>0))
            {
                  lpcTemp = T2CW(newUserData.contextName);
                  contextName = new wchar_t[size+1];
                  wcscpy_s(contextName, size+1, lpcTemp);
            }

                        
            if((newUserData.LoginName != NULL) && ((size =wcslen(newUserData.LoginName))>0))
            {
                  lpcTemp = T2CW(newUserData.LoginName);
                  LoginName = new wchar_t[size+1];
                  wcscpy_s(LoginName, size+1, (lpcTemp));
            }


            lpcTemp = T2CW(newUserData.shell);
            shell = new wchar_t[wcslen(lpcTemp)+1];
            wcscpy_s(shell, wcslen(lpcTemp)+1, (lpcTemp));

            lpcTemp = T2CW(newUserData.homeDir);
            homeDir = new wchar_t[wcslen(lpcTemp)+1];
            wcscpy_s(homeDir, wcslen(lpcTemp)+1, (lpcTemp));

            lpcTemp = T2CW(newUserData.primaryGroup);
            primaryGroup = new wchar_t[wcslen(lpcTemp)+1];
            wcscpy_s(primaryGroup, wcslen(lpcTemp)+1, (lpcTemp));

            lpcTemp = T2CW(newUserData.symarkUserRef);
            symarkUserRef = new wchar_t[wcslen(lpcTemp)+1];
            wcscpy_s(symarkUserRef, wcslen(lpcTemp)+1, (lpcTemp));
}
      void CUserContextData::DeleteUserContextData()
      {
            delete [] shell;
            delete [] homeDir;
            delete [] primaryGroup;
            delete [] symarkUserRef;
            //if(LoginName != NULL)
                  delete [] LoginName;
            delete [] contextName;
            delete [] lpcwContextName;
      }

      CUserContextData& CUserContextData::operator=(const CUserContextData& newUserData)
      {
            if(this != &newUserData)
            {
                  DeleteUserContextData();
                  Copier(newUserData);
            }
            return *this;
}

      CUserContextData::CUserContextData(void)
      {
            lpcwContextName = NULL;
            contextName = NULL;
            shell = NULL;
            homeDir = NULL;
            primaryGroup = NULL;
            symarkUserRef = NULL;
            LoginName = NULL;

            listViewIndex = NULL;
            UID = NULL;
            symarkUID=NULL;
            newObject = false; bWinLogOn = false; bUnifiedID = false; symarkEnabled = false; UIDChanged = false;
      }

      CUserContextData::CUserContextData(const CUserContextData &newUserData)
      {
            Copier(newUserData);
            
      }
      ~CUserContextData(void)
      {
            DeleteUserContextData();
      }


};

0
lapucca
Asked:
lapucca
  • 4
  • 3
  • 3
1 Solution
 
lapuccaAuthor Commented:
One more thing, I just notice when I step through in debug mode, the Copier method is called twice.  I believe because Copier is in both the Copy Contrctor and the Operator= overload, so it's called twice.  Should I just remove the Copy Constructor?

Thanks.
0
 
itsmeandnobodyelseCommented:
>>>> shell = new wchar_t[wcslen(lpcTemp)+1];

These kind of statements make no sense.  With the right side of the assignment you create a wchar_t array where the contents is undefined. std::wstring::operator=(const wchar_t*) will now try to determine the string length of the 'string value' passed as argument. As already said the contents of anewly allocated wide string is undefined. Hence, terminating zero characters might be found within array bounds or beyond.

>>>> wcscpy_s(shell, wcslen(lpcTemp)+1, (lpcTemp));

Actually, I don't believe that the statement compiles. 'shell' is std::wstring while wcscpy_s requires a wchar_t* . Did you use a self-written 'wcscpy_s' function? If so, could you post the code?

>>>> Do I need to allocate(new) and de-allocate(delete) memory space for wstring?

No, std::wstring is a complete class type that doesn't need external allocation.

>>>> Can you point out a good article on wstring conversion functions

wstring is defined as

    basic_string<wchar_t, char_traits<wchar_t>,
            allocator<wchar_t> >

That means it is a specialization of the template class basic_string where you might find cillions of documentations, e. g. http://www.roguewave.com/support/docs/leif/sourcepro/html/stdlibref/wstring.html.

Generally, you  don't need conversion or temporary wchar_t pointers when assigning a wstring to a wstring, but simply an assignment.

   shell = newUserData.shell;


Regards, Alex
0
 
chip3dCommented:
>I was also told to use wstring instead of wchar_t* declaration for string member variables in the class.
yes, use wstring... this object will do all allocation and deallocation for you. You can use it with wchar_t:
const wchar_t * str = L"Hello, world!";
wstring test = str; // no problem
and if you need a wchar_t pointer from wstring, just use the memberfunction c_str();
const wchar_t *test2 = test.c_str();
http://www.cppreference.com/cppstring/index.html

>I was told that the std vecor doesn't like stroing pointer to objects of class so here is my new declaration:
yes again, cuz the vectorclass will not free the memory, if you use pointers.
But you can use SmartPointers. They will do the dealloctaion for you.
http://www.cppreference.com/cppmisc/auto_ptr.html
http://www.boost.org/libs/smart_ptr/smart_ptr.htm


// you can make the code much simpler :)
class CUserContextData
{
public:
         
     bool newObject, bWinLogOn, bUnifiedID, symarkEnabled, UIDChanged;
     std::wstring shell, homeDir, primaryGroup, symarkUserRef, LoginName, contextName, ContextName;
     int symarkUID, listViewIndex, UID;

     CUserContextData& CUserContextData::operator=(const CUserContextData& newUserData)
     {
          if(this != &newUserData)
          {
               *this = newUserData;
          }
          return *this;
     }

     CUserContextData::CUserContextData(void) :  newObject(false), bWinLogOn(false),  bUnifiedID(false), symarkEnabled(false), UIDChanged(false),
                                                                        listViewIndex(0),  UID(0),   symarkUID(0)
     {
         // nothing to do
     }


     CUserContextData::CUserContextData(const CUserContextData &newUserData)
     {
           *this = newUserData;
     }

     ~CUserContextData(void)
     {
          // nothing to do
     }
};

0
Technology Partners: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

 
itsmeandnobodyelseCommented:
>>>> I'm getting Uninitilized Memory error

That is because of

     shell = new wchar_t[wcslen(lpcTemp)+1];

>>>> my dll would crash with unhandled win32 error

If you have a dll STL objects shouldn't be used in the interface and you couldn't create STL objects in the dll while deleting them in the main application. The reason for this is that STL classes use static member variables. If using a DLL the static members were created twice, once in the DLL and once in the Application. Hence, all operations requiring or accessing these static members (creation, destroying, reallocation) need to be done either in the DLL or in the Application but not in both.

Regards, Alex
0
 
lapuccaAuthor Commented:
Thank you, thank you, thank you...........  I've been stucked in this problem for a couple of weeks.  I've been gettting just bits and pieces of advises here and there, all valid and helpful but none that as complete and easy to follow as your.  The code I pasted truly didn't compile.  I wasn't sure how to use the wstring and you answer that.  

Question, the newUserData is declare on stack without using new.  Can I still pass it to return?  Do I not need the copy contructor in this case?  I'm posting a method where the newUserData is initiated on stack then pushed to the vector.  In this event, do I not need the copy constructor?  Thank you very much.
HRESULT CUserPage::RetrieveUserAtt(int count, int listViewIndex, HWND hwndListView, LPWSTR contextNames)
{
      //GTiven a context, this method will locate the symark-UserContext data object if
      //it exists, and push it to the vector object.
      HRESULT hResult;
      _bstr_t context, temp;
      VARIANT var;
      VariantInit(&var);
      IADs *ptSyUserObject = NULL;
      LPCWSTR lpcTemp;

      context = contextNames;

      _bstr_t syUserContextPath = L"LDAP://"  + bstrtServerDomain +
            L"/CN=" + userCN + L",CN=Users," + L"CN=" +context +
            L",CN=Contexts,CN=Unity,CN=Symark,CN=Program Data" +L"," + bstrDomainOnly;

      LPCWSTR lpcSyUserContextPath = syUserContextPath;

      hResult = ADsGetObject(lpcSyUserContextPath,IID_IADs,(void**)&ptSyUserObject);
      if(FAILED(hResult))//New Sy User
      {
            if(hResult != SY_OBJECT_NOT_EXIST)//Error, not becuase SY user not found
                  DisplayError( hResult, L"Error obtaining Unix User Attributes. - RetrieveUserAtt");
            goto CleanUp;
      }

      if(SUCCEEDED(hResult))
      {

            CUserContextData newUserData;
            //create the new userDataobject and push to Vector container
            newUserData.newObject = false;
            newUserData.UIDChanged = false;
            newUserData.listViewIndex = listViewIndex;

            newUserData.contextName = new wchar_t[wcslen((LPCWSTR)(contextNames))+1];
            int len = wcslen((LPCWSTR)(contextNames))+1;
            wcscpy_s(newUserData.contextName, wcslen((LPCWSTR)(contextNames))+1, (LPCWSTR)(contextNames));

            //wchar_t *lpcwCont = new wchar_t[wcslen((LPCWSTR)(contextNames))+1];
            //wcscpy_s(lpcwCont, wcslen((LPCWSTR)(contextNames))+1, (LPCWSTR)(contextNames));
            newUserData.lpcwContextName = (LPCWSTR)(newUserData.contextName);
            //newUserData.lpcwContextName = T2CW(lpcwCont);

            lpcTemp = userDN;
            newUserData.symarkUserRef = new wchar_t[wcslen(lpcTemp)+1];
            wcscpy_s(newUserData.symarkUserRef, wcslen(lpcTemp)+1, (lpcTemp));

            hResult =ptSyUserObject->Get(CComBSTR("symark-Enabled"), &var);
            if(FAILED(hResult))//New Sy User
            {
                  DisplayError( hResult, L"Error obtaining symark-Enabled. - RetrieveUserAtt");
                  goto CleanUp;
            }
            if(var.boolVal)
                  newUserData.symarkEnabled = true;
            else
                  newUserData.symarkEnabled = false;
      VariantClear(&var);


//Get the symark-UseUnifiedUID
            hResult =ptSyUserObject->Get(CComBSTR("symark-UseUnifiedUID"), &var);
            if(FAILED(hResult))//New Sy User
            {
                  DisplayError( hResult, L"Error obtaining symark-UseUnifiedUID. - RetrieveUserAtt");
                  goto CleanUp;
            }
            if(var.boolVal)
                  newUserData.bUnifiedID = true;
            else
                  newUserData.bUnifiedID = false;
            VariantClear(&var);

            hResult =ptSyUserObject->Get(CComBSTR("symark-UseWinLogName"), &var);
            if(FAILED(hResult))//New Sy User
            {
                  DisplayError( hResult, L"Error obtaining symark-UseWinLogName. - RetrieveUserAtt");
                  goto CleanUp;
            }
            if(var.boolVal)
                  newUserData.bWinLogOn = true;
            else
                  newUserData.bWinLogOn = false;
            VariantClear(&var);


            if(!newUserData.bWinLogOn)//if not using winlogin then get the ind loginname
            {
                  hResult =ptSyUserObject->Get(CComBSTR("symark-LoginName"), &var);
                  if(FAILED(hResult))//New Sy User
                  {
                        DisplayError( hResult, L"Error obtaining symark-LoginName. - RetrieveUserAtt");
                        goto CleanUp;
                  }
                  temp = var;
                  if(temp.length()>0)
                  {
                        lpcTemp = temp;
                        newUserData.LoginName = new wchar_t[wcslen(lpcTemp)+1];
                        wcscpy_s(newUserData.LoginName, wcslen(lpcTemp)+1, (lpcTemp));
                  }
                  else
                        newUserData.LoginName =NULL;
                  VariantClear(&var);
            }


//Get the Shell
                  hResult =ptSyUserObject->Get(CComBSTR("symark-Shell"), &var);
                  if(FAILED(hResult))//New Sy User
                  {
                        DisplayError( hResult, L"Error obtaining symark-Shell. - RetrieveUserAtt");
                        goto CleanUp;
                  }
                  temp = var.bstrVal;
                  if(temp.length()>0)
                  {
                        lpcTemp = temp;
                        newUserData.shell = new wchar_t[wcslen(lpcTemp)+1];
                        wcscpy_s(newUserData.shell, wcslen(lpcTemp)+1, (lpcTemp));
                  }
                  else
                        newUserData.shell =NULL;
                  VariantClear(&var);


//Get the Home
                  hResult =ptSyUserObject->Get(CComBSTR("symark-HomeDirectory"), &var);
                  if(FAILED(hResult))//New Sy User
                  {
                        DisplayError( hResult, L"Error obtaining symark-HomeDirectory. - RetrieveUserAtt");
                        goto CleanUp;
                  }
                  temp = var.bstrVal;
                  if(temp.length()>0)
                  {
                        lpcTemp = temp;
                        newUserData.homeDir = new wchar_t[wcslen(lpcTemp)+1];
                        wcscpy_s(newUserData.homeDir, wcslen(lpcTemp)+1, (lpcTemp));
                  }
                  else
                        newUserData.homeDir =NULL;
                  VariantClear(&var);

//Get the primegroup
                  hResult =ptSyUserObject->Get(CComBSTR("symark-PrimaryGroupReference"), &var);
                  if(FAILED(hResult))//New Sy User
                  {
                        DisplayError( hResult, L"Error obtaining symark-PrimaryGroupReference. - RetrieveUserAtt");
                        goto CleanUp;
                  }
                  temp = var.bstrVal;
                  if(temp.length()>0)
                  {
                        lpcTemp = temp;
                        newUserData.primaryGroup = new wchar_t[wcslen(lpcTemp)+1];
                        wcscpy_s(newUserData.primaryGroup, wcslen(lpcTemp)+1, (lpcTemp));
                  }
                  else
                        newUserData.primaryGroup =NULL;
                  VariantClear(&var);
            _ASSERTE(_CrtCheckMemory());


//Get the UID

            hResult =ptSyUserObject->Get(CComBSTR("symark-UID"), &var);
            if(FAILED(hResult))//New Sy User
            {
                  DisplayError( hResult, L"Error obtaining symark-UID. - RetrieveUserAtt");
                  goto CleanUp;
            }
            if(var.intVal == NULL)
            {
                  ::MessageBox(NULL, L"Fail to get either a UnifiedID or an Independent ID.", L"ID Error - RetrieveUserAtt", MB_ICONERROR | MB_OK);
                  hResult = E_FAIL;
                  goto CleanUp;
            }

            else
                   newUserData.symarkUID = var.intVal;
                  newUserData.UID = var.intVal;
                  //if we're using unified id then make user they matched
                  if ((newUserData.bUnifiedID)&& (newUserData.symarkUID != userSyUnifiedID))
                  {
                        ::MessageBox(NULL, L"The symark-UID Doesn't match User UnifiedID.", L"Unified ID Error - RetrieveUserAtt", MB_ICONERROR | MB_OK);
                  }
            
            
            userDataObjects.push_back(newUserData);
            //Retrieve record and fill the Dialog screen and select the item
            if( count == 0)
            {
                  FindUserDataObject(newUserData.contextName);      
                  DisplayUserData();
                  ListView_SetItemState(hwndListView, listViewIndex, LVIS_SELECTED, 0);
            }
            //Store in vector
            hResult = S_OK;
            
      }//end if we found the symark user object
0
 
lapuccaAuthor Commented:
   CUserContextData::CUserContextData(const CUserContextData &newUserData)
     {
           *this = newUserData;
     }

This copy contructor, do I need to add :return *this"?
0
 
chip3dCommented:
hi, lapucca

normally you only need a copyconstructor if you want to copy special parts or you have to create object on the heap. If you use types like bool or int and full objects like wstring, then c++ perform a bytebybyte copy of you object, that is ok in most cases. In your case, you don't need a copyconstructor, also not for th e vector...

If you declare an object on stack and you want to return, you have to return a copy, not a reference or pointer, cuz if you leaf the function, the stack will be cleaned up. This means the destructor of all object will be called and than the memory will be freed. One way would be to create the object before function call and pass it by reference to the function wich is setting the object, or you create the objec in the function and return the pointer. But than you have to relesse this object somewhere else, wich is not a good solution. In this case you could use SmartPointers. After you have created a object with new, you pass  the SmartPointer the pointer of the object, and the SmartPointer will take care of freeing the object, as soon as the last reference to the object is gone...


>This copy contructor, do I need to add :return *this"?
no, a Constructor has no return value.

for your function RetrieveUserAtt
You don't need to create wchar_t pointer anymore, cuz the wstring is doing all for you
          newUserData.symarkUserRef = new wchar_t[wcslen(lpcTemp)+1];
          wcscpy_s(newUserData.symarkUserRef, wcslen(lpcTemp)+1, (lpcTemp));
                           ->
          newUserData.symarkUserRef = lpcTemp;
the last line is all you need. wstring will create and copy all data
0
 
itsmeandnobodyelseCommented:
lapucca, why did you accept the answer of chip3d?  Actually, the solution of chip3d isn't quite correct and would not compile. Moreover, you would get stack overflow cause the following statement in the operator= function recursively invokes the assignment operator itself.

>>>> *this = newUserData;

In the copy constructor the staement is only valid if you have a properly implemented operator= function. Most developers prefer initialiser list rather than using the assignment operator in the copy constructor, same as in the default constructor. Then, you would have seen that not all members could be simply assigned but for some you need to get an own allocation.

 CUserContextData::CUserContextData(const CUserContextData &nud)
   :  newObject(nud.newObject), bWinLogOn(nud.bWinLogOn),  bUnifiedID(nud.bUnifiedID), ..., lpcwContextName(NULL)
 {
      if  (nud.lpcwContextName != NULL)
      {
          int len = wcslen(nud.lpcwContextName);
          lpcwContextName = new wchar_t[len+1];
          wcscpy(lpcwContextName, nud.lpcwContextName);
      }
 }

Note, as lpcwContextName is a pointer you cannot simply assign the pointer from the copied object. You would get access violation when the second of both objects would try to delete the pointer cause it already was deleted.

Regards, Alex
0
 
chip3dCommented:
>*this = newUserData;

sorry lapucca, itsmeandnobodyelse  is right, i have done a big mistake, this line will lead to a stack overflow...
0
 
lapuccaAuthor Commented:
Thank you Alex.  You're absolutely correct.  In your sample code, how do I initilize wstring variables?  I accepted the wrong answer becuase it compiled and I tought that would work. I'll will wait till it works before I accept an answer enxt time.  Thank you.
0

Featured Post

Industry Leaders: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

  • 4
  • 3
  • 3
Tackle projects and never again get stuck behind a technical roadblock.
Join Now