Returning a character buffer from a DLL
dushkin
I wrote a simple MFC application and a simple MFC DLL.
I need to return a string buffer from the DLL in a RunQuery function.
Please see the code below (Note that I cleared some unrelated lines
like catch, etc, which are not related to my problem now to make code
clearer.)
If I check the "TheValue.operator _bstr_t().operator char *()" value
in the DLL ,it gives me the correct value!
But if I check the returned value in the APP, I get junk...
What is wrong here?
What I did was as follows (after many trials, and the code shown here
is not necessarily what I think is the correct one...):
-------------------------
In the App:
-------------------------
char* CSAPI2VPGDlg::GetExtEventID( CString sInternalEventID )
{
RunQuery(hRC, m_sExtEventID, sInternalEventID); //m_sExtEventID is a
32 bytes char array member
return m_sExtEventID;
}
-------------------------
In the DLL:
-------------------------
char* CXDBApp::RunQuery(const CString& a_sQuery)
{
//Find field
CString s = a_sQuery;
CString sField = GetFieldForCollect(s);
_variant_t TheValue;
m_pRecordset.CreateInstance(__uuidof(Recordset));
try
{
m_pRecordset->Open((LPCSTR)a_sQuery,
m_pConnection.GetInterfacePtr(),
adOpenDynamic,
adLockOptimistic,
adCmdText);
while(!m_pRecordset->adoEOF)
{
TheValue = m_pRecordset->GetCollect((char*)_bstr_t(sField));
if(TheValue.vt!=VT_NULL){
m_pRecordset->Close();
//return((char*)_bstr_t(TheValue));
return TheValue.operator _bstr_t().operator char *();
}
}
m_pRecordset->Close();
}
}
dushkin
Giovanni Dicanio
news:496aa2e1-edd5-4dc8...@a21g2000yqc.googlegroups.com...
> I wrote a simple MFC application and a simple MFC DLL.
> I need to return a string buffer from the DLL in a RunQuery function.
I would suggest to return strings using instances of CString class, instead
of raw C-like char/TCHAR arrays or pointers.
> If I check the "TheValue.operator _bstr_t().operator char *()" value
> in the DLL ,it gives me the correct value!
IIRC, _bstr_t should be a wrapper to BSTR strings. These BSTR strings are
Unicode strings, so it seems strange to me that you return a char* (I would
expect a wchar_t *).
Is there some implicit conversion from Unicode to ANSI/MBCS happening here?
> But if I check the returned value in the APP, I get junk...
>
> What is wrong here?
You may want to check that you dynamically link with both CRT and MFC in
both EXE and DLL (don't use static linking in this case).
> -------------------------
> In the App:
> -------------------------
> char* CSAPI2VPGDlg::GetExtEventID( CString sInternalEventID )
I would pass input strings as const references, and just return CString
instances:
CString CSAPI2VPGDlg::GetExtEventID( const CString & sInternalEventID )
> -------------------------
> In the DLL:
> -------------------------
>
> char* CXDBApp::RunQuery(const CString& a_sQuery)
Again, I would return CString instances instead of raw char*
CString CXDBApp::RunQuery(const CString& a_sQuery)
> {
> //Find field
> CString s = a_sQuery;
> CString sField = GetFieldForCollect(s);
>
> _variant_t TheValue;
>
> m_pRecordset.CreateInstance(__uuidof(Recordset));
> try
> {
> m_pRecordset->Open((LPCSTR)a_sQuery,
The above LPCSTR cast is suspicious to me.
I would use a LPCTSTR cast (i.e. const TCHAR*) , at least.
> TheValue = m_pRecordset->GetCollect((char*)_bstr_t(sField));
_bstr_t's are Unicode strings. I suspect that the char* cast is wrong.
> //return((char*)_bstr_t(TheValue));
> return TheValue.operator _bstr_t().operator char *();
When the function terminates, the destructor is called for the instance of
_bstr_t, and that frees the wrapped BSTR string.
So, you are basically returning a dangling pointer to the caller using the
operator char*().
Instead, you should return an instance of CString.
Giovanni
David Wilkinson
1. Your use of RunQuery() does not match its definition.
2. It is not a good practice to pass calls objects (like CString) across a DLL
boundary.
3. It is almost always better to use Unicode build with wchar_t strings in
modern Windows programs, because UTF-16 is the native character set of Windows
NT/2000/XP/Vista/7.
4. You are returning a pointer to an object that goes out of scope (this is the
actual cause of your problem).
--
David Wilkinson
Visual C++ MVP
Giovanni Dicanio
news:OZKETbJj...@TK2MSFTNGP02.phx.gbl...
> 2. It is not a good practice to pass calls objects (like CString) across a
> DLL
> boundary.
David:
is it not good to pass CString instances across DLL boundaries when the DLL
is an MFC DLL?
I used to think that in the context of MFC DLLs passing instances of
CString's (and other MFC classes) was just fine...
Giovanni
Joseph M. Newcomer
On Sun, 3 Jan 2010 07:28:23 -0800 (PST), dushkin <tal...@gmail.com> wrote:
>Hi All,
>
> I wrote a simple MFC application and a simple MFC DLL.
> I need to return a string buffer from the DLL in a RunQuery function.
>
> Please see the code below (Note that I cleared some unrelated lines
>like catch, etc, which are not related to my problem now to make code
>clearer.)
>
> If I check the "TheValue.operator _bstr_t().operator char *()" value
>in the DLL ,it gives me the correct value!
> But if I check the returned value in the APP, I get junk...
>
> What is wrong here?
****
You are making a fundamental error. See below...
****
>
> What I did was as follows (after many trials, and the code shown here
>is not necessarily what I think is the correct one...):
>
>-------------------------
> In the App:
>-------------------------
>char* CSAPI2VPGDlg::GetExtEventID( CString sInternalEventID )
****
I find it odd that you are using the obsolete 'char *' data type here. At least use
LPTSTR so it could be compiled as Unicode! But frankly, I don't see why you would EVER
want to use a string pointer of any type here! A CString would be a good return type, but
char * or even LPTSTR are both Really Bad Ideas, and show an unfortunate throwback to the
poor programming practices of C. There is no reason I could imagine a string pointer as a
reasonable solution to this problem.
I am curious why m_sExtEventId is a char[32]. This seems an exceptionally poor choice of
representation, since it violates good practice (don't use the char data type) and good
practice (don't use fixed-size arrays on the heap or stack). The notion of char should be
considered dead, and the notion of fixed-size character arrays equally dead. They are
used only in VERY rare and exotic circumstances, and I see no evidence that this even
comes close to requiring either poor practice. THINK: AS SOON AS YOU WRITE char * YOU
HAVE MADE A FUNDAMENTAL DESIGN ERROR. It isn't always true, but it is close enough to the
truth to be a good rule to follow. Similarly, AS SOON AS YOU WRITE A FIXED-SIZE ARRAY
INTO WHICH YOU ARE GOING TO WRITE DATA, YOU HAVE MADE A FUNDAMENTAL DESIGN ERROR. Again,
it isn't always true, but it is close enough to consider as a rule to be followed except
in the most rare circumstances, and I do not see any evidence that justifies thinking that
this is such a situation.
Perhaps you have some bizarre notion about "efficiency" here. Forget it. You are wrong.
Efficiency here doesn't matter in the slightest. You are making a database query, then
somehow being concerned about a few tens of nanoseconds? There is more random delay in
the network delays to the server than any badly-formulated concept of "avoiding string
copy" will use up with an extra copy (especially of a short string!) You could copy tens
of thousands of strings in the time it takes the database disk to make one half of a
rotation (this assumes that your query can be resolved in a single disk read)
Is there any correlation between this call of RunQuery, which takes three arguments, and
the DLL version of RunQuery, shown below, which takes only one?
****
>{
> RunQuery(hRC, m_sExtEventID, sInternalEventID); //m_sExtEventID is a
>32 bytes char array member
>
> return m_sExtEventID;
>}
>
>
>-------------------------
> In the DLL:
>-------------------------
>
>char* CXDBApp::RunQuery(const CString& a_sQuery)
>{
> //Find field
> CString s = a_sQuery;
****
Why is this done? It is unnecessary, and just clutter
****
> CString sField = GetFieldForCollect(s);
>
> _variant_t TheValue;
****
This does not need to be declared until much later, so why is its declaration all the way
back here?
****
>
> m_pRecordset.CreateInstance(__uuidof(Recordset));
> try
> {
> m_pRecordset->Open((LPCSTR)a_sQuery,
> m_pConnection.GetInterfacePtr(),
> adOpenDynamic,
> adLockOptimistic,
> adCmdText);
>
> while(!m_pRecordset->adoEOF)
> {
> TheValue = m_pRecordset->GetCollect((char*)_bstr_t(sField));
****
This is just weird.
First, the variable should not be declared until this point.
Second, why are you taking the Unicode value of sField, casting it to pretend it is a
pointer to 8-bit characters, and passing it into a function? This is just WRONG! It
tells the compiler "I know you think this is a Unicode character pointer, but it isn't, so
pretend it is an 8-bit pointer, and use it as such". What is the GetCollect function? (I
don't see it as part of CRecordSet, I looked). Stop thinking char is a useful data type.
****
> if(TheValue.vt!=VT_NULL){
> m_pRecordset->Close();
>
> //return((char*)_bstr_t(TheValue));
> return TheValue.operator _bstr_t().operator char *();
****
At the point where you do this return, you are returning a pointer to a STACK-allocated
variable. When the scope is exited, the _variant_t type is deallocated. This likely
means the string pointed to is deallocated.
In addition, I have no idea why you think returning a pointer to a _bstr_t data type,
which is a Unicode string, and casting this to a char *, could POSSIBLY make sense! And
what are you going to do if the second form, which is more than a little strange anyway,
discovers that there is a Unicode character in the field that has no equivalent in ANSI.
Why are you trapped in a 8-bit character mentality? ASSUME that Unicode is what you
should be doing, and forget that char and char * exist except in very rare and exotic
circumstances, of which this is CLEARLY not an example!
Since you already presume the data type CString exists, you have a couple choices here.
For example, you could declare the return type CStringW (Unicode string), and store it as
CStringW result = _bstr_t(TheValue);
return result;
or, if you are willing to live with the information lossage that comes when you convert
WCHAR to char, you could simply declare a CStringA data type (not a great idea), and do
CStringA result(_bstr_t(TheValue));
return result;
which would also work as long as you don't get a real Unicode character (value > U00FF) in
the data.
In any case, you CANNOT return pointers to local variables or data managed by local
variables from ANY function, independent of whether it is in a DLL or not. The
fundamental error here is assuming you can return a pointer to something that is being
managed by something you have no control over and expect that it will magically be valid.
You should not be surprised that you see garbage on return (and by "garbage" you meant "I
saw some specific values, that looked like this..." and would have given the values, which
were likely something like 0xCDCD, 0xFEEE, 0xDFDF, or something quite similar, which is a
dead giveaway as to what is wrong); I would have been totally amazed if anything
intelligible had been returned at all.
Note that returning a CString is not returning a pointer to a local variable; in fact, it
is returning a CString object, so there is no problem here with scope.
In case you hadn't noticed, this is C++ code. Therefore, there is no need to declare
variables at the top of the context if they are not used until much later. TheValue does
not need to be declared until its first use! So why give it a scope any larger than it
needs?
I think your basic problem is you are still trying to program using the deeply-flawed
models of a long-dead language (C), and doing it in a modern language (C++).
joe
****
> }
> }
> m_pRecordset->Close();
> }
>}
Joseph M. Newcomer [MVP]
email: newc...@flounder.com
Web: http://www.flounder.com
MVP Tips: http://www.flounder.com/mvp_tips.htm
Mihai N.
> return result;
> which would also work as long as you don't get a real Unicode character
> (value > U00FF) in the data.
Actually, the constructor does a real code page conversion using the
current system code page. So Unicode characters above U+00FF will
do just fine, as long as they are supported by the current
code page. For instance Russian characters (starting at U+0400)
will map properly on a Russian system.
--
Mihai Nita [Microsoft MVP, Visual C++]
http://www.mihai-nita.net
------------------------------------------
Replace _year_ with _ to get the real email
David Wilkinson
> David:
>
> is it not good to pass CString instances across DLL boundaries when the
> DLL is an MFC DLL?
>
> I used to think that in the context of MFC DLLs passing instances of
> CString's (and other MFC classes) was just fine...
Giovanni:
I guess I missed that. But I would have to ask what kind of "MFC DLL" this is.
If the only exports are of the kind shown here I would prefer a paradigm that
did not rely on identical compiler settings in both modules.
dushkin
That was kind of a cold shower!!!
Anyhow I learned a lot.
And I used the CString solution at the end...
Thank you and thanks to all the other guys!