IDispatch COM Object - returning BSTR incorrectly

Mike the Red

I have created a COM object which implements IDispatch so I can call it from scripting languages. In the constructor, I'm trying to get the path to the App Data folder and store it in a member variable. I know that this is failing and I'm trying (unsuccessfully) to debug. Until testing is complete, my object has a method which exposes the HRESULT from the last function call to the scripting language as a string.

[id(10), helpstring("Get last function's HRESULT.")] HRESULT GetHRESULT( [out, retval] BSTR * bsHRES);

This function returns the value of another member variable, BSTR m_HR, which I set using m_HR = OLESTR("Some Text") before returning from any member function. As part of the constructor debugging process, I am trying to set the BSTR m_HR to the path of the App Data folder:

TCHAR * tcPath = new TCHAR;
HRESULT hres = SHGetFolderPath(NULL, CSIDL_APPDATA, NULL, SHGFP_TYPE_CURRENT, tcPath);

if ( FAILED(hres) ) {
m_HR = OLESTR("SHGetFolderPath failed.");
} else {
_bstr_t bst = _bstr_t(tcPath);

m\_HR = bst.copy();

};
delete tcPath;
return;

//<shlobj.h> is required for SHGetFolderPath.
//<comutil.h> and linking with comsupp.lib is required for _bstr_t.

I double checked the SHGetFolderPath bit from a console application and tcPath contains the path desired. When I use m_HR = OLESTR("Some Text"), calling GetHRESULT returns "Some Text" properly. With this constructor, however, successive object creations (and calls to GetHRESULT) return, alternately, "Files\Microsoft Visual Studio .NET 2003\SDK\v1.1\Lib\" and NULL. (That's not the App Data path.) I've been banging my head on this for a couple days now - I'd be grateful for any help ya'll can offer. MZR P.S. My apologies for the non-compilable code, but just the complete constructor is 70ish lines, plus the MIDL file, and the header file.... if you think you can help and need to see the complete code to do so, let me know and I'll happily email you the complete source.

CPallini

You say nothing about how bsHRES is set in your method (or am I wrong?). :)

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong. -- Iain Clarke
[My articles]

Mike the Red

Thanks for taking a look, Pallini. You're completely right. My apologies - here's the GetHRESULT(...) method definition. (I'm never sure how much code to post, though, in hindsight, this is an important bit.)

HRESULT Man::GetHRESULT(/*out*/BSTR *bsHRES) {
if ( bsHRES == NULL ) {
m_HR = OLESTR("ERROR_INVALID_PARAMETER");
return ERROR_INVALID_PARAMETER;
};

\*bsHRES = m\_HR;
m\_HR = OLESTR("S\_OK");
return S\_OK;

};

CPallini

You should use

*bsHRES = SysAllocString( m_HR );

I suppose. :)

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong. -- Iain Clarke
[My articles]

Mike the Red

Believe it or not, I HAD considered that - I wasn't sure if I needed a call to SysAllocString there or not.. Adding it had no effect, though. :( Thanks again for taking a look, though, Pallini. I'll find it eventually... :thumbsup:

CPallini

Mike the Red wrote:

Believe it or not, I HAD considered that

I find no diffuculty to believe that. :)

Mike the Red wrote:

I wasn't sure if I needed a call to SysAllocString there or not..

You should use it.

Mike the Red wrote:

Adding it had no effect, though.

So there is a problem in another piece of your code. Good luck. :)

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong. -- Iain Clarke
[My articles]

Stuart Dootson

Mike the Red wrote:

TCHAR * tcPath = new TCHAR;

You're allocating one (ONE) character to hold the path - that probably won't be enough :-) Try allocating tcPath like this:

TCHAR * tcPath = new TCHAR[MAX_PATH];

and deallocating it like this:

delete[] tcPath;

or even just declare it on the stack like this:

TCHAR tcPath[MAX_PATH];

and then you don't need to deallocate it....

Mike the Red wrote:

m_HR = OLESTR("SHGetFolderPath failed.");

Yeah, that's bad, m'kay? You should create BSTRs with SysAllocString[^]:

m_HR = ::SysAllocString(OLESTR("SHGetFolderPath failed."));

BSTRs aren't just a null-terminated string of OLECHARs.

Mike the Red wrote:

m_HR = bst.copy();

You could just use m_HR = bst.Detach(); and reduce the number of string copies you do.

Java, Basic, who cares - it's all a bunch of tree-hugging hippy cr*p

Mike the Red

After many, many rebuilds and relocating return; statements, I finally found the problem.. Some bad code... too embarassingly bad to post... was copying bad data to m_HR and returning before execution even reached the SHGetFolderPath bit... :~ I thank you both for your help. :thumbsup: MZR