Smart Pointer to solve a problem

[Mosfet]

Hi,

I am using a set of class that are supposed to help to manage outlook
contacts as shown below (I ma only showing method used in my test case):


You just need to know that from a CMAPI object you can retrieve a list
of messages (GetNewMessages method) and then for each message GetMessage
is called to analyse it.



class CMAPI:
------------
CMessage CMAPI::GetMessage(SBinary &Entry)
{
CMessage Message(*this, Entry);

return Message;
}



class CMessage:
------------
CMessage::CMessage() : m_pMAPI(NULL), m_pMessage(NULL),
m_propBody(NULL), m_propFrom(NULL), m_propFromEmail(NULL),
m_propSubject(NULL),
m_propHasAttach(NULL)
{
}

CMessage::CMessage(const CMessage &s)
{
*this = s;
}

CMessage& CMessage:perator =(const CMessage &s)
{
m_pMessage = s.m_pMessage;
if(m_pMessage)
m_pMessage->AddRef();
m_pMAPI = s.m_pMAPI;

m_propBody = s.m_propBody;
m_propFrom = s.m_propFrom;
m_propFromEmail = s.m_propFromEmail;
m_propSubject = s.m_propSubject;
m_propHasAttach = s.m_propHasAttach;

return *this;
}

CMessage::CMessage(const CMAPI& rMAPI, const SBinary &Entry) :
m_pMAPI(&(CMAPI&)rMAPI)
{
HRESULT hr;
ULONG ulObjType;

hr = ((CMAPI&)rMAPI).GetSession()->OpenEntry( Entry.cb, (LPENTRYID)
Entry.lpb,NULL, MAPI_BEST_ACCESS, &ulObjType,
(LPUNKNOWN*) &m_pMessage);
EXIT_ON_FAILED(hr);

hr = GetProperty(PR_SENDER_NAME, m_propFrom);
hr = GetProperty(PR_SENDER_EMAIL_ADDRESS, m_propFromEmail);
hr = GetProperty(PR_SUBJECT, m_propSubject);
hr = GetProperty(PR_HASATTACH, m_propHasAttach);
EXIT_ON_FAILED(hr);

m_propBody = NULL; // Body must be streamed - see GetBody() method

FuncExit:
return;
}

CMessage::~CMessage()
{

if(m_propBody) { MAPIFreeBuffer(m_propBody); m_propBody = NULL;}
if(m_propFrom) { MAPIFreeBuffer(m_propFrom); m_propFrom = NULL;}
if(m_propFromEmail) { MAPIFreeBuffer(m_propFromEmail); m_propFromEmail
= NULL;}
if(m_propSubject) { MAPIFreeBuffer(m_propSubject); m_propSubject = NULL;}

if(m_pMessage)
m_pMessage->Release();

}
....

What is important to know is the fact this class retrieve some message
properties(GetProperty) during its creation.

This class is used in the following scenario :

if (SUCCEEDED(m_mapi.GetNewMessages(entryList)))
{
for (int i = 0; i < entryList.cValues; i++)
{
CMessage msg = m_mapi.GetMessageW(entryList.lpbin);
m_LogWnd.Log(_T("====================\r\n"));
m_LogWnd.Log (_T("Message[%d]\r\n"), i);
m_LogWnd.Log (_T("SenderMail: %s\r\n"), msg.GetSenderEmail());
m_LogWnd.Log (_T("SenderName: %s\r\n"), msg.GetSenderName());
m_LogWnd.Log (_T("Subject: %s\r\n"), msg.GetSubject());
m_LogWnd.Log (_T("Body:\r\n%s\r\n"), msg.GetBody());
}
}

CMessage& CMessage:perator =(const CMessage&s)

So now the problem from what I understand is the fact that from my CMAPI
object (m_mapi) I am calling the GetNewMessages method that constructs
an object CMessage : CMessage Message(*this, Entry); and return it.
So during its construction it retrieves messages properties by
allocating some structures m_propXXX and filling them with proper values


Then because of this line
CMessage msg = m_mapi.GetMessageW(entryList.lpbin);

The assignement operator is called to copy properties BUT after the
desctructor is called and properties are deallocated.
So in my msg object I have properties pointing to deallocated memory.

Actually when running in debugger here is my call stack :

CMessage CMAPI::GetMessage(SBinary &Entry)
CMessage::CMessage(const CMAPI& rMAPI, const SBinary &Entry)
CMessage::CMessage(const CMessage&s)
CMessage& CMessage:perator =(const CMessage&s)
CMessage::~CMessage() // Here proprties are deallocated


First I would like to know if my analysis is correct and then how I
could fix it ?
Maybe I could copy properties instead of doing m_XXXX = s.m_XXXXX; but
I am afraid that it's not very efficient and I want a better solution.
I was thinking of smart pointers, could it help ?



Thanks

 

[Vaclav Haisman]

Mosfet wrote, On 3.3.2009 10:06:

Hi,

I am using a set of class that are supposed to help to manage outlook
contacts as shown below (I ma only showing method used in my test case):


You just need to know that from a CMAPI object you can retrieve a list
of messages (GetNewMessages method) and then for each message GetMessage
is called to analyse it. [...]

...

What is important to know is the fact this class retrieve some message
properties(GetProperty) during its creation.

This class is used in the following scenario :

if (SUCCEEDED(m_mapi.GetNewMessages(entryList)))
{
for (int i = 0; i < entryList.cValues; i++)
{
CMessage msg = m_mapi.GetMessageW(entryList.lpbin);
m_LogWnd.Log(_T("====================\r\n"));
m_LogWnd.Log (_T("Message[%d]\r\n"), i);
m_LogWnd.Log (_T("SenderMail: %s\r\n"), msg.GetSenderEmail());
m_LogWnd.Log (_T("SenderName: %s\r\n"), msg.GetSenderName());
m_LogWnd.Log (_T("Subject: %s\r\n"), msg.GetSubject());
m_LogWnd.Log (_T("Body:\r\n%s\r\n"), msg.GetBody());
}
}

CMessage& CMessage:perator =(const CMessage&s)

So now the problem from what I understand is the fact that from my CMAPI
object (m_mapi) I am calling the GetNewMessages method that constructs
an object CMessage : CMessage Message(*this, Entry); and return it.
So during its construction it retrieves messages properties by
allocating some structures m_propXXX and filling them with proper values


Then because of this line
CMessage msg = m_mapi.GetMessageW(entryList.lpbin);

The assignement operator is called to copy properties BUT after the
desctructor is called and properties are deallocated.
So in my msg object I have properties pointing to deallocated memory.

Actually when running in debugger here is my call stack :

CMessage CMAPI::GetMessage(SBinary &Entry)
CMessage::CMessage(const CMAPI& rMAPI, const SBinary &Entry)
CMessage::CMessage(const CMessage&s)

These two CMessage ctors calls seem odd. Why does CMessage copy ctor call
another CMessage ctor? But that's not the core of your problem.

CMessage& CMessage:perator =(const CMessage&s)
CMessage::~CMessage() // Here proprties are deallocated


First I would like to know if my analysis is correct and then how I

Yes, it seems you are right.

could fix it ?
Maybe I could copy properties instead of doing m_XXXX = s.m_XXXXX; but

Yes, that would probably fix the problem.

I am afraid that it's not very efficient and I want a better solution.

You should not worry about efficiency before correctness. Always make the
code correct first. Then profile and only then optimize the hot spots.

I was thinking of smart pointers, could it help ?

Yes, shared_ptr<> with custom deleter sounds like it would work well there.